Closed
Bug 1394348
Opened 7 years ago
Closed 7 years ago
Web Extensions executeScript induces a 1s delay
Categories
(WebExtensions :: General, defect, P1)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: adomas.ven, Assigned: zombie)
References
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36 Steps to reproduce: I've built a small testing framework for web extensions. Since Firefox 55 it is causing timeouts when opening tabs. Upon greater inspection I realised that each browser.runtime.executeScript() call induces a 1 sec async delay. 1. Clone git@github.com:adomasven/firefox-problems.git 2. Load in Firefox 55 3. Navigate to moz-extension://[ext-id]/test/test.html Relevant code is in test/tests/firefoxTabFailTest.js Actual results: Each executeScript invocation caused at least 1s async delay. Expected results: No delays should occur. This used to work fine in Firefox 55, works fine in Firefox ESR 52 and Chrome. https://bugzilla.mozilla.org/show_bug.cgi?id=1389381 may be relevant
Updated•7 years ago
|
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
Comment 1•7 years ago
|
||
Sorry, I can't really make any sense of what's going on in that code. There's too much indirection. Can you provide a simplified testcase that shows the problem you're seeing?
Comment 2•7 years ago
|
||
Ah, I see. This is because the default value for runAt is "document_idle", which prior to Firefox 55 we treated the same as "document_end", but now waits for an idle callback before injecting scripts as expected.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
What is the way to inject multiple scripts into a page and make sure that sequentiality is preserved?
Comment 4•7 years ago
|
||
You can use something like `runAt: "document_end"`
Assignee | ||
Comment 5•7 years ago
|
||
This seems like an unnecessary delay. I think that for tabs.executeScript(), if the document is already past `document_end`, we should treat both the default and `document_idle` as "right away".
Assignee: nobody → tomica
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Comment 6•7 years ago
|
||
document_idle means that the script is not urgent and should wait for an idle slot in the event queue before executing. It shouldn't make a difference whether the document has finished loading or not when executeScript is called. If callers want the script to execute sooner, they can use document_start or document_end runAt values.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 7•7 years ago
|
||
> <zombie> John-Galt: "In the case of "document_idle", the browser chooses a time to inject scripts between "document_end" and immediately after the window.onload event fires. "
> <John-Galt> zombie: It didn't used to be documented that way...
> <zombie> and we wait for the global event queue to be empty, which is not related to the document at hand
> <zombie> *might not be
> <John-Galt> OK, in that case I suppose we should Promise.race(idleCallback, loaded) :/
> <zombie> John-Galt: i still think that for content_scripts current behavior is fine
> <zombie> just for executeScript() seems unnecessary
> <John-Galt> We should probably just do the promise race thing.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Comment 8•7 years ago
|
||
Given the number of content scripts out there and how this affects the default, making a P1. If I'm reading this wrong, please let me know.
Priority: -- → P1
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8905696 -
Flags: review?(kmaglione+bmo)
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8905696 [details] Bug 1394348 - Fix document_idle run_at timing https://reviewboard.mozilla.org/r/177496/#review182570
Attachment #8905696 -
Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by tomica@gmail.com: https://hg.mozilla.org/integration/autoland/rev/7b60645c8bfc Fix document_idle run_at timing r=kmag
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7b60645c8bfc
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 18•6 years ago
|
||
Verified in FF57(Win7 64 bit) I have followed the steps from comment 0, loaded the extension and navigated to moz-extension://[firefoxproblems@adomasven.com]/test/test.html but all that I can see is a blank page that keeps loading. Nothing is displayed in browser console. Can you please provide more info regarding this issue or provide additional STR or if no manual testing is required please set the “qe-verify -“ flag.
Flags: needinfo?(tomica)
Assignee | ||
Comment 19•6 years ago
|
||
No need for manual verification, thanks.
Flags: needinfo?(tomica) → qe-verify-
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•