document_start scripts are running one tick later since the patch for bug 1437861

RESOLVED FIXED in Firefox 64

Status

enhancement
P1
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: robwu, Assigned: rpl)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

9 months ago
The content script injection logic is carefully designed to avoid awaiting microtasks (promises) when a document_start is being executed.

In this change, an unnecessary await was introduced:
https://hg.mozilla.org/mozilla-central/rev/f151c2fa525f#l1.12

In tests where content scripts run once, this does not result in an observable difference because the first script is not preloaded so we already have an await + wait for promise.

This problem becomes visible when a page is loaded again while a script is cached, because there is no document.blockParsing call.
Assignee: nobody → lgreco
Priority: -- → P1
Reporter

Comment 2

9 months ago
Comment on attachment 9008827 [details]
Bug 1491051 - Avoid awaiting a promise when all the content scripts are already precompiled and cached. r?mixedpuppy!,robwu

Rob Wu [:robwu] has approved the revision.
Attachment #9008827 - Flags: review+
Comment on attachment 9008827 [details]
Bug 1491051 - Avoid awaiting a promise when all the content scripts are already precompiled and cached. r?mixedpuppy!,robwu

Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #9008827 - Flags: review+

Comment 4

9 months ago
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/5d78ab715d4f
Avoid awaiting a promise when all the content scripts are already precompiled and cached. r=robwu,mixedpuppy

Comment 5

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5d78ab715d4f
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Comment 6

9 months ago
Can you please add some STRs(and a test webextesnsion if possible) or mark this issue as "qe-verify-" ?
Flags: needinfo?(lgreco)
Assignee

Comment 7

9 months ago
I'm marking this as qe-verify-, as it is tested in the automated tests added by the same patch that introduced the fix (and it is not an easy one to verify with manual testing).
Flags: needinfo?(lgreco) → qe-verify-
You need to log in before you can comment on or make changes to this bug.