Backed out changeset 7b3a02a659ef (bug 1767346) for causing geckoview failures. CLOSED TREE

docshell/base/BrowsingContext.cpp

@@ -21,7 +21,6 @@
 #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"
@@ -2642,29 +2641,25 @@ 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 (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);
-              }
+    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);
             }
           }
         }
-#endif
       }
     }
+#endif
   }
 
   PreOrderWalk([&](BrowsingContext* aContext) {

dom/ipc/BrowserHost.cpp

@@ -120,6 +120,10 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) {
     return NS_OK;
   }
 
+  bool priorityHint;
+  GetPriorityHint(&priorityHint);
+  ProcessPriorityManager::BrowserPriorityChanged(
+      GetBrowsingContext()->Canonical(), priorityHint || aRenderLayers);
   mRoot->SetRenderLayers(aRenderLayers);
   return NS_OK;
 }
@@ -127,7 +131,11 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) {
 /* readonly attribute boolean hasLayers; */
 NS_IMETHODIMP
 BrowserHost::GetHasLayers(bool* aHasLayers) {
-  *aHasLayers = mRoot && mRoot->GetHasLayers();
+  if (!mRoot) {
+    *aHasLayers = false;
+    return NS_OK;
+  }
+  *aHasLayers = mRoot->GetHasLayers();
   return NS_OK;
 }
 
@@ -137,19 +145,27 @@ 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) {
-  *aPriorityHint = mRoot && mRoot->GetPriorityHint();
+  if (!mRoot) {
+    *aPriorityHint = false;
+    return NS_OK;
+  }
+  *aPriorityHint = mRoot->GetPriorityHint();
   return NS_OK;
 }
 
 /* void resolutionChanged (); */
 NS_IMETHODIMP
-BrowserHost::NotifyResolutionChanged() {
+BrowserHost::NotifyResolutionChanged(void) {
   if (!mRoot) {
     return NS_OK;
   }
@@ -161,13 +177,13 @@ BrowserHost::NotifyResolutionChanged() {
 
 /* void deprioritize (); */
 NS_IMETHODIMP
-BrowserHost::Deprioritize() {
+BrowserHost::Deprioritize(void) {
   if (!mRoot) {
     return NS_OK;
   }
-  auto* bc = GetBrowsingContext()->Canonical();
-  ProcessPriorityManager::BrowserPriorityChanged(bc,
-                                                 /* aPriority = */ false);
+  ProcessPriorityManager::BrowserPriorityChanged(
+      GetBrowsingContext()->Canonical(),
+      /* aPriority = */ false);
   return NS_OK;
 }
 

dom/ipc/BrowserParent.cpp

@@ -162,6 +162,7 @@ using namespace mozilla::widget;
 using namespace mozilla::gfx;
 
 using mozilla::LazyLogModule;
+using mozilla::Unused;
 
 extern mozilla::LazyLogModule gSHIPBFCacheLog;
 
@@ -242,18 +243,10 @@ 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, and also that the process is correctly prioritized if we got
-  // created for a browsing context which was already active.
+  // active tab.
   if (aBrowsingContext->Top()->IsPriorityActive()) {
     ProcessPriorityManager::BrowserPriorityChanged(this, true);
   }
@@ -3484,13 +3477,6 @@ 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) {

dom/ipc/BrowserParent.h

@@ -132,8 +132,6 @@ class BrowserParent final : public PBrowserParent,
 
   CanonicalBrowsingContext* GetBrowsingContext() { return mBrowsingContext; }
 
-  void RecomputeProcessPriority();
-
   already_AddRefed<nsILoadContext> GetLoadContext();
 
   Element* GetOwnerElement() const { return mFrameElement; }

dom/ipc/ProcessPriorityManager.cpp

@@ -509,20 +509,10 @@ 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;
@@ -535,8 +525,6 @@ 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()) {
@@ -549,8 +537,6 @@ 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(
@@ -780,14 +766,13 @@ 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()),

dom/ipc/tests/browser_ProcessPriorityManager.js

@@ -318,19 +318,6 @@ 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;
@@ -377,9 +364,6 @@ 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;
     }
   );
 });

hal/android/AndroidProcessPriority.cpp

@@ -10,8 +10,6 @@
 #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.

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoViewTest.kt

@@ -95,15 +95,12 @@ class GeckoViewTest : BaseSessionTest() {
         val lowPids = low.map { sessionRule.getSessionPid(it) }.toSet()
 
         UiThreadUtils.waitForCondition({
-            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
+            getContentProcessesPriority(highPids).count { it > 100 } == 0
+                    && getContentProcessesPriority(lowPids).count { it < 300 } == 0
         }, env.defaultTimeoutMillis)
     }
 
-    fun getContentProcessesOomScore(pids: Collection<Int>) : List<Int> {
+    fun getContentProcessesPriority(pids: Collection<Int>) : List<Int> {
         return pids.map { pid ->
             File("/proc/$pid/oom_score").readText(Charsets.UTF_8).trim().toInt()
         }
@@ -165,6 +162,29 @@ 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() {
@@ -201,6 +221,9 @@ class GeckoViewTest : BaseSessionTest() {
     @Test
     @NullDelegate(Autofill.Delegate::class)
     fun setPriorityHint() {
+        // Bug 1767346
+        assumeThat(false, equalTo(true))
+
         val otherSession = setupPriorityTest()
 
         // Setting priorityHint to PRIORITY_HIGH raises priority