Bug 1701368 - Part3: Remove the MemPressure_Ongoing request. r=gsvelto

We had `NS_DispatchMemoryPressure` and `NS_DispatchEventualMemoryPressure`
to dispatch a memory-pressure event which took `MemPressure_New` and
`MemPressure_Ongoing` to translate into "low-memory" and "low-memory-ongoing"
message respectively.

With that model, we could end up sending a wrong message if somebody
called the API with `MemPressure_Ongoing` without sending `MemPressure_New`.
To avoid that, this patch removes `MemPressure_Ongoing` and makes
the API decide whether it should dispatch a "new" event or "ongoing" event.

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

widget/uikit/nsAppShell.mm

@@ -92,7 +92,7 @@ - (void)applicationWillResignActive:(UIApplication*)application {
 
 - (void)applicationDidReceiveMemoryWarning:(UIApplication*)application {
   ALOG("[AppShellDelegate applicationDidReceiveMemoryWarning:]");
-  NS_DispatchMemoryPressure(MemPressure_New);
+  NS_NotifyOfMemoryPressure(MemoryPressureState::LowMemory);
 }
 @end
 

xpcom/base/AvailableMemoryWatcherWin.cpp

@@ -247,7 +247,7 @@ void nsAvailableMemoryWatcher::OnLowMemory(const MutexAutoLock&) {
     }
 
     RecordLowMemoryEvent();
-    NS_DispatchEventualMemoryPressure(MemPressure_New);
+    NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
   }
 
   StartPollingIfUserInteracting();
@@ -256,7 +256,7 @@ void nsAvailableMemoryWatcher::OnLowMemory(const MutexAutoLock&) {
 void nsAvailableMemoryWatcher::OnHighMemory(const MutexAutoLock&) {
   mUnderMemoryPressure = false;
   mSavedReport = false;  // Will save a new report if memory gets low again
-  NS_DispatchEventualMemoryPressure(MemPressure_Stopping);
+  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::NoPressure);
   StopPolling();
   ListenForLowMemory();
 }
@@ -333,7 +333,7 @@ nsAvailableMemoryWatcher::Notify(nsITimer* aTimer) {
   }
 
   if (IsCommitSpaceLow()) {
-    NS_DispatchEventualMemoryPressure(MemPressure_Ongoing);
+    NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
   }
 
   return NS_OK;

xpcom/tests/gtest/TestMemoryPressure.cpp

@@ -0,0 +1,193 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include <thread>
+#include "gtest/gtest.h"
+
+#include "mozilla/Atomics.h"
+#include "mozilla/SpinEventLoopUntil.h"
+#include "nsMemoryPressure.h"
+#include "nsIObserver.h"
+#include "nsIObserverService.h"
+#include "nsServiceManagerUtils.h"
+#include "nsThreadUtils.h"
+
+using namespace mozilla;
+
+namespace {
+
+enum class MemoryPressureEventType {
+  LowMemory,
+  LowMemoryOngoing,
+  Stop,
+};
+
+class MemoryPressureObserver final : public nsIObserver {
+  nsCOMPtr<nsIObserverService> mObserverSvc;
+  Vector<MemoryPressureEventType> mEvents;
+
+  ~MemoryPressureObserver() {
+    EXPECT_TRUE(
+        NS_SUCCEEDED(mObserverSvc->RemoveObserver(this, kTopicMemoryPressure)));
+    EXPECT_TRUE(NS_SUCCEEDED(
+        mObserverSvc->RemoveObserver(this, kTopicMemoryPressureStop)));
+  }
+
+ public:
+  NS_DECL_ISUPPORTS
+
+  MemoryPressureObserver()
+      : mObserverSvc(do_GetService(NS_OBSERVERSERVICE_CONTRACTID)) {
+    EXPECT_TRUE(NS_SUCCEEDED(mObserverSvc->AddObserver(
+        this, kTopicMemoryPressure, /* ownsWeak */ false)));
+    EXPECT_TRUE(NS_SUCCEEDED(mObserverSvc->AddObserver(
+        this, kTopicMemoryPressureStop, /* ownsWeak */ false)));
+  }
+
+  NS_IMETHOD Observe(nsISupports* aSubject, const char* aTopic,
+                     const char16_t* aData) override {
+    Maybe<MemoryPressureEventType> event;
+    if (strcmp(aTopic, kTopicMemoryPressure) == 0) {
+      if (nsDependentString(aData) == kSubTopicLowMemoryNew) {
+        event = Some(MemoryPressureEventType::LowMemory);
+      } else if (nsDependentString(aData) == kSubTopicLowMemoryOngoing) {
+        event = Some(MemoryPressureEventType::LowMemoryOngoing);
+      } else {
+        fprintf(stderr, "Unexpected subtopic: %S\n",
+                reinterpret_cast<const wchar_t*>(aData));
+        EXPECT_TRUE(false);
+      }
+    } else if (strcmp(aTopic, kTopicMemoryPressureStop) == 0) {
+      event = Some(MemoryPressureEventType::Stop);
+    } else {
+      fprintf(stderr, "Unexpected topic: %s\n", aTopic);
+      EXPECT_TRUE(false);
+    }
+
+    if (event) {
+      Unused << mEvents.emplaceBack(event.value());
+    }
+    return NS_OK;
+  }
+
+  uint32_t GetCount() const { return mEvents.length(); }
+  void Reset() { mEvents.clear(); }
+  MemoryPressureEventType Top() const { return mEvents[0]; }
+
+  bool ValidateTransitions() const {
+    if (mEvents.length() == 0) {
+      return true;
+    }
+
+    for (size_t i = 1; i < mEvents.length(); ++i) {
+      MemoryPressureEventType eventFrom = mEvents[i - 1];
+      MemoryPressureEventType eventTo = mEvents[i];
+      if ((eventFrom == MemoryPressureEventType::LowMemory &&
+           eventTo == MemoryPressureEventType::LowMemoryOngoing) ||
+          (eventFrom == MemoryPressureEventType::LowMemoryOngoing &&
+           eventTo == MemoryPressureEventType::LowMemoryOngoing) ||
+          (eventFrom == MemoryPressureEventType::Stop &&
+           eventTo == MemoryPressureEventType::LowMemory) ||
+          (eventFrom == MemoryPressureEventType::LowMemoryOngoing &&
+           eventTo == MemoryPressureEventType::Stop) ||
+          (eventFrom == MemoryPressureEventType::LowMemory &&
+           eventTo == MemoryPressureEventType::Stop)) {
+        // Only these transitions are valid.
+        continue;
+      }
+
+      fprintf(stderr, "Invalid transition: %d -> %d\n", eventFrom, eventTo);
+      return false;
+    }
+    return true;
+  }
+};
+
+NS_IMPL_ISUPPORTS(MemoryPressureObserver, nsIObserver)
+
+template <MemoryPressureState State>
+void PressureSender(Atomic<bool>& aContinue) {
+  while (aContinue) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    NS_NotifyOfEventualMemoryPressure(State);
+  }
+}
+
+template <MemoryPressureState State>
+void PressureSenderQuick(Atomic<bool>& aContinue) {
+  while (aContinue) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    NS_NotifyOfMemoryPressure(State);
+  }
+}
+
+}  // anonymous namespace
+
+TEST(MemoryPressure, Singlethread)
+{
+  RefPtr observer(new MemoryPressureObserver);
+  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
+  SpinEventLoopUntil([&observer]() { return observer->GetCount() == 1; });
+  EXPECT_EQ(observer->Top(), MemoryPressureEventType::LowMemory);
+
+  observer->Reset();
+  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
+  SpinEventLoopUntil([&observer]() { return observer->GetCount() == 1; });
+  EXPECT_EQ(observer->Top(), MemoryPressureEventType::LowMemoryOngoing);
+
+  observer->Reset();
+  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::LowMemory);
+  SpinEventLoopUntil([&observer]() { return observer->GetCount() == 1; });
+  EXPECT_EQ(observer->Top(), MemoryPressureEventType::LowMemoryOngoing);
+
+  observer->Reset();
+  NS_NotifyOfEventualMemoryPressure(MemoryPressureState::NoPressure);
+  SpinEventLoopUntil([&observer]() { return observer->GetCount() == 1; });
+  EXPECT_EQ(observer->Top(), MemoryPressureEventType::Stop);
+}
+
+TEST(MemoryPressure, Multithread)
+{
+  // Start |kNumThreads| threads each for the following thread type:
+  //  - LowMemory via NS_NotifyOfEventualMemoryPressure
+  //  - LowMemory via NS_NotifyOfMemoryPressure
+  //  - LowMemoryOngoing via NS_NotifyOfEventualMemoryPressure
+  //  - LowMemoryOngoing via NS_NotifyOfMemoryPressure
+  // and keep them running until |kNumEventsToValidate| memory-pressure events
+  // are received.
+  constexpr int kNumThreads = 5;
+  constexpr int kNumEventsToValidate = 200;
+
+  Atomic<bool> shouldContinue(true);
+  Vector<std::thread> threads;
+  for (int i = 0; i < kNumThreads; ++i) {
+    Unused << threads.emplaceBack(
+        PressureSender<MemoryPressureState::LowMemory>,
+        std::ref(shouldContinue));
+    Unused << threads.emplaceBack(
+        PressureSender<MemoryPressureState::NoPressure>,
+        std::ref(shouldContinue));
+    Unused << threads.emplaceBack(
+        PressureSenderQuick<MemoryPressureState::LowMemory>,
+        std::ref(shouldContinue));
+    Unused << threads.emplaceBack(
+        PressureSenderQuick<MemoryPressureState::NoPressure>,
+        std::ref(shouldContinue));
+  }
+
+  RefPtr observer(new MemoryPressureObserver);
+
+  // We cannot sleep here because the main thread needs to keep running.
+  SpinEventLoopUntil(
+      [&observer]() { return observer->GetCount() >= kNumEventsToValidate; });
+
+  shouldContinue = false;
+  for (auto& thread : threads) {
+    thread.join();
+  }
+
+  EXPECT_TRUE(observer->ValidateTransitions());
+}

xpcom/tests/gtest/TestNonBlockingAsyncInputStream.cpp

@@ -3,6 +3,7 @@
 #include "mozilla/NonBlockingAsyncInputStream.h"
 #include "mozilla/SpinEventLoopUntil.h"
 #include "nsIAsyncInputStream.h"
+#include "nsIThread.h"
 #include "nsStreamUtils.h"
 #include "nsString.h"
 #include "nsStringStream.h"

xpcom/tests/gtest/TestThreads.cpp

@@ -9,10 +9,13 @@
 #include <stdlib.h>
 #include "nspr.h"
 #include "nsCOMPtr.h"
+#include "nsIThread.h"
 #include "nsXPCOM.h"
 #include "mozilla/Monitor.h"
 #include "gtest/gtest.h"
 
+using namespace mozilla;
+
 class nsRunner final : public Runnable {
   ~nsRunner() = default;
 

xpcom/tests/gtest/moz.build

@@ -27,6 +27,7 @@ UNIFIED_SOURCES += [
     "TestInputStreamLengthHelper.cpp",
     "TestLogCommandLineHandler.cpp",
     "TestLogging.cpp",
+    "TestMemoryPressure.cpp",
     "TestMoveString.cpp",
     "TestMozPromise.cpp",
     "TestMruCache.cpp",

xpcom/threads/nsMemoryPressure.cpp

@@ -12,38 +12,91 @@
 
 using namespace mozilla;
 
-static Atomic<int32_t, Relaxed> sMemoryPressurePending;
-static_assert(MemPressure_None == 0,
-              "Bad static initialization with the default constructor.");
+const char* const kTopicMemoryPressure = "memory-pressure";
+const char* const kTopicMemoryPressureStop = "memory-pressure-stop";
+const char16_t* const kSubTopicLowMemoryNew = u"low-memory";
+const char16_t* const kSubTopicLowMemoryOngoing = u"low-memory-ongoing";
 
-MemoryPressureState NS_GetPendingMemoryPressure() {
-  int32_t value = sMemoryPressurePending.exchange(MemPressure_None);
-  return MemoryPressureState(value);
-}
+// This is accessed from any thread through NS_NotifyOfEventualMemoryPressure
+static Atomic<MemoryPressureState, Relaxed> sMemoryPressurePending(
+    MemoryPressureState::NoPressure);
+
+void NS_NotifyOfEventualMemoryPressure(MemoryPressureState aState) {
+  MOZ_ASSERT(aState != MemoryPressureState::None);
 
-void NS_DispatchEventualMemoryPressure(MemoryPressureState aState) {
   /*
    * A new memory pressure event erases an ongoing (or stop of) memory pressure,
    * but an existing "new" memory pressure event takes precedence over a new
    * "ongoing" or "stop" memory pressure event.
    */
   switch (aState) {
-    case MemPressure_None:
-      sMemoryPressurePending = MemPressure_None;
+    case MemoryPressureState::None:
+    case MemoryPressureState::LowMemory:
+      sMemoryPressurePending = aState;
       break;
-    case MemPressure_New:
-      sMemoryPressurePending = MemPressure_New;
-      break;
-    case MemPressure_Ongoing:
-    case MemPressure_Stopping:
-      sMemoryPressurePending.compareExchange(MemPressure_None, aState);
+    case MemoryPressureState::NoPressure:
+      sMemoryPressurePending.compareExchange(MemoryPressureState::None, aState);
       break;
   }
 }
 
-nsresult NS_DispatchMemoryPressure(MemoryPressureState aState) {
-  NS_DispatchEventualMemoryPressure(aState);
+nsresult NS_NotifyOfMemoryPressure(MemoryPressureState aState) {
+  NS_NotifyOfEventualMemoryPressure(aState);
   nsCOMPtr<nsIRunnable> event =
       new Runnable("NS_DispatchEventualMemoryPressure");
   return NS_DispatchToMainThread(event);
 }
+
+void NS_DispatchMemoryPressure() {
+  MOZ_ASSERT(NS_IsMainThread());
+  static MemoryPressureState sMemoryPressureStatus =
+      MemoryPressureState::NoPressure;
+
+  MemoryPressureState mpPending =
+      sMemoryPressurePending.exchange(MemoryPressureState::None);
+  if (mpPending == MemoryPressureState::None) {
+    return;
+  }
+
+  nsCOMPtr<nsIObserverService> os = services::GetObserverService();
+  if (!os) {
+    NS_WARNING("Can't get observer service!");
+    return;
+  }
+
+  switch (mpPending) {
+    case MemoryPressureState::None:
+      MOZ_ASSERT_UNREACHABLE("Already handled this case above.");
+      break;
+    case MemoryPressureState::LowMemory:
+      switch (sMemoryPressureStatus) {
+        case MemoryPressureState::None:
+          MOZ_ASSERT_UNREACHABLE("The internal status should never be None.");
+          break;
+        case MemoryPressureState::NoPressure:
+          sMemoryPressureStatus = MemoryPressureState::LowMemory;
+          os->NotifyObservers(nullptr, kTopicMemoryPressure,
+                              kSubTopicLowMemoryNew);
+          break;
+        case MemoryPressureState::LowMemory:
+          os->NotifyObservers(nullptr, kTopicMemoryPressure,
+                              kSubTopicLowMemoryOngoing);
+          break;
+      }
+      break;
+    case MemoryPressureState::NoPressure:
+      switch (sMemoryPressureStatus) {
+        case MemoryPressureState::None:
+          MOZ_ASSERT_UNREACHABLE("The internal status should never be None.");
+          break;
+        case MemoryPressureState::NoPressure:
+          // Already no pressure.  Do nothing.
+          break;
+        case MemoryPressureState::LowMemory:
+          sMemoryPressureStatus = MemoryPressureState::NoPressure;
+          os->NotifyObservers(nullptr, kTopicMemoryPressureStop, nullptr);
+          break;
+      }
+      break;
+  }
+}