(no longer active) Reporter
	Description • 7 years ago

We'll use this bug to track the work to enable the work done in bug 1325467 by default.

	Ben Kelly [:bkelly, not reviewing]
	Comment 1 • 7 years ago

Seems like maybe we should fix bug 1339909 before enabling this?  Or are you not going to use that feature?

	Chris Peterson [:cpeterson]
	Comment 2 • 7 years ago

Andreas, can you take a look at ThrottleTrackingTimeoutsCallback bug 1339909? It looks like Ehsan posted a patch last month, but Ben asked for some changes.

	Andreas Farre [:farre] Assignee
	Comment 3 • 7 years ago

On it.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 5 • 7 years ago

I'd wish we had prefs for this and enable this also in foreground tabs in nightly.
That way we'll see hopefully sooner whether this causes regressions.

	Ben Kelly [:bkelly, not reviewing]
	Comment 6 • 7 years ago

A pref would be nice.  It makes it a lot easier to handle problems in the wild since we can flip it via a system addon without cutting a new release.

	Andreas Farre [:farre] Assignee
	Comment 7 • 7 years ago

Accidentally almost turned on foreground throttling as well. Changed it to be on using an ifdef NIGHTLY_BUILD.

With the current setup this is controlled by the hidden prefs and default values:

* dom.timeout.tracking_throttling_delay: 30,000 (30 seconds)
* dom.min_tracking_timeout_value:
    if nightly then 10,000 (10 seconds) else dom.min_timeout_value
* dom.min_tracking_background_timeout_value: 10,000 (10 seconds)

	(no longer active) Reporter
	Comment 9 • 7 years ago

Comment on attachment 8864225 [details] [diff] [review]
0001-Bug-1355311-Set-default-values-for-throttling-backgr.patch

Review of attachment 8864225 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Please make sure an intent to ship email is sent indicating that this is being turned on for Nightly...

::: modules/libpref/init/all.js
@@ +1210,4 @@
>  pref("dom.min_timeout_value", 4);
>  // And for background windows
>  pref("dom.min_background_timeout_value", 1000);
> +// Timeout clamp in ms for tracking timeouts we clamp

Please add a comment saying that the prefs below will only have an effect if the privacy.trackingprotection.annotate_channels pref is set to true.  You can copy the comment that we have for the privacy.trackingprotection.lower_network_priority already.

	Andreas Farre [:farre] Assignee
	Comment 10 • 7 years ago

Added comment about privacy.trackingprotection.annotate_channels pref. Carrying over r+.

	Eric Shepherd [:sheppy]
	Comment 11 • 7 years ago

The throttling will potentially effect web developers, so this change needs to be covered by documentation.

For document team: reference this email thread, which is full of very useful information: http://bit.ly/2qe9vhZ

	Pulsebot
	Comment 12 • 7 years ago

Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0542e8e2295b
Set default values for throttling background timeouts. r=ehsan

	Iris Hsiao [:ihsiao]
	Comment 13 • 7 years ago

sorry had to back this out for assertion failures in TimeoutManager.cpp like https://treeherder.mozilla.org/logviewer.html#?job_id=99078033&repo=mozilla-inbound&lineNumber=8458

	Pulsebot
	Comment 14 • 7 years ago

Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ff22abe5736
Backed out changeset 0542e8e2295b for assertion failure in TimeoutManager.cpp

	Andreas Farre [:farre] Assignee
	Comment 15 • 7 years ago

Thanks :ihsiao, I'm on it.

	Pulsebot
	Comment 16 • 7 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6411a4abcc1a
Set default values for throttling background timeouts. r=ehsan

	Phil Ringnalda (:philor)
	Comment 17 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6411a4abcc1a

	Chris Mills [:cmills]
	Comment 18 • 7 years ago

I've added notes (and updated browser compat info) on:

https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Throttling_of_tracking_timeout_scripts
https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setInterval#Throttling_of_intervals

I've also added a note to the Fx55 rel notes page:

https://developer.mozilla.org/en-US/Firefox/Releases/55#DOM_HTML_DOM

Let me know if this reads OK, or if you think anything else is required. Thanks!

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 19 • 7 years ago

Backed out for causing crash in mozilla::dom::TimeoutManager::MaybeStartThrottleTrackingTimout (bug 1366812):

https://hg.mozilla.org/mozilla-central/rev/f9ca97a334296facd2e0ea5582e7f12d0fe70fe4

	William Chen [:wchen]
	Comment 20 • 7 years ago

Relanding because bug 1367025 may have fixed the cause of the crashes.

	Pulsebot
	Comment 21 • 7 years ago

Pushed by wchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecb5a09e6371
Set default values for throttling background timeouts. r=ehsan

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 22 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/ecb5a09e6371