1488974 - Disable FastBlock after the load event is triggered

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[François Marier [:francois]]

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).

Comment 1

[François Marier [:francois]]

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?

Comment 2

[(no longer active)]

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

Comment 3

[Gary Chen [:xeonchen]]

This would probably break the test cases in bug 1477046, which adjusts browser.fastblock.timeout dynamically.

Comment 4

[François Marier [:francois]]

Attachment: Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

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 5

[François Marier [:francois]]

Manually tested using https://mozilla.github.io/tracking-test/fastblock-bug1488974.html

Comment 6

[(no longer active)]

Comment on attachment 9012435 [details]
Bug 1488974 - Disable FastBlock after the load event has fired. r?ehsan,mayhemer

:Ehsan Akhgari has approved the revision.

Comment 7

[Honza Bambas (:mayhemer)]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c1057bba224
Disable FastBlock after the load event has fired. r=mayhemer,Ehsan

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/0c1057bba224

Comment 10

[François Marier [:francois]]

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.

Comment 11

[Pascal Chevrel:pascalc]

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.

Comment 12

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c6d12dab44f0