Bug 1320753 - Refactor for new event target idea (r=ehsan)

MozReview-Commit-ID: AyhemNqBedb

dom/base/Dispatcher.cpp

@@ -6,6 +6,7 @@
 
 #include "mozilla/dom/Dispatcher.h"
 #include "mozilla/Move.h"
+#include "nsINamed.h"
 
 using namespace mozilla;
 
@@ -14,12 +15,21 @@ DispatcherTrait::Dispatch(const char* aName,
                           TaskCategory aCategory,
                           already_AddRefed<nsIRunnable>&& aRunnable)
 {
-  return NS_DispatchToMainThread(Move(aRunnable));
+  nsCOMPtr<nsIRunnable> runnable(aRunnable);
+  if (aName) {
+    if (nsCOMPtr<nsINamed> named = do_QueryInterface(runnable)) {
+      named->SetName(aName);
+    }
+  }
+  if (NS_IsMainThread()) {
+    return NS_DispatchToCurrentThread(runnable.forget());
+  } else {
+    return NS_DispatchToMainThread(runnable.forget());
+  }
 }
 
 already_AddRefed<nsIEventTarget>
-DispatcherTrait::CreateEventTarget(const char* aName,
-                                   TaskCategory aCategory)
+DispatcherTrait::EventTargetFor(TaskCategory aCategory) const
 {
   nsCOMPtr<nsIEventTarget> main = do_GetMainThread();
   return main.forget();
@@ -30,15 +40,11 @@ namespace {
 class DispatcherEventTarget final : public nsIEventTarget
 {
   RefPtr<dom::Dispatcher> mDispatcher;
-  const char* mName;
   TaskCategory mCategory;
 
 public:
-  DispatcherEventTarget(dom::Dispatcher* aDispatcher,
-                        const char* aName,
-                        TaskCategory aCategory)
+  DispatcherEventTarget(dom::Dispatcher* aDispatcher, TaskCategory aCategory)
    : mDispatcher(aDispatcher)
-   , mName(aName)
    , mCategory(aCategory)
   {}
 
@@ -67,7 +73,7 @@ DispatcherEventTarget::Dispatch(already_AddRefed<nsIRunnable> aRunnable, uint32_
   if (NS_WARN_IF(aFlags != NS_DISPATCH_NORMAL)) {
     return NS_ERROR_UNEXPECTED;
   }
-  return mDispatcher->Dispatch(mName, mCategory, Move(aRunnable));
+  return mDispatcher->Dispatch(nullptr, mCategory, Move(aRunnable));
 }
 
 NS_IMETHODIMP
@@ -84,9 +90,9 @@ DispatcherEventTarget::IsOnCurrentThread(bool* aIsOnCurrentThread)
 }
 
 already_AddRefed<nsIEventTarget>
-Dispatcher::CreateEventTarget(const char* aName, TaskCategory aCategory)
+Dispatcher::CreateEventTargetFor(TaskCategory aCategory)
 {
   RefPtr<DispatcherEventTarget> target =
-    new DispatcherEventTarget(this, aName, aCategory);
+    new DispatcherEventTarget(this, aCategory);
   return target.forget();
 }

dom/base/Dispatcher.h

@@ -34,6 +34,8 @@ enum class TaskCategory {
 
   // Most DOM events (postMessage, media, plugins)
   Other,
+
+  Count
 };
 
 // This trait should be attached to classes like nsIGlobalObject and nsIDocument
@@ -52,7 +54,7 @@ public:
   // it is safe. For nsIGlobalWindow it is not safe. The nsIEventTarget can
   // always be used off the main thread.
   virtual already_AddRefed<nsIEventTarget>
-  CreateEventTarget(const char* aName, TaskCategory aCategory);
+  EventTargetFor(TaskCategory aCategory) const;
 };
 
 // Base class for DocGroup and TabGroup.
@@ -66,7 +68,11 @@ public:
   // This method is always safe to call off the main thread. The nsIEventTarget
   // can always be used off the main thread.
   virtual already_AddRefed<nsIEventTarget>
-  CreateEventTarget(const char* aName, TaskCategory aCategory);
+  EventTargetFor(TaskCategory aCategory) const = 0;
+
+protected:
+  virtual already_AddRefed<nsIEventTarget>
+  CreateEventTargetFor(TaskCategory aCategory);
 };
 
 } // namespace dom

dom/base/DocGroup.cpp

@@ -60,7 +60,13 @@ DocGroup::Dispatch(const char* aName,
                    TaskCategory aCategory,
                    already_AddRefed<nsIRunnable>&& aRunnable)
 {
-  return NS_DispatchToMainThread(Move(aRunnable));
+  return mTabGroup->Dispatch(aName, aCategory, Move(aRunnable));
+}
+
+already_AddRefed<nsIEventTarget>
+DocGroup::EventTargetFor(TaskCategory aCategory) const
+{
+  return mTabGroup->EventTargetFor(aCategory);
 }
 
 }

dom/base/DocGroup.h

@@ -69,6 +69,9 @@ public:
                             TaskCategory aCategory,
                             already_AddRefed<nsIRunnable>&& aRunnable) override;
 
+  virtual already_AddRefed<nsIEventTarget>
+  EventTargetFor(TaskCategory aCategory) const override;
+
 private:
   DocGroup(TabGroup* aTabGroup, const nsACString& aKey);
   ~DocGroup();

dom/base/TabGroup.cpp

@@ -21,7 +21,13 @@ namespace dom {
 static StaticRefPtr<TabGroup> sChromeTabGroup;
 
 TabGroup::TabGroup(bool aIsChrome)
+ : mLastWindowLeft(false)
 {
+  for (size_t i = 0; i < size_t(TaskCategory::Count); i++) {
+    TaskCategory category = static_cast<TaskCategory>(i);
+    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.
@@ -99,6 +105,18 @@ TabGroup::Leave(nsPIDOMWindowOuter* aWindow)
 {
   MOZ_ASSERT(mWindows.Contains(aWindow));
   mWindows.RemoveElement(aWindow);
+
+  if (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;
+    }
+  }
 }
 
 nsresult
@@ -172,7 +190,25 @@ TabGroup::Dispatch(const char* aName,
                    TaskCategory aCategory,
                    already_AddRefed<nsIRunnable>&& aRunnable)
 {
-  return NS_DispatchToMainThread(Move(aRunnable));
+  nsCOMPtr<nsIRunnable> runnable(aRunnable);
+  if (aName) {
+    if (nsCOMPtr<nsINamed> named = do_QueryInterface(runnable)) {
+      named->SetName(aName);
+    }
+  }
+  if (NS_IsMainThread()) {
+    return NS_DispatchToCurrentThread(runnable.forget());
+  } else {
+    return NS_DispatchToMainThread(runnable.forget());
+  }
+}
+
+already_AddRefed<nsIEventTarget>
+TabGroup::EventTargetFor(TaskCategory aCategory) const
+{
+  MOZ_RELEASE_ASSERT(!mLastWindowLeft);
+  nsCOMPtr<nsIEventTarget> target = mEventTargets[size_t(aCategory)];
+  return target.forget();
 }
 
 }

dom/base/TabGroup.h

@@ -110,11 +110,16 @@ public:
                             TaskCategory aCategory,
                             already_AddRefed<nsIRunnable>&& aRunnable) override;
 
+  virtual already_AddRefed<nsIEventTarget>
+  EventTargetFor(TaskCategory aCategory) const override;
+
 private:
   ~TabGroup();
   DocGroupMap mDocGroups;
+  bool mLastWindowLeft;
   nsTArray<nsPIDOMWindowOuter*> mWindows;
   RefPtr<ThrottledEventQueue> mThrottledEventQueue;
+  nsCOMPtr<nsIEventTarget> mEventTargets[size_t(TaskCategory::Count)];
 };
 
 } // namespace dom

dom/base/nsDocument.cpp

@@ -2873,7 +2873,7 @@ nsDocument::SetPrincipal(nsIPrincipal *aNewPrincipal)
 }
 
 mozilla::dom::DocGroup*
-nsIDocument::GetDocGroup()
+nsIDocument::GetDocGroup() const
 {
 #ifdef DEBUG
   // Sanity check that we have an up-to-date and accurate docgroup
@@ -2901,12 +2901,12 @@ nsIDocument::Dispatch(const char* aName,
 }
 
 already_AddRefed<nsIEventTarget>
-nsIDocument::CreateEventTarget(const char* aName, TaskCategory aCategory)
+nsIDocument::EventTargetFor(TaskCategory aCategory) const
 {
   if (mDocGroup) {
-    return mDocGroup->CreateEventTarget(aName, aCategory);
+    return mDocGroup->EventTargetFor(aCategory);
   }
-  return DispatcherTrait::CreateEventTarget(aName, aCategory);
+  return DispatcherTrait::EventTargetFor(aCategory);
 }
 
 NS_IMETHODIMP

dom/base/nsGlobalWindow.cpp

@@ -14998,7 +14998,7 @@ nsPIDOMWindow<T>::TabGroup()
 
 template<typename T>
 mozilla::dom::DocGroup*
-nsPIDOMWindow<T>::GetDocGroup()
+nsPIDOMWindow<T>::GetDocGroup() const
 {
   nsIDocument* doc = GetExtantDoc();
   if (doc) {
@@ -15020,14 +15020,13 @@ nsGlobalWindow::Dispatch(const char* aName,
 }
 
 already_AddRefed<nsIEventTarget>
-nsGlobalWindow::CreateEventTarget(const char* aName,
-                                  TaskCategory aCategory)
+nsGlobalWindow::EventTargetFor(TaskCategory aCategory) const
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   if (GetDocGroup()) {
-    return GetDocGroup()->CreateEventTarget(aName, aCategory);
+    return GetDocGroup()->EventTargetFor(aCategory);
   }
-  return DispatcherTrait::CreateEventTarget(aName, aCategory);
+  return DispatcherTrait::EventTargetFor(aCategory);
 }
 
 nsGlobalWindow::TemporarilyDisableDialogs::TemporarilyDisableDialogs(

dom/base/nsGlobalWindow.h

@@ -1731,7 +1731,7 @@ public:
                             already_AddRefed<nsIRunnable>&& aRunnable) override;
 
   virtual already_AddRefed<nsIEventTarget>
-  CreateEventTarget(const char* aName, mozilla::dom::TaskCategory aCategory) override;
+  EventTargetFor(mozilla::dom::TaskCategory aCategory) const override;
 
 protected:
   // These members are only used on outer window objects. Make sure

dom/base/nsIDocument.h

@@ -2856,7 +2856,7 @@ public:
     return mHasScrollLinkedEffect;
   }
 
-  mozilla::dom::DocGroup* GetDocGroup();
+  mozilla::dom::DocGroup* GetDocGroup() const;
 
   virtual void AddIntersectionObserver(
     mozilla::dom::DOMIntersectionObserver* aObserver) = 0;
@@ -2873,8 +2873,7 @@ public:
                             already_AddRefed<nsIRunnable>&& aRunnable) override;
 
   virtual already_AddRefed<nsIEventTarget>
-  CreateEventTarget(const char* aName,
-                    mozilla::dom::TaskCategory aCategory) override;
+  EventTargetFor(mozilla::dom::TaskCategory aCategory) const override;
 
 protected:
   bool GetUseCounter(mozilla::UseCounter aUseCounter)

dom/base/nsPIDOMWindow.h

@@ -580,7 +580,7 @@ public:
 
   mozilla::dom::TabGroup* TabGroup();
 
-  mozilla::dom::DocGroup* GetDocGroup();
+  mozilla::dom::DocGroup* GetDocGroup() const;
 
   virtual mozilla::ThrottledEventQueue* GetThrottledEventQueue() = 0;