diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index b037675dc733538abb8a0723cce49b694458cf28..d47e8056fde52704a618fe148f0320ad94a3bad8 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -21,6 +21,7 @@ #endif #include "mozilla/AppShutdown.h" #include "mozilla/dom/CanonicalBrowsingContext.h" +#include "mozilla/dom/BrowserHost.h" #include "mozilla/dom/BrowserChild.h" #include "mozilla/dom/BrowserParent.h" #include "mozilla/dom/BrowsingContextGroup.h" @@ -2641,25 +2642,29 @@ void BrowsingContext::DidSet(FieldIndex<IDX_ExplicitActive>, if (IsTop()) { Group()->UpdateToplevelsSuspendedIfNeeded(); + if (XRE_IsParentProcess()) { + auto* bc = Canonical(); + if (BrowserParent* bp = bc->GetBrowserParent()) { + bp->RecomputeProcessPriority(); #if defined(XP_WIN) && defined(ACCESSIBILITY) - if (XRE_IsParentProcess() && a11y::Compatibility::IsDolphin()) { - // update active accessible documents on windows - if (BrowserParent* bp = Canonical()->GetBrowserParent()) { - if (a11y::DocAccessibleParent* tabDoc = - bp->GetTopLevelDocAccessible()) { - HWND window = tabDoc->GetEmulatedWindowHandle(); - MOZ_ASSERT(window); - if (window) { - if (isActive) { - a11y::nsWinUtils::ShowNativeWindow(window); - } else { - a11y::nsWinUtils::HideNativeWindow(window); + if (a11y::Compatibility::IsDolphin()) { + // update active accessible documents on windows + if (a11y::DocAccessibleParent* tabDoc = + bp->GetTopLevelDocAccessible()) { + HWND window = tabDoc->GetEmulatedWindowHandle(); + MOZ_ASSERT(window); + if (window) { + if (isActive) { + a11y::nsWinUtils::ShowNativeWindow(window); + } else { + a11y::nsWinUtils::HideNativeWindow(window); + } } } } +#endif } } -#endif } PreOrderWalk([&](BrowsingContext* aContext) { diff --git a/dom/ipc/BrowserHost.cpp b/dom/ipc/BrowserHost.cpp index 91843ea768c1377a0c35791d92d19bb19512a690..d0ce7c6ba46b5db58777d18c07542bf49f7dda67 100644 --- a/dom/ipc/BrowserHost.cpp +++ b/dom/ipc/BrowserHost.cpp @@ -120,10 +120,6 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) { return NS_OK; } - bool priorityHint; - GetPriorityHint(&priorityHint); - ProcessPriorityManager::BrowserPriorityChanged( - GetBrowsingContext()->Canonical(), priorityHint || aRenderLayers); mRoot->SetRenderLayers(aRenderLayers); return NS_OK; } @@ -131,11 +127,7 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) { /* readonly attribute boolean hasLayers; */ NS_IMETHODIMP BrowserHost::GetHasLayers(bool* aHasLayers) { - if (!mRoot) { - *aHasLayers = false; - return NS_OK; - } - *aHasLayers = mRoot->GetHasLayers(); + *aHasLayers = mRoot && mRoot->GetHasLayers(); return NS_OK; } @@ -145,27 +137,19 @@ BrowserHost::SetPriorityHint(bool aPriorityHint) { if (!mRoot) { return NS_OK; } - bool renderLayers; - GetRenderLayers(&renderLayers); - ProcessPriorityManager::BrowserPriorityChanged( - GetBrowsingContext()->Canonical(), aPriorityHint || renderLayers); mRoot->SetPriorityHint(aPriorityHint); return NS_OK; } NS_IMETHODIMP BrowserHost::GetPriorityHint(bool* aPriorityHint) { - if (!mRoot) { - *aPriorityHint = false; - return NS_OK; - } - *aPriorityHint = mRoot->GetPriorityHint(); + *aPriorityHint = mRoot && mRoot->GetPriorityHint(); return NS_OK; } /* void resolutionChanged (); */ NS_IMETHODIMP -BrowserHost::NotifyResolutionChanged(void) { +BrowserHost::NotifyResolutionChanged() { if (!mRoot) { return NS_OK; } @@ -177,13 +161,13 @@ BrowserHost::NotifyResolutionChanged(void) { /* void deprioritize (); */ NS_IMETHODIMP -BrowserHost::Deprioritize(void) { +BrowserHost::Deprioritize() { if (!mRoot) { return NS_OK; } - ProcessPriorityManager::BrowserPriorityChanged( - GetBrowsingContext()->Canonical(), - /* aPriority = */ false); + auto* bc = GetBrowsingContext()->Canonical(); + ProcessPriorityManager::BrowserPriorityChanged(bc, + /* aPriority = */ false); return NS_OK; } diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 6995bca654fc0beb8e3d4d8bfeb56c76981b9daf..fb46192d1cc22367be20ee549a427435dbaed246 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -162,7 +162,6 @@ using namespace mozilla::widget; using namespace mozilla::gfx; using mozilla::LazyLogModule; -using mozilla::Unused; extern mozilla::LazyLogModule gSHIPBFCacheLog; @@ -243,10 +242,18 @@ BrowserParent::BrowserParent(ContentParent* aManager, const TabId& aTabId, // that some input events are dispatched before PBrowserConstructor. mIsReadyToHandleInputEvents = !ContentParent::IsInputEventQueueSupported(); + // Make sure to compute our process priority if needed before the block of + // code below. This makes sure the block below prioritizes our process if + // needed. + if (aBrowsingContext->IsTop()) { + RecomputeProcessPriority(); + } + // If we're in a BC tree that is active with respect to the priority manager, // ensure that this new BrowserParent is marked as active. This ensures that // the process will be prioritized in a cross-site iframe navigation in an - // active tab. + // active tab, and also that the process is correctly prioritized if we got + // created for a browsing context which was already active. if (aBrowsingContext->Top()->IsPriorityActive()) { ProcessPriorityManager::BrowserPriorityChanged(this, true); } @@ -3477,6 +3484,13 @@ bool BrowserParent::GetPriorityHint() { return mPriorityHint; } void BrowserParent::SetPriorityHint(bool aPriorityHint) { mPriorityHint = aPriorityHint; + RecomputeProcessPriority(); +} + +void BrowserParent::RecomputeProcessPriority() { + auto* bc = GetBrowsingContext(); + ProcessPriorityManager::BrowserPriorityChanged( + bc, bc->IsActive() || mPriorityHint); } void BrowserParent::PreserveLayers(bool aPreserveLayers) { diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index 3edfcdc95d245c18d34112109809a5156c5ba9cf..3c1ababb03364872bc9ca70d781e23586347501b 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -132,6 +132,8 @@ class BrowserParent final : public PBrowserParent, CanonicalBrowsingContext* GetBrowsingContext() { return mBrowsingContext; } + void RecomputeProcessPriority(); + already_AddRefed<nsILoadContext> GetLoadContext(); Element* GetOwnerElement() const { return mFrameElement; } diff --git a/dom/ipc/ProcessPriorityManager.cpp b/dom/ipc/ProcessPriorityManager.cpp index 7a3e670fd7908e358b7229d2750e5d5f83711c09..94c68177cf8a970c55c823cbd07f9c195fe942aa 100644 --- a/dom/ipc/ProcessPriorityManager.cpp +++ b/dom/ipc/ProcessPriorityManager.cpp @@ -509,10 +509,20 @@ void ProcessPriorityManagerImpl::NotifyProcessPriorityChanged( } } +static nsCString BCToString(dom::CanonicalBrowsingContext* aBC) { + nsCOMPtr<nsIURI> uri = aBC->GetCurrentURI(); + return nsPrintfCString("id=%" PRIu64 " uri=%s active=%d pactive=%d", + aBC->Id(), + uri ? uri->GetSpecOrDefault().get() : nullptr, + aBC->IsActive(), aBC->IsPriorityActive()); +} + void ProcessPriorityManagerImpl::BrowserPriorityChanged( dom::CanonicalBrowsingContext* aBC, bool aPriority) { MOZ_ASSERT(aBC->IsTop()); + LOG("BrowserPriorityChanged(%s, %d)\n", BCToString(aBC).get(), aPriority); + bool alreadyActive = aBC->IsPriorityActive(); if (alreadyActive == aPriority) { return; @@ -525,6 +535,8 @@ void ProcessPriorityManagerImpl::BrowserPriorityChanged( aBC->PreOrderWalk([&](BrowsingContext* aContext) { CanonicalBrowsingContext* canonical = aContext->Canonical(); + LOG("PreOrderWalk for %p: %p -> %p, %p\n", aBC, canonical, + canonical->GetContentParent(), canonical->GetBrowserParent()); if (ContentParent* cp = canonical->GetContentParent()) { if (RefPtr pppm = GetParticularProcessPriorityManager(cp)) { if (auto* bp = canonical->GetBrowserParent()) { @@ -537,6 +549,8 @@ void ProcessPriorityManagerImpl::BrowserPriorityChanged( void ProcessPriorityManagerImpl::BrowserPriorityChanged( BrowserParent* aBrowserParent, bool aPriority) { + LOG("BrowserPriorityChanged(bp=%p, %d)\n", aBrowserParent, aPriority); + if (RefPtr pppm = GetParticularProcessPriorityManager(aBrowserParent->Manager())) { Telemetry::ScalarAdd( @@ -766,13 +780,14 @@ void ParticularProcessPriorityManager::SetPriorityNow( return; } + LOGP("Changing priority from %s to %s (cp=%p).", + ProcessPriorityToString(mPriority), ProcessPriorityToString(aPriority), + mContentParent); + if (!mContentParent || mPriority == aPriority) { return; } - LOGP("Changing priority from %s to %s.", ProcessPriorityToString(mPriority), - ProcessPriorityToString(aPriority)); - PROFILER_MARKER( "Subprocess Priority", OTHER, MarkerOptions(MarkerThreadId::MainThread(), MarkerStack::Capture()), diff --git a/dom/ipc/tests/browser_ProcessPriorityManager.js b/dom/ipc/tests/browser_ProcessPriorityManager.js index cc53f6fad2911031a5b201f111c010a7b91683f4..c5f0fb0338fb6fd052db46e050099f45b4aa012f 100644 --- a/dom/ipc/tests/browser_ProcessPriorityManager.js +++ b/dom/ipc/tests/browser_ProcessPriorityManager.js @@ -318,6 +318,19 @@ add_task(async function test_normal_background_tab() { "PriorityHint of the original tab should be false on default" ); + // Changing renderLayers doesn't change priority of the background tab. + originalTab.linkedBrowser.preserveLayers(true); + originalTab.linkedBrowser.renderLayers = true; + await new Promise(resolve => + // eslint-disable-next-line mozilla/no-arbitrary-setTimeout + setTimeout(resolve, WAIT_FOR_CHANGE_TIME_MS) + ); + Assert.equal( + gTabPriorityWatcher.currentPriority(origtabID), + PROCESS_PRIORITY_BACKGROUND, + "Tab didn't get prioritized only due to renderLayers" + ); + // Test when priorityHint is true, the original tab priority // becomes PROCESS_PRIORITY_FOREGROUND. originalTab.linkedBrowser.frameLoader.remoteTab.priorityHint = true; @@ -364,6 +377,9 @@ add_task(async function test_normal_background_tab() { PROCESS_PRIORITY_FOREGROUND, "Setting priorityHint to false should maintain the new tab priority as foreground" ); + + originalTab.linkedBrowser.preserveLayers(false); + originalTab.linkedBrowser.renderLayers = false; } ); }); diff --git a/hal/android/AndroidProcessPriority.cpp b/hal/android/AndroidProcessPriority.cpp index 64708ee0d71b57384b3339986be90d68340cec41..aab2bf50efdd72d663acff7a4824686205d8e582 100644 --- a/hal/android/AndroidProcessPriority.cpp +++ b/hal/android/AndroidProcessPriority.cpp @@ -10,6 +10,8 @@ #include "mozilla/java/GeckoProcessTypeWrappers.h" #include "mozilla/java/ServiceAllocatorWrappers.h" +using namespace mozilla::hal; + /** * Bucket the Gecko HAL process priority level into one of the three Android * priority levels. diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoViewTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoViewTest.kt index 713225ce60fa49e02cea15efafa09ef3038e2004..ad961d68525da15cc01cbd2aee1dd114ff9bb5a4 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoViewTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoViewTest.kt @@ -95,12 +95,15 @@ class GeckoViewTest : BaseSessionTest() { val lowPids = low.map { sessionRule.getSessionPid(it) }.toSet() UiThreadUtils.waitForCondition({ - getContentProcessesPriority(highPids).count { it > 100 } == 0 - && getContentProcessesPriority(lowPids).count { it < 300 } == 0 + val shouldBeHighPri = getContentProcessesOomScore(highPids) + val shouldBeLowPri = getContentProcessesOomScore(lowPids) + // Note that higher oom score means less priority + shouldBeHighPri.count { it > 100 } == 0 + && shouldBeLowPri.count { it < 300 } == 0 }, env.defaultTimeoutMillis) } - fun getContentProcessesPriority(pids: Collection<Int>) : List<Int> { + fun getContentProcessesOomScore(pids: Collection<Int>) : List<Int> { return pids.map { pid -> File("/proc/$pid/oom_score").readText(Charsets.UTF_8).trim().toInt() } @@ -162,29 +165,6 @@ class GeckoViewTest : BaseSessionTest() { } } - @Test - @NullDelegate(Autofill.Delegate::class) - fun extensionCurrentTabRaisesPriority() { - // Bug 1767346 - assumeThat(false, equalTo(true)) - - val otherSession = setupPriorityTest() - - // Setting priorityHint to PRIORITY_HIGH raises priority - mainSession.setPriorityHint(GeckoSession.PRIORITY_HIGH) - - waitUntilContentProcessPriority( - high = listOf(mainSession, otherSession), low = listOf() - ) - - // Setting the priorityHint to default should lower priority - mainSession.setPriorityHint(GeckoSession.PRIORITY_DEFAULT) - - waitUntilContentProcessPriority( - high = listOf(mainSession), low = listOf(otherSession) - ) - } - @Test @NullDelegate(Autofill.Delegate::class) fun processPriorityTest() { @@ -221,8 +201,8 @@ class GeckoViewTest : BaseSessionTest() { @Test @NullDelegate(Autofill.Delegate::class) fun setPriorityHint() { - // Bug 1767346 - assumeThat(false, equalTo(true)) + // Bug 1768102 - Doesn't seem to work on Fission + assumeThat(env.isFission || env.isIsolatedProcess, equalTo(false)) val otherSession = setupPriorityTest()