From d8100b452d34e7c3cf81fe065cf91b8f302a8be9 Mon Sep 17 00:00:00 2001
From: Jens Stutte <jstutte@mozilla.com>
Date: Sat, 19 Nov 2022 10:15:32 +0000
Subject: [PATCH] Bug 1768581 - Part 12: Swap the order of MaybeFastShutdown
 and KillClearOnShutdown inside AdvanceShutdownPhase and add extra
 NS_ProcessPendingEvents for the main thread. r=xpcom-reviewers,nika

This change will effectively anticipate the fast shutdown by half a phase, making it more coherent with the phase it is defined to happen.

Depends on D160250

Differential Revision: https://phabricator.services.mozilla.com/D160628
---
 modules/libpref/init/StaticPrefList.yaml |  2 +-
 xpcom/base/AppShutdown.cpp               | 34 +++++++++++++-----------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
index 6e63c96d7a95..e603baaab4b1 100644
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -13580,7 +13580,7 @@
 #ifdef MOZ_CODE_COVERAGE
   value: 0
 #else
-  value: 3
+  value: 2
 #endif
   mirror: always
 
diff --git a/xpcom/base/AppShutdown.cpp b/xpcom/base/AppShutdown.cpp
index 123f83ee32a4..caf62befec18 100644
--- a/xpcom/base/AppShutdown.cpp
+++ b/xpcom/base/AppShutdown.cpp
@@ -350,14 +350,14 @@ void AppShutdown::AdvanceShutdownPhaseInternal(
 
   nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
   if (sCurrentShutdownPhase >= ShutdownPhase::AppShutdownConfirmed) {
-    // Give runnables dispatched so far as part of the ongoing phase a chance
-    // to run before actually advancing the phase. We can do this only after
-    // we passed the point of no return and thus can expect a linear flow
-    // through our shutdown phases. This way the processing is also covered
-    // by the terminator's timer.
-    // Note that this happens only for main thread runnables, such that the
-    // correct way of ensuring shutdown processing remains to have an async
-    // shutdown blocker.
+    // Give runnables dispatched between two calls to AdvanceShutdownPhase
+    // a chance to run before actually advancing the phase. We must do this
+    // only after we passed the point of no return and thus can expect a linear
+    // flow through our shutdown phases. This way the processing is also
+    // covered by the terminator's timer of the previous phase.
+    // Note that this affects only main thread runnables, such that the correct
+    // way of ensuring shutdown processing remains to have an async shutdown
+    // blocker.
     if (thread) {
       NS_ProcessPendingEvents(thread);
     }
@@ -366,22 +366,24 @@ void AppShutdown::AdvanceShutdownPhaseInternal(
   // From now on any IsInOrBeyond checks will find the new phase set.
   sCurrentShutdownPhase = aPhase;
 
-  // TODO: Bug 1768581
-  // We think it would be more logical to have the following order here:
-  //    AppShutdown::MaybeFastShutdown(aPhase);
-  //    sTerminator->AdvancePhase(aPhase);
-  //    obsService->NotifyObservers(...);
-  //    mozilla::KillClearOnShutdown(aPhase);
-
 #ifndef ANDROID
   if (sTerminator) {
     sTerminator->AdvancePhase(aPhase);
   }
 #endif
 
+  AppShutdown::MaybeFastShutdown(aPhase);
+
+  // This will null out the gathered pointers for this phase synchronously.
+  // Note that we keep the old order here to avoid breakage, so be aware that
+  // the notifications fired below will find these already cleared in case
+  // you expected the opposite.
   mozilla::KillClearOnShutdown(aPhase);
 
-  AppShutdown::MaybeFastShutdown(aPhase);
+  // Empty our MT event queue to process any side effects thereof.
+  if (thread) {
+    NS_ProcessPendingEvents(thread);
+  }
 
   if (doNotify) {
     const char* aTopic = AppShutdown::GetObserverKey(aPhase);