Closed Bug 1488974 Opened Last year Closed Last year

Disable FastBlock after the load event is triggered

Categories

(Core :: Networking, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: francois, Assigned: francois)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

We should disable FastBlock as soon as the load event for the page has fired.

This will let some trackers load (the one which are delayed until after the page has finished loading).
Whiteboard: [necko-triaged]
My plan is to copy baku's patch from bug 1485492 (https://hg.mozilla.org/mozilla-central/rev/3b26f3da46d8) and add a hasDocumentLoaded field to the LoadInfo object.

I would set that field to true in LoadInfo from within nsIDocument::UnblockDOMContentLoaded().

Ehsan, does that sound good to you or is there an easier way to do this?
Flags: needinfo?(ehsan)
That sounds about right.  One question though, did you want to capture the load event or DOMContentLoaded?  The above is for the former.  The latter can be caught here: https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/base/nsGlobalWindowInner.cpp#2076
Flags: needinfo?(ehsan)
This would probably break the test cases in bug 1477046, which adjusts browser.fastblock.timeout dynamically.
The test used to assume that the load event didn't matter and so
the expected values had to be updated to match the new behavior.

A new "slowIFrame" test was added to capture what was previously
tested by the "badIFrame".
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

:Ehsan Akhgari has approved the revision.
Attachment #9012435 - Flags: review+
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

Honza Bambas (:mayhemer) has approved the revision.
Attachment #9012435 - Flags: review+
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c1057bba224
Disable FastBlock after the load event has fired. r=mayhemer,Ehsan
https://hg.mozilla.org/mozilla-central/rev/0c1057bba224
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

Approval Request Comment
[Feature/Bug causing the regression]: FastBlock
[User impact if declined]: Scripts, iframes, XHRs that occur after a page load can get blocked by FastBlock leading to various page breakage.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes, see comment 5.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not very.
[Why is the change risky/not risky?]: It's a relatively straightforward change and is covered by mochitests.
[String changes made/needed]: None.
Attachment #9012435 - Flags: approval-mozilla-beta?
Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

P1 on a feature for which we conduct a shield study on 63, uplift accepted for 63 beta 11, thanks.
Attachment #9012435 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.