Closed Bug 1393359 Opened 7 years ago Closed 7 years ago

Make budget throttling depend on top level window

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: farre, Assigned: farre)

References

Details

Attachments

(2 files, 1 obsolete file)

At the moment when we try to decide if we should allow budget throttling we handle different reasons for disable budget throttling in different ways, e.g. audio playing and indexeddb are for all windows with the same top level window, while gUM and WebRTC are per actual window.

This should be changed so that everything is per top level window.
Blocks: 1393056, 1377766
Assignee: nobody → afarre
No longer blocks: 1393056
This changes the existing behaviour of HasActivePeerConnections slightly, since it considers active connections for the entire tree of windows given a top level window.

This should be fine, since the only other consumer than TimeoutManager::BudgetThrottlingEnabled is nsDocument::CanSavePresentation, which will recusively check all windows anyway.
Attachment #8903603 - Flags: review?(bugs)
Attachment #8903602 - Flags: review?(bugs) → review+
Comment on attachment 8903603 [details] [diff] [review]
0002-Bug-1393359-Register-active-peer-connections-on-top-.patch

Keep the assertions about inner windowness.
Attachment #8903603 - Flags: review?(bugs) → review+
Comment on attachment 8903602 [details] [diff] [review]
0001-Bug-1393359-Register-active-user-media-on-top-level-.patch

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

Lgtm.

::: dom/base/TimeoutManager.cpp
@@ +1262,1 @@
>      return false;

Not a change in this patch, just wanted mention this definition of "active" includes windows with pending gum permission prompts.

There's a GetActiveMediaCaptureWindows() that UX uses to return the narrower subset that's actively capturing, but chasing that might be tricky to get right, and probably not worth chasing.

[1] https://dxr.mozilla.org/mozilla-central/rev/13d241d08912be31884f9d0d0e805b25343d6c0a/dom/media/MediaManager.cpp#3280
Attachment #8903602 - Flags: review?(jib) → review+
Added patch with MOZ_ASSERT(IsInnerWindow()); r+ed for posterity.
Attachment #8903603 - Attachment is obsolete: true
Attachment #8904235 - Flags: review+
Pushed by afarre@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9ea5a59464c
Register active user media on top level window. r=smaug,jib
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbcfe5871b83
Register active peer connections on top level window. r=smaug
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.