Bug 1318506 - Assign TabGroup to runnable for TabGroup's throttled event queue (r?bkelly) draft
authorBill McCloskey <billm@mozilla.com>
Wed, 09 Nov 2016 20:38:01 -0800
changeset 441470 d32019e32470fc6e9c31eec59eeda30658c17556
parent 441469 0e92ad2ecc53da8a62a6b7cd17ba51e989ff7f73
child 441471 52e17e06c1fa2c60adc20cb75670dbc86404f5b2
push id36424
push userbmo:wmccloskey@mozilla.com
push dateFri, 18 Nov 2016 22:04:48 +0000
reviewersbkelly
bugs1318506
milestone53.0a1
Bug 1318506 - Assign TabGroup to runnable for TabGroup's throttled event queue (r?bkelly) This patch ensures that all the runnables that go in a TabGroup's ThrottledEventQueue get labeled with the TabGroup. It's hard to give a proper name/category to the stuff in the queue, but at least we'll have the right TabGroup. Eventually it would be nice to be able to pass through a name/category from the ThrottledEventQueue. MozReview-Commit-ID: 51PbDOrxa6d
dom/base/TabGroup.cpp
--- a/dom/base/TabGroup.cpp
+++ b/dom/base/TabGroup.cpp
@@ -48,23 +48,21 @@ TabGroup::~TabGroup()
 
 void
 TabGroup::EnsureThrottledEventQueue()
 {
   if (mThrottledEventQueue) {
     return;
   }
 
-  nsCOMPtr<nsIThread> mainThread;
-  NS_GetMainThread(getter_AddRefs(mainThread));
-  MOZ_DIAGNOSTIC_ASSERT(mainThread);
+  nsCOMPtr<nsIEventTarget> target = CreateEventTarget("ThrottledQueue", TaskCategory::Other);
 
   // This may return nullptr during xpcom shutdown.  This is ok as we
   // do not guarantee a ThrottledEventQueue will be present.
-  mThrottledEventQueue = ThrottledEventQueue::Create(mainThread);
+  mThrottledEventQueue = ThrottledEventQueue::Create(target);
 }
 
 TabGroup*
 TabGroup::GetChromeTabGroup()
 {
   if (!sChromeTabGroup) {
     sChromeTabGroup = new TabGroup(true /* chrome tab group */);
     ClearOnShutdown(&sChromeTabGroup);
@@ -133,24 +131,37 @@ TabGroup::AddDocument(const nsACString& 
 TabGroup::Join(nsPIDOMWindowOuter* aWindow, TabGroup* aTabGroup)
 {
   RefPtr<TabGroup> tabGroup = aTabGroup;
   if (!tabGroup) {
     tabGroup = new TabGroup();
   }
   MOZ_ASSERT(!tabGroup->mWindows.Contains(aWindow));
   tabGroup->mWindows.AppendElement(aWindow);
+
+  // Assert we have a throttled event queue. We null it out in Leave, but we
+  // should never have a Join after the last window Leaves.
+  MOZ_ASSERT(tabGroup->mThrottledEventQueue);
   return tabGroup.forget();
 }
 
 void
 TabGroup::Leave(nsPIDOMWindowOuter* aWindow)
 {
   MOZ_ASSERT(mWindows.Contains(aWindow));
   mWindows.RemoveElement(aWindow);
+
+  if (mWindows.IsEmpty()) {
+    // There is a RefPtr cycle TabGroup -> ThrottledEventQueue ->
+    // 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.
+    mThrottledEventQueue = nullptr;
+  }
 }
 
 nsresult
 TabGroup::FindItemWithName(const nsAString& aName,
                            nsIDocShellTreeItem* aRequestor,
                            nsIDocShellTreeItem* aOriginalRequestor,
                            nsIDocShellTreeItem** aFoundItem)
 {