Bug 1361461 - Dispatch the compartment-nuking part of WindowDestroyedEvent to the idle queue; r=smaug

devtools/client/webconsole/test/browser_console_dead_objects.js

@@ -52,7 +52,7 @@ function test() {
 
     gBrowser.removeCurrentTab();
 
-    yield TestUtils.topicObserved("outer-window-destroyed", (subject, data) => {
+    yield TestUtils.topicObserved("outer-window-nuked", (subject, data) => {
       let id = subject.QueryInterface(Components.interfaces.nsISupportsPRUint64).data;
       return id == winID;
     });

dom/base/nsGlobalWindow.cpp

@@ -9628,16 +9628,24 @@ struct BrowserCompartmentMatcher : public js::CompartmentFilter {
 };
 
 
-class WindowDestroyedEvent : public Runnable
+class WindowDestroyedEvent final : public Runnable
 {
 public:
   WindowDestroyedEvent(nsIDOMWindow* aWindow, uint64_t aID,
                        const char* aTopic) :
-    mID(aID), mTopic(aTopic)
+    mID(aID),
+    mPhase(Phase::Destroying),
+    mTopic(aTopic)
   {
     mWindow = do_GetWeakReference(aWindow);
   }
 
+  enum class Phase
+  {
+    Destroying,
+    Nuking
+  };
+
   NS_IMETHOD Run() override
   {
     PROFILER_LABEL("WindowDestroyedEvent", "Run",
@@ -9645,51 +9653,81 @@ public:
 
     nsCOMPtr<nsIObserverService> observerService =
       services::GetObserverService();
-    if (observerService) {
-      nsCOMPtr<nsISupportsPRUint64> wrapper =
-        do_CreateInstance(NS_SUPPORTS_PRUINT64_CONTRACTID);
-      if (wrapper) {
-        wrapper->SetData(mID);
-        observerService->NotifyObservers(wrapper, mTopic.get(), nullptr);
-      }
+    if (!observerService) {
+      return NS_OK;
     }
 
-    bool skipNukeCrossCompartment = false;
+    nsCOMPtr<nsISupportsPRUint64> wrapper =
+      do_CreateInstance(NS_SUPPORTS_PRUINT64_CONTRACTID);
+    if (wrapper) {
+      wrapper->SetData(mID);
+      observerService->NotifyObservers(wrapper, mTopic.get(), nullptr);
+    }
+
+    switch (mPhase) {
+      case Phase::Destroying:
+      {
+        bool skipNukeCrossCompartment = false;
 #ifndef DEBUG
-    nsCOMPtr<nsIAppStartup> appStartup =
-      do_GetService(NS_APPSTARTUP_CONTRACTID);
+        nsCOMPtr<nsIAppStartup> appStartup =
+          do_GetService(NS_APPSTARTUP_CONTRACTID);
 
-    if (appStartup) {
-      appStartup->GetShuttingDown(&skipNukeCrossCompartment);
-    }
+        if (appStartup) {
+          appStartup->GetShuttingDown(&skipNukeCrossCompartment);
+        }
 #endif
 
-    nsCOMPtr<nsISupports> window = do_QueryReferent(mWindow);
-    if (!skipNukeCrossCompartment && window) {
-      nsGlobalWindow* win = nsGlobalWindow::FromSupports(window);
-      nsGlobalWindow* currentInner = win->IsInnerWindow() ? win : win->GetCurrentInnerWindowInternal();
-      NS_ENSURE_TRUE(currentInner, NS_OK);
+        if (!skipNukeCrossCompartment) {
+          // The compartment nuking phase might be too expensive, so do that
+          // part off of idle dispatch.
 
-      AutoSafeJSContext cx;
-      JS::Rooted<JSObject*> obj(cx, currentInner->FastGetGlobalJSObject());
-      if (obj && !js::IsSystemCompartment(js::GetObjectCompartment(obj))) {
-        JSCompartment* cpt = js::GetObjectCompartment(obj);
-        nsCOMPtr<nsIPrincipal> pc = nsJSPrincipals::get(JS_GetCompartmentPrincipals(cpt));
+          // For the compartment nuking phase, we dispatch either an
+          // inner-window-nuked or an outer-window-nuked notification.
+          // This will allow tests to wait for compartment nuking to happen.
+          if (mTopic.EqualsLiteral("inner-window-destroyed")) {
+            mTopic.AssignLiteral("inner-window-nuked");
+          } else if (mTopic.EqualsLiteral("outer-window-destroyed")) {
+            mTopic.AssignLiteral("outer-window-nuked");
+          }
+          mPhase = Phase::Nuking;
 
-        nsAutoString addonId;
-        if (NS_SUCCEEDED(pc->GetAddonId(addonId)) && !addonId.IsEmpty()) {
-          // We want to nuke all references to the add-on compartment.
-          xpc::NukeAllWrappersForCompartment(cx, cpt,
-                                             win->IsInnerWindow() ? js::DontNukeWindowReferences
-                                                                  : js::NukeWindowReferences);
-        } else {
-          // We only want to nuke wrappers for the chrome->content case
-          js::NukeCrossCompartmentWrappers(cx, BrowserCompartmentMatcher(), cpt,
-                                           win->IsInnerWindow() ? js::DontNukeWindowReferences
-                                                                : js::NukeWindowReferences,
-                                           js::NukeIncomingReferences);
+          nsCOMPtr<nsIRunnable> copy(this);
+          NS_IdleDispatchToCurrentThread(copy.forget());
         }
       }
+      break;
+
+      case Phase::Nuking:
+      {
+        nsCOMPtr<nsISupports> window = do_QueryReferent(mWindow);
+        if (window) {
+          nsGlobalWindow* win = nsGlobalWindow::FromSupports(window);
+          nsGlobalWindow* currentInner = win->IsInnerWindow() ? win : win->GetCurrentInnerWindowInternal();
+          NS_ENSURE_TRUE(currentInner, NS_OK);
+
+          AutoSafeJSContext cx;
+          JS::Rooted<JSObject*> obj(cx, currentInner->FastGetGlobalJSObject());
+          if (obj && !js::IsSystemCompartment(js::GetObjectCompartment(obj))) {
+            JSCompartment* cpt = js::GetObjectCompartment(obj);
+            nsCOMPtr<nsIPrincipal> pc = nsJSPrincipals::get(JS_GetCompartmentPrincipals(cpt));
+
+            nsAutoString addonId;
+            if (NS_SUCCEEDED(pc->GetAddonId(addonId)) && !addonId.IsEmpty()) {
+              // We want to nuke all references to the add-on compartment.
+              xpc::NukeAllWrappersForCompartment(cx, cpt,
+                                                 win->IsInnerWindow() ? js::DontNukeWindowReferences
+                                                                      : js::NukeWindowReferences);
+            } else {
+              // We only want to nuke wrappers for the chrome->content case
+              js::NukeCrossCompartmentWrappers(cx, BrowserCompartmentMatcher(), cpt,
+                                               win->IsInnerWindow() ? js::DontNukeWindowReferences
+                                                                    : js::NukeWindowReferences,
+                                               js::NukeIncomingReferences);
+            }
+          }
+        }
+      }
+      break;
     }
 
     return NS_OK;
@@ -9697,6 +9735,7 @@ public:
 
 private:
   uint64_t mID;
+  Phase mPhase;
   nsCString mTopic;
   nsWeakPtr mWindow;
 };

js/xpconnect/tests/browser/browser_dead_object.js

@@ -13,7 +13,7 @@ add_task(function* test() {
   let contentDocDead = yield ContentTask.spawn(browser,{innerWindowId}, function*(args){
     let doc = content.document;
     let {TestUtils} = Components.utils.import("resource://testing-common/TestUtils.jsm", {});
-    let promise = TestUtils.topicObserved("inner-window-destroyed", (subject, data) => {
+    let promise = TestUtils.topicObserved("inner-window-nuked", (subject, data) => {
       let id = subject.QueryInterface(Components.interfaces.nsISupportsPRUint64).data;
       return id == args.innerWindowId;
     });

js/xpconnect/tests/chrome/test_windowProxyDeadWrapper.html

@@ -49,7 +49,7 @@ function go() {
         // once the window is destroyed.
         frame.remove();
 
-        TestUtils.topicObserved("outer-window-destroyed", (subject, data) => {
+        TestUtils.topicObserved("outer-window-nuked", (subject, data) => {
             let id = subject.QueryInterface(SpecialPowers.Ci.nsISupportsPRUint64).data;
             return id == winID;
         }).then(() => {

js/xpconnect/tests/mochitest/test_nukeContentWindow.html

@@ -25,11 +25,11 @@ function waitForWindowDestroyed(winID, callback) {
       if (id != winID) {
         return;
       }
-      SpecialPowers.removeObserver(observer, "outer-window-destroyed");
+      SpecialPowers.removeObserver(observer, "outer-window-nuked");
       SpecialPowers.executeSoon(callback);
     }
   };
-  SpecialPowers.addObserver(observer, "outer-window-destroyed");
+  SpecialPowers.addObserver(observer, "outer-window-nuked");
 }
 
 add_task(function* () {

js/xpconnect/tests/unit/test_nuke_webextension_wrappers.js

@@ -61,8 +61,8 @@ add_task(function*() {
 
   webnavB.close();
 
-  // Wrappers are destroyed asynchronously, so wait for that to happen.
-  yield TestUtils.topicObserved("inner-window-destroyed");
+  // Wrappers are nuked asynchronously, so wait for that to happen.
+  yield TestUtils.topicObserved("inner-window-nuked");
 
   // Check that it can't be accessed after he window has been closed.
   let result = getThing();