Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chrome.scripting.executeScript doesn't resolve/reject for frozen state tabs #527

Open
piyu-sh opened this issue Jan 19, 2024 · 6 comments
Open
Labels
breaking change discussion Needs further discussion spec clarification Needs clarification when specified

Comments

@piyu-sh
Copy link

piyu-sh commented Jan 19, 2024

Prior context:
I originally raised an issue here https://bugs.chromium.org/p/chromium/issues/detail?id=1429905
then started a thread on google groups where @oliverdunk suggested to open an issue here

Problem:
When trying to run chrome.scripting.executeScript over tabs that are in frozen state, the promise doesn't return, neither it resolves nor rejects.
The operation is queued, and when the tab is unfrozen, then the operation runs.
This is problematic, because a developer has no way of knowing that a tab is frozen, so they might try running an operation on such a tab but will get stuck because executeScript will just get stuck.

Suggestions on handling this:

  1. resolve/reject from executeScript and throw an error just like executeScript throws error for unloaded/discarded tabs
  2. provide a field like isFrozen in Tab Object which is returned from chrome.tabs.query() so that user can decide what to do with such tabs
@maxnikulin
Copy link

I like the idea to be able to determine if a script can be executed in the particular tab. Besides tabs.query() it would be handy to have similar fields in Tab objects passed to action.onClicked and contextMenus.onClicked event listeners.

I am unsure however that it is always possible. A tab may be busy in a tight loop executing some page script function. In general it is susceptible to race conditions since tab state may change between tabs.query() and sctipting.executeScript() calls.

To handle scripting.executeScript() issues I suggested to add an AbortSignal parameter, see #415.

It seems there are 2 different use cases for scripting.executeScript():

  • Install event listeners when the page is ready, so it is not a failure if the script is never executed since the user never interacts with the page.
  • Obtain some data from the page or fail immediately if it is busy/frozen/loading.

To make second use case an atomic operation, it is necessary to add a scripting.executeScript() parameter determining execution policy.

@Rob--W
Copy link
Member

Rob--W commented Feb 15, 2024

  1. resolve/reject from executeScript and throw an error just like executeScript throws error for unloaded/discarded tabs

I'd be willing to specify this behavior more clearly, and/or change the default behavior based on options.

  1. provide a field like isFrozen in Tab Object which is returned from chrome.tabs.query() so that user can decide what to do with such tabs

Depending on what "frozen" means, this is prone to race conditions and would also require potentially expensive bookkeeping on the browser's end.

@fregante
Copy link

Frozen just means that JavaScript thread is busy. It "freezes" by definition of being single-threaded, so what you're asking is essentially to have a timeout:

  • The tab didn't answer for 100ms, is it frozen?
  • The tab didn't answer for 1s, is it frozen?

When adding code to execute to the event loop, via DOM events or via setTimeout, you basically expect the code to "run eventually". I'm not sure that this situation is substantially different. You can implement your own timeout or perhaps add support for signal in executeScripts

@oliverdunk
Copy link
Member

Frozen just means that JavaScript thread is busy.

In this case it's a discrete state the browser can use: https://developer.chrome.com/docs/web-platform/page-lifecycle-api#states. I don't know if there are internal timeouts but theoretically, it would be possible for it to remain in this state for a long time and then for the script to execute when it is eventually restored.

Depending on what "frozen" means, this is prone to race conditions and would also require potentially expensive bookkeeping on the browser's end.

Agreed on the race condition concern, and a new property on APIs like tabs.query would only resolve things for developers who go out of their way to handle this state. I don't think bookkeeping is a problem (at least in Chrome) since we show the state at chrome://discards/.

Also I just wanted to clarify - I'm not a huge fan of the behaviour today. I mentioned in the mailing list that the current behaviour might be ok, but I think that was bad phrasing on my part. What I wanted to call out was that this isn't necessarily a bug, since the request wasn't being lost and did have a chance of eventually being handled. I agree it isn't particularly useful behaviour though (at least, I don't think so).

@fregante
Copy link

Browsers freeze pages as a way to preserve CPU/battery/data usage

Oh, I didn't realize this. Since this is intentional, the browser should really just thaw the tab. The same thing happens when you message the worker: it wakes up after the browser deactivates it.

Also yes, since this is an official "state" of the tab, then probably it should be exposed the same way "discarded" is

@piyu-sh
Copy link
Author

piyu-sh commented Feb 16, 2024

  1. provide a field like isFrozen in Tab Object which is returned from chrome.tabs.query() so that user can decide what to do with such tabs

Depending on what "frozen" means, this is prone to race conditions and would also require potentially expensive bookkeeping on the browser's end.

Hmm, agreed on the race condition part, but can this race condition not happen currently for discarded tabs as well, then why allow it for a certain state and not for other?

  1. resolve/reject from executeScript and throw an error just like executeScript throws error for unloaded/discarded tabs

I'd be willing to specify this behavior more clearly, and/or change the default behavior based on options.

Would adding a flag like failIImediately to executeScript's ScriptInjection help? This flag when true rejects the promise early if tab is frozen instead of queing the task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change discussion Needs further discussion spec clarification Needs clarification when specified
Projects
None yet
Development

No branches or pull requests

6 participants