Bug 1361461 - Dispatch the compartment-nuking part of WindowDestroyedEvent to the idle queue; r=smaug
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 24 May 2017 16:08:08 -0400
changeset 362974 7ae0d4245ce74b3be7157bee494d85d64f278c0b
parent 362973 280dedc5b8c803a9c8ac42a90c38062123da12a7
child 362975 479b56b160d835e6d78055d4b05d50a883b69e41
push id31992
push userkwierso@gmail.com
push dateFri, 09 Jun 2017 01:11:44 +0000
treeherdermozilla-central@b42d50cafb15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1361461
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1361461 - Dispatch the compartment-nuking part of WindowDestroyedEvent to the idle queue; r=smaug
devtools/client/webconsole/test/browser_console_dead_objects.js
dom/base/nsGlobalWindow.cpp
js/xpconnect/tests/browser/browser_dead_object.js
js/xpconnect/tests/chrome/test_windowProxyDeadWrapper.html
js/xpconnect/tests/mochitest/test_nukeContentWindow.html
js/xpconnect/tests/unit/test_nuke_webextension_wrappers.js
--- a/devtools/client/webconsole/test/browser_console_dead_objects.js
+++ b/devtools/client/webconsole/test/browser_console_dead_objects.js
@@ -47,17 +47,17 @@ function test() {
                   "Cu.import('resource://gre/modules/Services.jsm');" +
                   "chromeWindow = Services.wm.getMostRecentWindow('" +
                   "navigator:browser');" +
                   "foobarzTezt = chromeWindow.content.document;" +
                   "delete chromeWindow");
 
     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;
     });
 
     let msg = yield jsterm.execute("foobarzTezt");
 
     isnot(hud.outputNode.textContent.indexOf("[object DeadObject]"), -1,
           "dead object found");
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -9623,85 +9623,124 @@ struct BrowserCompartmentMatcher : publi
   bool match(JSCompartment* aC) const override
   {
     nsCOMPtr<nsIPrincipal> pc = nsJSPrincipals::get(JS_GetCompartmentPrincipals(aC));
     return nsContentUtils::IsSystemOrExpandedPrincipal(pc);
   }
 };
 
 
-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",
                    js::ProfileEntry::Category::OTHER);
 
     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);
-      }
-    }
-
-    bool skipNukeCrossCompartment = false;
+    if (!observerService) {
+      return NS_OK;
+    }
+
+    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);
-
-    if (appStartup) {
-      appStartup->GetShuttingDown(&skipNukeCrossCompartment);
-    }
+        nsCOMPtr<nsIAppStartup> appStartup =
+          do_GetService(NS_APPSTARTUP_CONTRACTID);
+
+        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);
-
-      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);
+        if (!skipNukeCrossCompartment) {
+          // The compartment nuking phase might be too expensive, so do that
+          // part off of idle dispatch.
+
+          // 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;
+
+          nsCOMPtr<nsIRunnable> copy(this);
+          NS_IdleDispatchToCurrentThread(copy.forget(), 1000);
         }
       }
+      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;
   }
 
 private:
   uint64_t mID;
+  Phase mPhase;
   nsCString mTopic;
   nsWeakPtr mWindow;
 };
 
 void
 nsGlobalWindow::NotifyWindowIDDestroyed(const char* aTopic)
 {
   nsCOMPtr<nsIRunnable> runnable = new WindowDestroyedEvent(this, mWindowID, aTopic);
--- a/js/xpconnect/tests/browser/browser_dead_object.js
+++ b/js/xpconnect/tests/browser/browser_dead_object.js
@@ -8,17 +8,17 @@
 add_task(function* test() {
   const url = "http://mochi.test:8888/browser/js/xpconnect/tests/browser/browser_deadObjectOnUnload.html";
   let newTab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, url);
   let browser = gBrowser.selectedBrowser;
   let innerWindowId = browser.innerWindowID;
   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;
     });
     content.location = "about:home";
     yield promise;
     return Components.utils.isDeadWrapper(doc);
   });
   is(contentDocDead, true, "wrapper is dead");
--- a/js/xpconnect/tests/chrome/test_windowProxyDeadWrapper.html
+++ b/js/xpconnect/tests/chrome/test_windowProxyDeadWrapper.html
@@ -44,17 +44,17 @@ function go() {
         let winID = w.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
                      .getInterface(Components.interfaces.nsIDOMWindowUtils)
                      .outerWindowID;
         // Remove the frame. This will nuke the WindowProxy wrapper from our chrome
         // document's global, so evaluating 'this' in it will return a dead wrapper
         // 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(() => {
             ok(checkDead(), "Expected a dead object wrapper");
 
             // Wrapping the Window should throw now.
             var exc;
             try {
--- a/js/xpconnect/tests/mochitest/test_nukeContentWindow.html
+++ b/js/xpconnect/tests/mochitest/test_nukeContentWindow.html
@@ -20,21 +20,21 @@ https://bugzilla.mozilla.org/show_bug.cg
 
 function waitForWindowDestroyed(winID, callback) {
   let observer = {
     observe: function(subject, topic, data) {
       let id = subject.QueryInterface(SpecialPowers.Ci.nsISupportsPRUint64).data;
       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* () {
   let frame = $('subframe');
   frame.src = "data:text/html,";
   yield new Promise(resolve => frame.addEventListener("load", resolve, {once: true}));
 
   let win = frame.contentWindow;
--- a/js/xpconnect/tests/unit/test_nuke_webextension_wrappers.js
+++ b/js/xpconnect/tests/unit/test_nuke_webextension_wrappers.js
@@ -56,18 +56,18 @@ add_task(function*() {
     }
   }));
 
   // Check that the object can be accessed normally before windowB is closed.
   equal(getThing(), "bar");
 
   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();
   ok(/dead object/.test(result),
      `Result should show a dead wrapper error: ${result}`);
 
   webnavA.close();