Closed Bug 1488951 Opened Last year Closed Last year

Disable FastBlock after a certain number of seconds after navigation start

Categories

(Core :: Networking, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox63 --- verified
firefox64 --- verified

People

(Reporter: francois, Assigned: francois)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In order to put an upper bound on the amount of post-load blocking that FastBlock could do, let's put a limit after which it will disable itself.
I used 20 seconds as the default limit based on the fact that it's greater than the average webpage load time regardless of country and category:

https://www.machmetrics.com/speed-blog/average-page-load-times-websites-2018/
https://analytics.googleblog.com/2012/04/global-site-speed-overview-how-fast-are.html

Here's how I manually tested this:

1. Enable FastBlock Set the limit to 15 seconds (default)
2. Load https://techcrunch.com/ with devtools console open (Errors and Warnings only).
3. Wait for the page to finish loading.
4. Confirm that a bunch of tracker domains are blocked.
5. Scroll down to the bottom of the page.
6. Clear the devtools console messages.
7. Click on "Load More".
8. Confirm that trackers are not blocked anymore (nothing in the console).
Comment on attachment 9006742 [details]
Bug 1488951 - Put a limit on how long FastBlock runs. r?mayhemer,ehsan

:Ehsan Akhgari has approved the revision.
Attachment #9006742 - Flags: review+
Honza, just putting this on your Bugzilla radar in case you missed the Phabricator notification. We're hoping to uplift into Beta 63 after it lands in Nightly.
Flags: needinfo?(honzab.moz)
I'm not sure I understand how the motivation for this bug and its execution tune together.  I've seen some complain that fastblock after the 5 seconds limit causes some breakage when interacting.  Does the same happen with TP on?  If not, then this bug is definitely WONTFIX.  Because after the default 5 seconds for FB, we should behave EXACTLY the same as when TP is on.  Hence, we have already made necessary precautions to not break interaction while still blocking trackers.
Flags: needinfo?(honzab.moz) → needinfo?(francois)
(In reply to Honza Bambas (:mayhemer) from comment #6)
> I've seen some complain that fastblock after the 5 seconds limit causes
> some breakage when interacting.  Does the same happen with TP on?

Yes, it's the same.

The idea behind FastBlock is that it's intended purely as a perf feature to speed up the initial page load. As much as possible, we don't want to affect the page after it has loaded. That means that if a site delays loading ads until after the page has finished loading, we don't want to block those ads.

Ehsan and I discussed that and came up with three different ways in which we could disable FastBlock after a page load:

1. disable it on user interaction (bug 1485492)
2. disable it once the load event fires (bug 1488974)
3. disable it after a certain amount of time (this bug)

We don't know yet which of these will end up working and so we've decided to do all three and then measure them.
Flags: needinfo?(francois)
Comment on attachment 9006742 [details]
Bug 1488951 - Put a limit on how long FastBlock runs. r?mayhemer,ehsan

Honza Bambas (:mayhemer) has approved the revision.
Attachment #9006742 - Flags: review+
(In reply to François Marier [:francois] from comment #7)
> (In reply to Honza Bambas (:mayhemer) from comment #6)

Thanks for making this clear for me.
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e97cfb1a3f88
Put a limit on how long FastBlock runs. r=mayhemer,Ehsan
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3aa3df7274f4
Backed out changeset e97cfb1a3f88 by bhearsum`s request.
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3199cd1e46de
Put a limit on how long FastBlock runs. r=mayhemer,Ehsan. CLOSED TREE
https://hg.mozilla.org/mozilla-central/rev/3199cd1e46de
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment on attachment 9006742 [details]
Bug 1488951 - Put a limit on how long FastBlock runs. r?mayhemer,ehsan

Approval Request Comment
[Feature/Bug causing the regression]: FastBlock
[User impact if declined]: We will not be able to toggle a parameter in FastBlock that can reduce breakage.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes, I have manually tested it following the steps in comment 2.
[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?]: No
[Why is the change risky/not risky?]: This is only enabled in Nightly. It is disabled in Beta and release (it's set to 0).
[String changes made/needed]: None
Attachment #9006742 - Flags: approval-mozilla-beta?
Comment on attachment 9006742 [details]
Bug 1488951 - Put a limit on how long FastBlock runs. r?mayhemer,ehsan

Low-risk patch as it is a feature disabled on beta, uplift approved for 63 beta 6 thanks.
Attachment #9006742 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have reproduced the issue in Nightly v64.0a1 (2018-09-05) and I can confirm the fix in Nightly v64.0a1 (2018-09-13).
Awaiting beta build for uplift.
This issue does not seem to be fixed in Beta v63.0b6. Considering this, I have re-reproduced it in Nightly v64.0a1 from 2018-09-05 and re-confirmed the fix in Nightly v64.0a1 from 2018-09-16.

I have used the following steps to reproduce:
1. Set pref "browser.contentblocking.ui.enabled" to true;
2. Set pref "browser.fastblock.enabled" to true;
3. Set pref "browser.fastblock.timeout" to 15000 (15000miliseconds=15seconds until fastblock stops blocking);
4. Open browser and browser console;
5. Load page "https://techcrunch.com/";
(A pop-up will appear and a confirmation will be needed in order to load the page)
6. Confirm that some tracker domains are being blocked (If not, reload the page and hit the "Load more" button from the bottom of the page before the 15 seconds have elapsed to confirm that some tracker domains get blocked);
7. Wait for the 15 seconds to pass, clear the browser console and hit the "Load more" button again;
Expected: There aren't any slow loading tracker domains being blocked (errors/warnings in the browser console);
Actual: There ARE errors about slow loading tracker domains being blocked in the browser console.

@François Marier: Can you have a look at this? Is there something wrong with my STR or something else?
Flags: needinfo?(francois)
(In reply to Bodea Daniel [:danibodea] from comment #18)
> 3. Set pref "browser.fastblock.timeout" to 15000 (15000miliseconds=15seconds
> until fastblock stops blocking);

This is the problem. The pref you want to change is browser.fastblock.limit.

You can leave browser.fastblock.timeout to its default value.
Flags: needinfo?(francois)
Depends on: 1492087
This issue has been re-verified on Nightly v64.0a1 (2018-09-18) and verified in Beta v63.0b6 on Windows 10 and Ubuntu 16.04.
The issue is fixed and the uplift is successful. 

Thank you.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.