1393359 - Make budget throttling depend on top level window

Status: RESOLVED FIXED

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

Description

[Andreas Farre [:farre]]

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.

Comment 1

[Andreas Farre [:farre]]

Attachment: 0001-Bug-1393359-Register-active-user-media-on-top-level-.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=350ff594df5f37f409bfe4acc71311ad0100dc68&group_state=expanded&selectedJob=127722372

Comment 2

[Andreas Farre [:farre]]

Attachment: 0002-Bug-1393359-Register-active-peer-connections-on-top-.patch

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.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8903603 [details] [diff] [review]
0002-Bug-1393359-Register-active-peer-connections-on-top-.patch

Keep the assertions about inner windowness.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 5

[Andreas Farre [:farre]]

Attachment: 0002-Bug-1393359-Register-active-peer-connections-on-top-.patch

Added patch with MOZ_ASSERT(IsInnerWindow()); r+ed for posterity.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b9ea5a59464c
https://hg.mozilla.org/mozilla-central/rev/cbcfe5871b83