Bug 1332095 - Never set mLastWindowLeft on the Chrome TabGroup, r=billm
authorMichael Layzell <michael@thelayzells.com>
Wed, 18 Jan 2017 17:25:07 -0500
changeset 330240 0fdc7261d01752d1a6d45c16f67078eb5903658c
parent 330239 d6c6e2dae5f431fba8e51518a4b61c5808d74ed1
child 330241 3ed3535bf8edfa0f43b330107daa026ab245578c
push id31233
push userphilringnalda@gmail.com
push dateFri, 20 Jan 2017 06:05:15 +0000
treeherdermozilla-central@aa3e49299a3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1332095
milestone53.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 1332095 - Never set mLastWindowLeft on the Chrome TabGroup, r=billm MozReview-Commit-ID: K2jLCuTUTSd
dom/base/TabGroup.cpp
--- a/dom/base/TabGroup.cpp
+++ b/dom/base/TabGroup.cpp
@@ -23,17 +23,24 @@ namespace dom {
 static StaticRefPtr<TabGroup> sChromeTabGroup;
 
 TabGroup::TabGroup(bool aIsChrome)
  : mLastWindowLeft(false)
  , mThrottledQueuesInitialized(false)
 {
   for (size_t i = 0; i < size_t(TaskCategory::Count); i++) {
     TaskCategory category = static_cast<TaskCategory>(i);
-    mEventTargets[i] = CreateEventTargetFor(category);
+    if (aIsChrome) {
+      // The chrome TabGroup dispatches directly to the main thread. This means
+      // that we don't have to worry about cyclical references when cleaning up
+      // the chrome TabGroup.
+      mEventTargets[i] = do_GetMainThread();
+    } else {
+      mEventTargets[i] = CreateEventTargetFor(category);
+    }
   }
 
   // Do not throttle runnables from chrome windows.  In theory we should
   // not have abuse issues from these windows and many browser chrome
   // tests have races that fail if we do throttle chrome runnables.
   if (aIsChrome) {
     MOZ_ASSERT(!sChromeTabGroup);
     return;
@@ -71,17 +78,17 @@ TabGroup::EnsureThrottledEventQueues()
         // This may return nullptr during xpcom shutdown.  This is ok as we
         // do not guarantee a ThrottledEventQueue will be present.
         mEventTargets[i] = target;
       }
     }
   }
 }
 
-TabGroup*
+/* static */ TabGroup*
 TabGroup::GetChromeTabGroup()
 {
   if (!sChromeTabGroup) {
     sChromeTabGroup = new TabGroup(true /* chrome tab group */);
     ClearOnShutdown(&sChromeTabGroup);
   }
   return sChromeTabGroup;
 }
@@ -157,17 +164,20 @@ TabGroup::Join(nsPIDOMWindowOuter* aWind
 }
 
 void
 TabGroup::Leave(nsPIDOMWindowOuter* aWindow)
 {
   MOZ_ASSERT(mWindows.Contains(aWindow));
   mWindows.RemoveElement(aWindow);
 
-  if (mWindows.IsEmpty()) {
+  // The Chrome TabGroup doesn't have cyclical references through mEventTargets
+  // to itself, meaning that we don't have to worry about nulling mEventTargets
+  // out after the last window leaves.
+  if (sChromeTabGroup != this && mWindows.IsEmpty()) {
     mLastWindowLeft = true;
 
     // There is a RefPtr cycle TabGroup -> DispatcherEventTarget -> TabGroup. To
     // avoid leaks, we need to break the chain somewhere. We shouldn't be using
     // the ThrottledEventQueue for this TabGroup when no windows belong to it,
     // so it's safe to null out the queue here.
     for (size_t i = 0; i < size_t(TaskCategory::Count); i++) {
       mEventTargets[i] = nullptr;