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 440694 c43128ccf21b0e70874b076c91de27c5e7abba17
parent 440693 f23356f3f18a4f703ba8b83cb1a2d0d3b66fe8c7
child 440695 d3226ea42a55a13c550e6253b9032b7b401f700b
push id36297
push userbmo:wmccloskey@mozilla.com
push dateFri, 18 Nov 2016 00:53: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);
@@ -131,24 +129,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(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)
 {