Closed
Bug 1355311
Opened 8 years ago
Closed 8 years ago
Enable throttling of tracking timeouts by default
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: farre)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
farre
:
review+
|
Details | Diff | Splinter Review |
We'll use this bug to track the work to enable the work done in bug 1325467 by default.
Comment 1•8 years ago
|
||
Seems like maybe we should fix bug 1339909 before enabling this? Or are you not going to use that feature?
Depends on: 1339909
Comment 2•8 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.
Flags: needinfo?(afarre)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8864124 -
Flags: review?(ehsan)
Comment 5•8 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.
Comment 6•8 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.
Assignee | ||
Comment 7•8 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)
Attachment #8864124 -
Attachment is obsolete: true
Attachment #8864124 -
Flags: review?(ehsan)
Attachment #8864142 -
Flags: review?(ehsan)
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8864142 -
Attachment is obsolete: true
Attachment #8864142 -
Flags: review?(ehsan)
Attachment #8864225 -
Flags: review?(ehsan)
Reporter | ||
Comment 9•8 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.
Attachment #8864225 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 10•8 years ago
|
||
Added comment about privacy.trackingprotection.annotate_channels pref. Carrying over r+.
Attachment #8864225 -
Attachment is obsolete: true
Attachment #8864508 -
Flags: review+
Comment 11•8 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
Keywords: dev-doc-needed
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 12•8 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
Keywords: checkin-needed
Comment 13•8 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
Flags: needinfo?(afarre)
Comment 14•8 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
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 16•8 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
Keywords: checkin-needed
Comment 17•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 18•8 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!
Keywords: dev-doc-needed → dev-doc-complete
Comment 19•8 years ago
|
||
Backed out for causing crash in mozilla::dom::TimeoutManager::MaybeStartThrottleTrackingTimout (bug 1366812):
https://hg.mozilla.org/mozilla-central/rev/f9ca97a334296facd2e0ea5582e7f12d0fe70fe4
Status: RESOLVED → REOPENED
Flags: needinfo?(afarre)
Resolution: FIXED → ---
Comment 20•8 years ago
|
||
Relanding because bug 1367025 may have fixed the cause of the crashes.
Comment 21•8 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
Comment 22•8 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•