Backed out changeset 575e351a12af (bug 1361461) for causing frequent reftest assertion failures like Assertion failure: false (Ran out of memory while building cycle collector graph), at z:/build/build/src/xpcom/base/nsCycleCollector.cpp:929
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Thu, 08 Jun 2017 11:27:43 +0200
changeset 413423 82bdfe54a6cd08087b41e971c282a903355ba5ab
parent 413422 1b675aa9a437be904cdf145a6727b9addd161de2
child 413424 f46c3d67b5eb210dd769c68b9e3d34841a756824
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1361461
milestone55.0a1
backs out575e351a12af55a98990dd88c81bf20fbf983977
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
Backed out changeset 575e351a12af (bug 1361461) for causing frequent reftest assertion failures like Assertion failure: false (Ran out of memory while building cycle collector graph), at z:/build/build/src/xpcom/base/nsCycleCollector.cpp:929
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-nuked", (subject, data) => {
+    yield TestUtils.topicObserved("outer-window-destroyed", (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,124 +9623,85 @@ struct BrowserCompartmentMatcher : publi
   bool match(JSCompartment* aC) const override
   {
     nsCOMPtr<nsIPrincipal> pc = nsJSPrincipals::get(JS_GetCompartmentPrincipals(aC));
     return nsContentUtils::IsSystemOrExpandedPrincipal(pc);
   }
 };
 
 
-class WindowDestroyedEvent final : public Runnable
+class WindowDestroyedEvent : public Runnable
 {
 public:
   WindowDestroyedEvent(nsIDOMWindow* aWindow, uint64_t aID,
                        const char* aTopic) :
-    mID(aID),
-    mPhase(Phase::Destroying),
-    mTopic(aTopic)
+    mID(aID), 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) {
-      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;
+    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;
 #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
 
-        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);
+    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);
         }
       }
-      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-nuked", (subject, data) => {
+    let promise = TestUtils.topicObserved("inner-window-destroyed", (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-nuked", (subject, data) => {
+        TestUtils.topicObserved("outer-window-destroyed", (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-nuked");
+      SpecialPowers.removeObserver(observer, "outer-window-destroyed");
       SpecialPowers.executeSoon(callback);
     }
   };
-  SpecialPowers.addObserver(observer, "outer-window-nuked");
+  SpecialPowers.addObserver(observer, "outer-window-destroyed");
 }
 
 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 nuked asynchronously, so wait for that to happen.
-  yield TestUtils.topicObserved("inner-window-nuked");
+  // Wrappers are destroyed asynchronously, so wait for that to happen.
+  yield TestUtils.topicObserved("inner-window-destroyed");
 
   // 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();