Bug 1701368 - Part6: Tab unloading precedes memory pressure events. r=gsvelto

This is the main part to address bug 1701368.

Before this patch, `nsAvailableMemoryWatcher` directly broadcasted a memory-pressure
event when we enter into a low-memory situation and `TabUnloader` unloaded a tab in
response to the memory-pressure message.  We want to decouple `TabUnloader` from
memory-pressure listeners because unloading a tab may solve a low-memory situation
alone.

With this patch, if `nsAvailableMemoryWatcher` detects a low-memory situation,
it invokes `TabUnloader` synchronously via an XPCOM interface.  If `TabUnloader`
unloads a tab, we don't do any further action.  If there is no discardable tab,
`TabUnloader` notifies back `nsAvailableMemoryWatcher` via another XPCOM interface,
so that `nsAvailableMemoryWatcher` can notify of a memory-pressure event.

Differential Revision: https://phabricator.services.mozilla.com/D117673

browser/modules/TabUnloader.jsm

@@ -142,19 +142,34 @@ var TabUnloader = {
    */
   init() {
     if (Services.prefs.getBoolPref("browser.tabs.unloadOnLowMemory", true)) {
-      Services.obs.addObserver(this, "memory-pressure", /* ownsWeak */ true);
-
-      // eslint-disable-next-line no-unused-vars
       const watcher = Cc["@mozilla.org/xpcom/memory-watcher;1"].getService(
         Ci.nsIAvailableMemoryWatcherBase
       );
+      watcher.registerTabUnloader(this);
     }
   },
 
-  observe(subject, topic, data) {
-    if (topic == "memory-pressure" && data != "heap-minimize") {
-      this.unloadLeastRecentlyUsedTab();
+  // This method is exposed on nsITabUnloader
+  async unloadTabAsync() {
+    const watcher = Cc["@mozilla.org/xpcom/memory-watcher;1"].getService(
+      Ci.nsIAvailableMemoryWatcherBase
+    );
+
+    if (this._isUnloading) {
+      // Don't post multiple unloading requests.  The situation may be solved
+      // when the active unloading task is completed.
+      Services.console.logStringMessage("Unloading a tab is in progress.");
+      watcher.onUnloadAttemptCompleted(Cr.NS_ERROR_ABORT);
+      return;
     }
+
+    this._isUnloading = true;
+    const isTabUnloaded = await this.unloadLeastRecentlyUsedTab();
+    this._isUnloading = false;
+
+    watcher.onUnloadAttemptCompleted(
+      isTabUnloaded ? Cr.NS_OK : Cr.NS_ERROR_NOT_AVAILABLE
+    );
   },
 
   /**
@@ -249,19 +264,25 @@ var TabUnloader = {
 
   /**
    * Select and discard one tab.
+   * @returns true if a tab was unloaded, otherwise false.
    */
   async unloadLeastRecentlyUsedTab() {
     let sortedTabs = await this.getSortedTabs();
 
     for (let tabInfo of sortedTabs) {
       if (tabInfo.weight == NEVER_DISCARD) {
-        return;
+        return false;
       }
 
+      const remoteType = tabInfo.tab?.linkedBrowser?.remoteType;
       if (tabInfo.gBrowser.discardBrowser(tabInfo.tab)) {
-        return;
+        Services.console.logStringMessage(
+          `TabUnloader discarded <${remoteType}>`
+        );
+        return true;
       }
     }
+    return false;
   },
 
   QueryInterface: ChromeUtils.generateQI([

browser/modules/test/browser/browser_TabUnloader.js

@@ -65,16 +65,36 @@ async function addWebRTCTab(win = window) {
   return tab;
 }
 
-async function pressure(tab, observerData = "low-memory") {
+async function pressure(tab) {
   let tabDiscarded = BrowserTestUtils.waitForEvent(
     document,
     "TabBrowserDiscarded",
     true
   );
-  TabUnloader.observe(null, "memory-pressure", observerData);
+  TabUnloader.unloadTabAsync();
   return tabDiscarded;
 }
 
+function pressureAndObserve(aExpectedTopic) {
+  const promise = new Promise(resolve => {
+    const observer = {
+      QueryInterface: ChromeUtils.generateQI([
+        "nsIObserver",
+        "nsISupportsWeakReference",
+      ]),
+      observe(aSubject, aTopicInner, aData) {
+        if (aTopicInner == aExpectedTopic) {
+          Services.obs.removeObserver(observer, aTopicInner);
+          resolve(aData);
+        }
+      },
+    };
+    Services.obs.addObserver(observer, aExpectedTopic);
+  });
+  TabUnloader.unloadTabAsync();
+  return promise;
+}
+
 async function compareTabOrder(expectedOrder) {
   let tabInfo = await TabUnloader.getSortedTabs();
 
@@ -92,8 +112,15 @@ const PREF_PERMISSION_FAKE = "media.navigator.permission.fake";
 const PREF_AUDIO_LOOPBACK = "media.audio_loopback_dev";
 const PREF_VIDEO_LOOPBACK = "media.video_loopback_dev";
 const PREF_FAKE_STREAMS = "media.navigator.streams.fake";
+const PREF_ENABLE_UNLOADER = "browser.tabs.unloadOnLowMemory";
 
 add_task(async function test() {
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref(PREF_ENABLE_UNLOADER);
+  });
+  Services.prefs.setBoolPref(PREF_ENABLE_UNLOADER, true);
+  TabUnloader.init();
+
   // Set some WebRTC simulation preferences.
   let prefs = [
     [PREF_PERMISSION_FAKE, true],
@@ -149,26 +176,6 @@ add_task(async function test() {
     "tabs are present"
   );
 
-  // Check that heap-minimize memory-pressure events do not unload tabs
-  TabUnloader.observe(null, "memory-pressure", "heap-minimize");
-  ok(
-    tab1.linkedPanel &&
-      tab2.linkedPanel &&
-      pinnedTab.linkedPanel &&
-      soundTab.linkedPanel &&
-      pinnedSoundTab.linkedPanel,
-    "heap-minimize memory-pressure notification did not unload a tab"
-  );
-
-  await compareTabOrder([
-    tab1,
-    tab2,
-    pinnedTab,
-    soundTab,
-    pinnedSoundTab,
-    tab0,
-  ]);
-
   // Check that low-memory memory-pressure events unload tabs
   await pressure(tab1);
   ok(
@@ -202,18 +209,13 @@ add_task(async function test() {
   ok(!pinnedSoundTab.linkedPanel, "unloaded a pinned tab playing sound");
   await compareTabOrder([]); // note that no tabs are returned when there are no discardable tabs.
 
-  // Check low-memory-ongoing events
-  await BrowserTestUtils.switchTab(gBrowser, tab1);
-  await BrowserTestUtils.switchTab(gBrowser, tab0);
-
-  await compareTabOrder([tab1, tab0]);
-
-  await pressure(tab1, "low-memory-ongoing");
-  ok(
-    !tab1.linkedPanel,
-    "low-memory-ongoing memory-pressure notification unloaded the LRU tab"
+  // It's possible that we're already in the memory-pressure state
+  // and we may receive the "ongoing" message.
+  const message = await pressureAndObserve("memory-pressure");
+  Assert.ok(
+    message == "low-memory" || message == "low-memory-ongoing",
+    "observed the memory-pressure notification because of no discardable tab"
   );
-  await compareTabOrder([]);
 
   // Add a WebRTC tab and another sound tab.
   let webrtcTab = await addWebRTCTab();

xpcom/base/AvailableMemoryWatcher.cpp

@@ -7,12 +7,35 @@
 #include "AvailableMemoryWatcher.h"
 
 #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/dom/Promise.h"
+#include "mozilla/ErrorResult.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticPtr.h"
+#include "nsMemoryPressure.h"
 #include "nsXULAppAPI.h"
 
 namespace mozilla {
 
+// Use this class as the initial value of
+// nsAvailableMemoryWatcherBase::mCallback until RegisterCallback() is called
+// so that nsAvailableMemoryWatcherBase does not have to check if its callback
+// object is valid or not.
+class NullTabUnloader final : public nsITabUnloader {
+  ~NullTabUnloader() = default;
+
+ public:
+  NullTabUnloader() = default;
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSITABUNLOADER
+};
+
+NS_IMPL_ISUPPORTS(NullTabUnloader, nsITabUnloader)
+
+NS_IMETHODIMP NullTabUnloader::UnloadTabAsync() {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 StaticRefPtr<nsAvailableMemoryWatcherBase>
     nsAvailableMemoryWatcherBase::sSingleton;
 
@@ -29,11 +52,27 @@ nsAvailableMemoryWatcherBase::GetSingleton() {
 
 NS_IMPL_ISUPPORTS(nsAvailableMemoryWatcherBase, nsIAvailableMemoryWatcherBase);
 
-nsAvailableMemoryWatcherBase::nsAvailableMemoryWatcherBase() {
+nsAvailableMemoryWatcherBase::nsAvailableMemoryWatcherBase()
+    : mTabUnloader(new NullTabUnloader) {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Watching memory only in the main process.");
 }
 
+nsresult nsAvailableMemoryWatcherBase::RegisterTabUnloader(
+    nsITabUnloader* aTabUnloader) {
+  mTabUnloader = aTabUnloader;
+  return NS_OK;
+}
+
+nsresult nsAvailableMemoryWatcherBase::OnUnloadAttemptCompleted(
+    nsresult aResult) {
+  if (aResult == NS_ERROR_NOT_AVAILABLE) {
+    // If there was no unloadable tab, declare the memory-pressure
+    NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
+  }
+  return NS_OK;
+}
+
 // Define the fallback method for a platform for which a platform-specific
 // CreateAvailableMemoryWatcher() is not defined.
 #if !defined(XP_WIN)

xpcom/base/AvailableMemoryWatcher.h

@@ -21,6 +21,8 @@ class nsAvailableMemoryWatcherBase : public nsIAvailableMemoryWatcherBase {
   static StaticRefPtr<nsAvailableMemoryWatcherBase> sSingleton;
 
  protected:
+  nsCOMPtr<nsITabUnloader> mTabUnloader;
+
   virtual ~nsAvailableMemoryWatcherBase() = default;
 
  public:

xpcom/base/AvailableMemoryWatcherWin.cpp

@@ -251,17 +251,26 @@ void nsAvailableMemoryWatcher::OnLowMemory(const MutexAutoLock& aLock) {
 
   if (NS_IsMainThread()) {
     MaybeSaveMemoryReport(aLock);
+    {
+      // Don't invoke UnloadTabAsync() with the lock to avoid deadlock
+      // because nsAvailableMemoryWatcher::Notify may be invoked while
+      // running the method.
+      MutexAutoUnlock unlock(mMutex);
+      mTabUnloader->UnloadTabAsync();
+    }
   } else {
-    // SaveMemoryReport needs to be run in the main thread
+    // SaveMemoryReport and mTabUnloader needs to be run in the main thread
     // (See nsMemoryReporterManager::GetReportsForThisProcessExtended)
     NS_DispatchToMainThread(NS_NewRunnableFunction(
         "nsAvailableMemoryWatcher::OnLowMemory", [self = RefPtr{this}]() {
-          MutexAutoLock lock(self->mMutex);
-          self->MaybeSaveMemoryReport(lock);
+          {
+            MutexAutoLock lock(self->mMutex);
+            self->MaybeSaveMemoryReport(lock);
+          }
+          self->mTabUnloader->UnloadTabAsync();
         }));
   }
 
-  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
   StartPollingIfUserInteracting(aLock);
 }
 

xpcom/base/nsIAvailableMemoryWatcherBase.idl

@@ -6,12 +6,23 @@
 #include "nsISupports.idl"
 
 /**
+ * nsITabUnloader: interface to represent TabUnloader
+ *
  * nsIAvailableMemoryWatcherBase: interface to watch the system's memory
- * status and send a memory-pressure event.
- * The logic to detect such a memory situation is defined per platform.
+ * status and invoke a registered TabUnloader when it detected a low-memory
+ * and high-memory situation.  The logic to detect such a memory situation
+ * is defined per platform.
  */
 
+[scriptable, uuid(2e530956-6054-464f-9f4c-0ae6f8de5523)]
+interface nsITabUnloader : nsISupports
+{
+  void unloadTabAsync();
+};
+
 [scriptable, uuid(b0b5701e-239d-49db-9009-37e89f86441c)]
 interface nsIAvailableMemoryWatcherBase : nsISupports
 {
+  void registerTabUnloader(in nsITabUnloader aTabUnloader);
+  void onUnloadAttemptCompleted(in nsresult aResult);
 };