Bug 1361305. P1 - use forwarding reference for the ListenerImpl constructor to enable move whenever possible. draft
authorJW Wang <jwwang@mozilla.com>
Tue, 02 May 2017 17:11:51 +0800
changeset 571674 323e0a6d9715d1b1b563a99784380db55e333013
parent 571650 d63e7fe3008aee5282bae7b5027319138d7f8ac3
child 571675 49428e3d7b43ed84267ed2a9d62ed924764054e5
push id56887
push userjwwang@mozilla.com
push dateWed, 03 May 2017 04:46:55 +0000
bugs1361305
milestone55.0a1
Bug 1361305. P1 - use forwarding reference for the ListenerImpl constructor to enable move whenever possible. MozReview-Commit-ID: AFsbYtx0xMR
dom/media/MediaEventSource.h
--- a/dom/media/MediaEventSource.h
+++ b/dom/media/MediaEventSource.h
@@ -162,32 +162,36 @@ private:
 
 /**
  * Store the registered target thread and function so it knows where and to
  * whom to send the event data.
  */
 template <typename Target, typename Function, typename... As>
 class ListenerImpl : public Listener<As...>
 {
+  // Strip CV and reference from Function.
+  using FunctionStorage = typename Decay<Function>::Type;
+
 public:
-  ListenerImpl(Target* aTarget, const Function& aFunction)
+  template <typename F>
+  ListenerImpl(Target* aTarget, F&& aFunction)
     : mTarget(aTarget)
-    , mFunction(aFunction)
+    , mFunction(Forward<F>(aFunction))
   {
   }
 
 private:
   void DispatchTask(already_AddRefed<nsIRunnable> aTask) override
   {
     EventTarget<Target>::Dispatch(mTarget.get(), Move(aTask));
   }
 
   bool CanTakeArgs() const override
   {
-    return TakeArgs<Function>::value;
+    return TakeArgs<FunctionStorage>::value;
   }
 
   // |F| takes one or more arguments.
   template <typename F>
   typename EnableIf<TakeArgs<F>::value, void>::Type
   ApplyWithArgsImpl(const F& aFunc, As&&... aEvents)
   {
     aFunc(Move(aEvents)...);
@@ -231,17 +235,17 @@ private:
     MOZ_RELEASE_ASSERT(!TakeArgs<Function>::value);
     // Don't call the listener if it is disconnected.
     if (!RevocableToken::IsRevoked()) {
       ApplyWithNoArgsImpl(mFunction);
     }
   }
 
   const RefPtr<Target> mTarget;
-  Function mFunction;
+  FunctionStorage mFunction;
 };
 
 /**
  * Return true if any type is a reference type.
  */
 template <typename Head, typename... Tails>
 struct IsAnyReference {
   static const bool value = IsReference<Head>::value ||
@@ -326,66 +330,67 @@ class MediaEventSourceImpl {
       if (mListeners[i]->IsRevoked()) {
         mListeners.RemoveElementAt(i);
       }
     }
   }
 
   template<typename Target, typename Function>
   MediaEventListener
-  ConnectInternal(Target* aTarget, const Function& aFunction) {
+  ConnectInternal(Target* aTarget, Function&& aFunction) {
     MutexAutoLock lock(mMutex);
     PruneListeners();
     MOZ_ASSERT(Lp == ListenerPolicy::NonExclusive || mListeners.IsEmpty());
     auto l = mListeners.AppendElement();
-    *l = new ListenerImpl<Target, Function>(aTarget, aFunction);
+    *l = new ListenerImpl<Target, Function>(
+      aTarget, Forward<Function>(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) {
-      (thiz.get()->*aMethod)(Move(aEvents)...);
-    };
-    return ConnectInternal(aTarget, f);
+    return ConnectInternal(aTarget,
+      [=](ArgType<Es>&&... aEvents) {
+        (thiz.get()->*aMethod)(Move(aEvents)...);
+      });
   }
 
   // |Method| takes no arguments. Don't bother passing the event data.
   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 = [=] () {
-      (thiz.get()->*aMethod)();
-    };
-    return ConnectInternal(aTarget, f);
+    return ConnectInternal(aTarget,
+      [=]() {
+        (thiz.get()->*aMethod)();
+      });
   }
 
 public:
   /**
    * Register a function to receive notifications from the event source.
    *
    * @param aTarget The target thread on which the function will run.
    * @param aFunction A function to be called on the target thread. The function
    *                  parameter must be convertible from |EventType|.
    * @return An object used to disconnect from the event source.
    */
   template<typename Function>
   MediaEventListener
-  Connect(AbstractThread* aTarget, const Function& aFunction) {
-    return ConnectInternal(aTarget, aFunction);
+  Connect(AbstractThread* aTarget, Function&& aFunction) {
+    return ConnectInternal(aTarget, Forward<Function>(aFunction));
   }
 
   template<typename Function>
   MediaEventListener
-  Connect(nsIEventTarget* aTarget, const Function& aFunction) {
-    return ConnectInternal(aTarget, aFunction);
+  Connect(nsIEventTarget* aTarget, Function&& aFunction) {
+    return ConnectInternal(aTarget, Forward<Function>(aFunction));
   }
 
   /**
    * As above.
    *
    * Note we deliberately keep a weak reference to |aThis| in order not to
    * change its lifetime. This is because notifications are dispatched
    * asynchronously and removing a listener doesn't always break the reference