Bug 1361259. P1 - let ListenerBase inherit RevocableToken. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 28 Apr 2017 14:18:38 +0800
changeset 571199 f87215a2c7fc27140dad879561ec18a735639229
parent 571131 2e7c10a9b86e30691f67855f6c8f98d984508d7c
child 571200 05c792557e9c01740f6c95b2313ad4336e5b7d5f
push id56725
push userjwwang@mozilla.com
push dateTue, 02 May 2017 08:46:33 +0000
bugs1361259
milestone55.0a1
Bug 1361259. P1 - let ListenerBase inherit RevocableToken. This is needed by P2 where |Listener| must be ref-counted so we can use NewRunnableMethod() to pass event data to the listener function. MozReview-Commit-ID: CpAgOmxcijc
dom/media/MediaEventSource.h
--- a/dom/media/MediaEventSource.h
+++ b/dom/media/MediaEventSource.h
@@ -8,17 +8,16 @@
 #define MediaEventSource_h_
 
 #include "mozilla/AbstractThread.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/IndexSequence.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/Tuple.h"
 #include "mozilla/TypeTraits.h"
-#include "mozilla/UniquePtr.h"
 
 #include "nsISupportsImpl.h"
 #include "nsTArray.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 
 /**
@@ -39,18 +38,22 @@ public:
   void Revoke() {
     mRevoked = true;
   }
 
   bool IsRevoked() const {
     return mRevoked;
   }
 
+protected:
+  // Virtual destructor is required since we might delete a Listener object
+  // through its base type pointer.
+  virtual ~RevocableToken() { }
+
 private:
-  ~RevocableToken() {}
   Atomic<bool> mRevoked;
 };
 
 enum class ListenerPolicy : int8_t {
   // Allow at most one listener. Move will be used when possible
   // to pass the event data to save copy.
   Exclusive,
   // Allow multiple listeners. Event data will always be copied when passed
@@ -208,70 +211,66 @@ private:
  * @Copy Data will always be copied. Each listener gets a copy.
  * @Move Data will always be moved.
  */
 enum class EventPassMode : int8_t {
   Copy,
   Move
 };
 
-class ListenerBase {
-public:
-  ListenerBase() : mToken(new RevocableToken()) {}
-  ~ListenerBase() {
-    MOZ_ASSERT(Token()->IsRevoked(), "Must disconnect the listener.");
+class ListenerBase : public RevocableToken
+{
+protected:
+  virtual ~ListenerBase()
+  {
+    MOZ_ASSERT(IsRevoked(), "Must disconnect the listener.");
   }
-  RevocableToken* Token() const {
-    return mToken;
-  }
-private:
-  const RefPtr<RevocableToken> mToken;
 };
 
 /**
  * Stored by MediaEventSource to send notifications to the listener.
  * Since virtual methods can not be templated, this class is specialized
  * to provide different Dispatch() overloads depending on EventPassMode.
  */
 template <EventPassMode Mode, typename... As>
-class Listener : public ListenerBase {
+class Listener : public ListenerBase
+{
 public:
-  virtual ~Listener() {}
   virtual void Dispatch(const As&... aEvents) = 0;
 };
 
 template <typename... As>
-class Listener<EventPassMode::Move, As...> : public ListenerBase {
+class Listener<EventPassMode::Move, As...> : public ListenerBase
+{
 public:
-  virtual ~Listener() {}
   virtual void Dispatch(As... aEvents) = 0;
 };
 
 /**
  * Store the registered target thread and function so it knows where and to
  * whom to send the event data.
  */
 template <typename Target, typename Function, EventPassMode, typename... As>
 class ListenerImpl : public Listener<EventPassMode::Copy, As...> {
 public:
   ListenerImpl(Target* aTarget, const Function& aFunction)
-    : mHelper(ListenerBase::Token(), aTarget, aFunction) {}
+    : mHelper(this, aTarget, aFunction) {}
   void Dispatch(const As&... aEvents) override {
     mHelper.Dispatch(aEvents...);
   }
 private:
   ListenerHelper<Target, Function> mHelper;
 };
 
 template <typename Target, typename Function, typename... As>
 class ListenerImpl<Target, Function, EventPassMode::Move, As...>
   : public Listener<EventPassMode::Move, As...> {
 public:
   ListenerImpl(Target* aTarget, const Function& aFunction)
-    : mHelper(ListenerBase::Token(), aTarget, aFunction) {}
+    : mHelper(this, aTarget, aFunction) {}
   void Dispatch(As... aEvents) override {
     mHelper.Dispatch(Move(aEvents)...);
   }
 private:
   ListenerHelper<Target, Function> mHelper;
 };
 
 /**
@@ -373,31 +372,31 @@ class MediaEventSourceImpl {
     detail::ListenerImpl<Target, Func, PassMode, ArgType<Es>...>;
 
   template <typename Method>
   using TakeArgs = detail::TakeArgs<Method>;
 
   void PruneListeners() {
     int32_t last = static_cast<int32_t>(mListeners.Length()) - 1;
     for (int32_t i = last; i >= 0; --i) {
-      if (mListeners[i]->Token()->IsRevoked()) {
+      if (mListeners[i]->IsRevoked()) {
         mListeners.RemoveElementAt(i);
       }
     }
   }
 
   template<typename Target, typename Function>
   MediaEventListener
   ConnectInternal(Target* aTarget, const Function& aFunction) {
     MutexAutoLock lock(mMutex);
     PruneListeners();
     MOZ_ASSERT(Lp == ListenerPolicy::NonExclusive || mListeners.IsEmpty());
     auto l = mListeners.AppendElement();
-    l->reset(new ListenerImpl<Target, Function>(aTarget, aFunction));
-    return MediaEventListener((*l)->Token());
+    *l = new ListenerImpl<Target, Function>(aTarget, aFunction);
+    return MediaEventListener(*l);
   }
 
   // |Method| takes one or more arguments.
   template <typename Target, typename This, typename Method>
   typename EnableIf<TakeArgs<Method>::value, MediaEventListener>::Type
   ConnectInternal(Target* aTarget, This* aThis, Method aMethod) {
     detail::RawPtr<This> thiz(aThis);
     auto f = [=] (ArgType<Es>&&... aEvents) {
@@ -467,27 +466,27 @@ protected:
   template <typename... Ts>
   void NotifyInternal(Ts&&... aEvents) {
     MutexAutoLock lock(mMutex);
     int32_t last = static_cast<int32_t>(mListeners.Length()) - 1;
     for (int32_t i = last; i >= 0; --i) {
       auto&& l = mListeners[i];
       // Remove disconnected listeners.
       // It is not optimal but is simple and works well.
-      if (l->Token()->IsRevoked()) {
+      if (l->IsRevoked()) {
         mListeners.RemoveElementAt(i);
         continue;
       }
       l->Dispatch(Forward<Ts>(aEvents)...);
     }
   }
 
 private:
   Mutex mMutex;
-  nsTArray<UniquePtr<Listener>> mListeners;
+  nsTArray<RefPtr<Listener>> mListeners;
 };
 
 template <typename... Es>
 using MediaEventSource =
   MediaEventSourceImpl<ListenerPolicy::NonExclusive, Es...>;
 
 template <typename... Es>
 using MediaEventSourceExc =