Backed out changesets d0eee19c83cd, 0c54ee53678f, 5b202008a431, 81291b4e6dc3, acbc0d7e21cd, a7ceb6162a63 (bug 1194112) for Linux x64 Hazard failure. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 18 Aug 2015 11:11:38 +0200
changeset 258145 2b568650c03a0927ea23eba30bb953fb6c80d587
parent 258144 bbe4babf533ce71b9046c4375216c01ad3f9d7fb
child 258146 acca05b8182e86e12b33c3359cae87d63c7d0c4b
push id63835
push userarchaeopteryx@coole-files.de
push dateTue, 18 Aug 2015 09:12:03 +0000
treeherdermozilla-inbound@2b568650c03a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1194112
milestone43.0a1
backs outd0eee19c83cd9b2130c11eeea832dbceeafa37a9
0c54ee53678f9efd84330945e45b3a1349c54e71
5b202008a43160d00a21087c739bdf934175d04d
81291b4e6dc34fb0d085e8a4b7b25707ee86b7da
acbc0d7e21cd337076dbe3b9081ea85f5cb7b691
a7ceb6162a630fae3679888c8a9f588704ecbd01
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changesets d0eee19c83cd, 0c54ee53678f, 5b202008a431, 81291b4e6dc3, acbc0d7e21cd, a7ceb6162a63 (bug 1194112) for Linux x64 Hazard failure. r=backout Backed out changeset d0eee19c83cd (bug 1194112) Backed out changeset 0c54ee53678f (bug 1194112) Backed out changeset 5b202008a431 (bug 1194112) Backed out changeset 81291b4e6dc3 (bug 1194112) Backed out changeset acbc0d7e21cd (bug 1194112) Backed out changeset a7ceb6162a63 (bug 1194112)
dom/media/MediaEventSource.h
dom/media/gtest/TestMediaEventSource.cpp
--- a/dom/media/MediaEventSource.h
+++ b/dom/media/MediaEventSource.h
@@ -71,30 +71,27 @@ template <>
 struct EventTypeTraits<void> {
   typedef bool ArgType;
 };
 
 /**
  * Test if a method function or lambda accepts one or more arguments.
  */
 template <typename T>
-class TakeArgsHelper {
+class TakeArgs {
   template <typename C> static FalseType test(void(C::*)(), int);
   template <typename C> static FalseType test(void(C::*)() const, int);
   template <typename C> static FalseType test(void(C::*)() volatile, int);
   template <typename C> static FalseType test(void(C::*)() const volatile, int);
   template <typename F> static FalseType test(F&&, decltype(DeclVal<F>()(), 0));
   static TrueType test(...);
 public:
   typedef decltype(test(DeclVal<T>(), 0)) Type;
 };
 
-template <typename T>
-struct TakeArgs : public TakeArgsHelper<T>::Type {};
-
 template <typename T> struct EventTarget;
 
 template <>
 struct EventTarget<nsIEventTarget> {
   static void
   Dispatch(nsIEventTarget* aTarget, already_AddRefed<nsIRunnable>&& aTask) {
     aTarget->Dispatch(Move(aTask), NS_DISPATCH_NORMAL);
   }
@@ -116,203 +113,16 @@ template <typename T>
 class RawPtr {
 public:
   explicit RawPtr(T* aPtr) : mPtr(aPtr) {}
   T* get() const { return mPtr; }
 private:
   T* const mPtr;
 };
 
-/**
- * A helper class to pass event data to the listeners. Optimized to save
- * copy when Move is possible or |Function| takes no arguments.
- */
-template<typename Target, typename Function>
-class ListenerHelper {
-  // Define our custom runnable to minimize copy of the event data.
-  // NS_NewRunnableFunction will result in 2 copies of the event data.
-  // One is captured by the lambda and the other is the copy of the lambda.
-  template <typename T>
-  class R : public nsRunnable {
-    typedef typename RemoveCV<typename RemoveReference<T>::Type>::Type ArgType;
-  public:
-    template <typename U>
-    R(RevocableToken* aToken, const Function& aFunction, U&& aEvent)
-      : mToken(aToken), mFunction(aFunction), mEvent(Forward<U>(aEvent)) {}
-
-    NS_IMETHOD Run() override {
-      // Don't call the listener if it is disconnected.
-      if (!mToken->IsRevoked()) {
-        // Enable move whenever possible since mEvent won't be used anymore.
-        mFunction(Move(mEvent));
-      }
-      return NS_OK;
-    }
-
-  private:
-    nsRefPtr<RevocableToken> mToken;
-    Function mFunction;
-    ArgType mEvent;
-  };
-
-public:
-  ListenerHelper(RevocableToken* aToken, Target* aTarget, const Function& aFunc)
-    : mToken(aToken), mTarget(aTarget), mFunction(aFunc) {}
-
-  // |F| takes one argument.
-  template <typename F, typename T>
-  typename EnableIf<TakeArgs<F>::value, void>::Type
-  Dispatch(const F& aFunc, T&& aEvent) {
-    nsCOMPtr<nsIRunnable> r = new R<T>(mToken, aFunc, Forward<T>(aEvent));
-    EventTarget<Target>::Dispatch(mTarget.get(), r.forget());
-  }
-
-  // |F| takes no arguments. Don't bother passing aEvent.
-  template <typename F, typename T>
-  typename EnableIf<!TakeArgs<F>::value, void>::Type
-  Dispatch(const F& aFunc, T&&) {
-    const nsRefPtr<RevocableToken>& token = mToken;
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-      // Don't call the listener if it is disconnected.
-      if (!token->IsRevoked()) {
-        aFunc();
-      }
-    });
-    EventTarget<Target>::Dispatch(mTarget.get(), r.forget());
-  }
-
-  template <typename T>
-  void Dispatch(T&& aEvent) {
-    Dispatch(mFunction, Forward<T>(aEvent));
-  }
-
-private:
-  nsRefPtr<RevocableToken> mToken;
-  const nsRefPtr<Target> mTarget;
-  Function mFunction;
-};
-
-/**
- * Define whether an event data should be copied or moved to the listeners.
- *
- * @Copy Data will always be copied. Each listener gets a copy.
- * @Move Data will always be moved.
- * @Both Data will be moved when possible or copied when necessary.
- */
-enum class EventPassMode : int8_t {
-  Copy,
-  Move,
-  Both
-};
-
-class ListenerBase {
-public:
-  ListenerBase() : mToken(new RevocableToken()) {}
-  ~ListenerBase() {
-    MOZ_ASSERT(Token()->IsRevoked(), "Must disconnect the listener.");
-  }
-  RevocableToken* Token() const {
-    return mToken;
-  }
-private:
-  const nsRefPtr<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 <typename ArgType, EventPassMode Mode = EventPassMode::Copy>
-class Listener : public ListenerBase {
-public:
-  virtual ~Listener() {}
-  virtual void Dispatch(const ArgType& aEvent) = 0;
-};
-
-template <typename ArgType>
-class Listener<ArgType, EventPassMode::Both> : public ListenerBase {
-public:
-  virtual ~Listener() {}
-  virtual void Dispatch(const ArgType& aEvent) = 0;
-  virtual void Dispatch(ArgType&& aEvent) = 0;
-};
-
-template <typename ArgType>
-class Listener<ArgType, EventPassMode::Move> : public ListenerBase {
-public:
-  virtual ~Listener() {}
-  virtual void Dispatch(ArgType&& aEvent) = 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, typename ArgType, EventPassMode>
-class ListenerImpl : public Listener<ArgType, EventPassMode::Copy> {
-public:
-  ListenerImpl(Target* aTarget, const Function& aFunction)
-    : mHelper(ListenerBase::Token(), aTarget, aFunction) {}
-  void Dispatch(const ArgType& aEvent) override {
-    mHelper.Dispatch(aEvent);
-  }
-private:
-  ListenerHelper<Target, Function> mHelper;
-};
-
-template <typename Target, typename Function, typename ArgType>
-class ListenerImpl<Target, Function, ArgType, EventPassMode::Both>
-  : public Listener<ArgType, EventPassMode::Both> {
-public:
-  ListenerImpl(Target* aTarget, const Function& aFunction)
-    : mHelper(ListenerBase::Token(), aTarget, aFunction) {}
-  void Dispatch(const ArgType& aEvent) override {
-    mHelper.Dispatch(aEvent);
-  }
-  void Dispatch(ArgType&& aEvent) override {
-    mHelper.Dispatch(Move(aEvent));
-  }
-private:
-  ListenerHelper<Target, Function> mHelper;
-};
-
-template <typename Target, typename Function, typename ArgType>
-class ListenerImpl<Target, Function, ArgType, EventPassMode::Move>
-  : public Listener<ArgType, EventPassMode::Move> {
-public:
-  ListenerImpl(Target* aTarget, const Function& aFunction)
-    : mHelper(ListenerBase::Token(), aTarget, aFunction) {}
-  void Dispatch(ArgType&& aEvent) override {
-    mHelper.Dispatch(Move(aEvent));
-  }
-private:
-  ListenerHelper<Target, Function> mHelper;
-};
-
-/**
- * Select EventPassMode based on ListenerMode and if the type is copyable.
- *
- * @Copy Selected when ListenerMode is NonExclusive because each listener
- * must get a copy.
- *
- * @Move Selected when ListenerMode is Exclusive and the type is move-only.
- *
- * @Both Selected when ListenerMode is Exclusive and the type is copyable.
- * The data will be moved when possible and copied when necessary.
- */
-template <typename ArgType, ListenerMode Mode>
-struct PassModePicker {
-  // TODO: pick EventPassMode::Both when we can detect if a type is
-  // copy-constructible to allow copy-only types in Exclusive mode.
-  static const EventPassMode Value =
-    Mode == ListenerMode::NonExclusive ?
-    EventPassMode::Copy : EventPassMode::Move;
-};
-
 } // namespace detail
 
 template <typename T, ListenerMode> class MediaEventSource;
 
 /**
  * Not thread-safe since this is not meant to be shared and therefore only
  * move constructor is provided. Used to hold the result of
  * MediaEventSource<T>::Connect() and call Disconnect() to disconnect the
@@ -352,51 +162,125 @@ private:
 
 /**
  * A generic and thread-safe class to implement the observer pattern.
  */
 template <typename EventType, ListenerMode Mode = ListenerMode::NonExclusive>
 class MediaEventSource {
   static_assert(!IsReference<EventType>::value, "Ref-type not supported!");
   typedef typename detail::EventTypeTraits<EventType>::ArgType ArgType;
-  static const detail::EventPassMode PassMode
-    = detail::PassModePicker<ArgType, Mode>::Value;
-  typedef detail::Listener<ArgType, PassMode> Listener;
+
+  /**
+   * Stored by MediaEventSource to send notifications to the listener.
+   */
+  class Listener {
+  public:
+    Listener() : mToken(new RevocableToken()) {}
+
+    virtual ~Listener() {
+      MOZ_ASSERT(Token()->IsRevoked(), "Must disconnect the listener.");
+    }
+
+    virtual void Dispatch(const ArgType& aEvent) = 0;
+
+    RevocableToken* Token() const {
+      return mToken;
+    }
+
+  private:
+    const nsRefPtr<RevocableToken> mToken;
+  };
+
+  /**
+   * Store the registered target thread and function so it knows where and to
+   * whom to send the event data.
+   */
+  template<typename Target, typename Function>
+  class ListenerImpl : public Listener {
+  public:
+    explicit ListenerImpl(Target* aTarget, const Function& aFunction)
+      : mTarget(aTarget), mFunction(aFunction) {}
 
-  template<typename Target, typename Func>
-  using ListenerImpl = detail::ListenerImpl<Target, Func, ArgType, PassMode>;
+    // |Function| takes one argument.
+    void Dispatch(const ArgType& aEvent, TrueType) {
+      // Define our custom runnable to minimize copy of the event data.
+      // NS_NewRunnableFunction will result in 2 copies of the event data.
+      // One is captured by the lambda and the other is the copy of the lambda.
+      class R : public nsRunnable {
+      public:
+        R(RevocableToken* aToken,
+          const Function& aFunction, const ArgType& aEvent)
+          : mToken(aToken), mFunction(aFunction), mEvent(aEvent) {}
+
+        NS_IMETHOD Run() override {
+          // Don't call the listener if it is disconnected.
+          if (!mToken->IsRevoked()) {
+            // Enable move whenever possible since mEvent won't be used anymore.
+            mFunction(Move(mEvent));
+          }
+          return NS_OK;
+        }
+
+      private:
+        nsRefPtr<RevocableToken> mToken;
+        Function mFunction;
+        ArgType mEvent;
+      };
 
-  template <typename Method>
-  using TakeArgs = detail::TakeArgs<Method>;
+      nsCOMPtr<nsIRunnable> r = new R(this->Token(), mFunction, aEvent);
+      detail::EventTarget<Target>::Dispatch(mTarget.get(), r.forget());
+    }
+
+    // |Function| takes no arguments. Don't bother passing aEvent.
+    void Dispatch(const ArgType& aEvent, FalseType) {
+      nsRefPtr<RevocableToken> token = this->Token();
+      const Function& function = mFunction;
+      nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
+        // Don't call the listener if it is disconnected.
+        if (!token->IsRevoked()) {
+          function();
+        }
+      });
+      detail::EventTarget<Target>::Dispatch(mTarget.get(), r.forget());
+    }
+
+    void Dispatch(const ArgType& aEvent) override {
+      Dispatch(aEvent, typename detail::TakeArgs<Function>::Type());
+    }
+
+  private:
+    const nsRefPtr<Target> mTarget;
+    Function mFunction;
+  };
 
   template<typename Target, typename Function>
   MediaEventListener
   ConnectInternal(Target* aTarget, const Function& aFunction) {
     MutexAutoLock lock(mMutex);
     MOZ_ASSERT(Mode == ListenerMode::NonExclusive || mListeners.IsEmpty());
     auto l = mListeners.AppendElement();
     l->reset(new ListenerImpl<Target, Function>(aTarget, aFunction));
     return MediaEventListener((*l)->Token());
   }
 
   // |Method| takes one argument.
   template <typename Target, typename This, typename Method>
-  typename EnableIf<TakeArgs<Method>::value, MediaEventListener>::Type
-  ConnectInternal(Target* aTarget, This* aThis, Method aMethod) {
+  MediaEventListener
+  ConnectInternal(Target* aTarget, This* aThis, Method aMethod, TrueType) {
     detail::RawPtr<This> thiz(aThis);
     auto f = [=] (ArgType&& aEvent) {
       (thiz.get()->*aMethod)(Move(aEvent));
     };
     return ConnectInternal(aTarget, f);
   }
 
   // |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) {
+  MediaEventListener
+  ConnectInternal(Target* aTarget, This* aThis, Method aMethod, FalseType) {
     detail::RawPtr<This> thiz(aThis);
     auto f = [=] () {
       (thiz.get()->*aMethod)();
     };
     return ConnectInternal(aTarget, f);
   }
 
 public:
@@ -429,59 +313,59 @@ public:
    * cycle for the pending event could still hold a reference to |aThis|.
    *
    * The caller must call MediaEventListener::Disconnect() to avoid dangling
    * pointers.
    */
   template <typename This, typename Method>
   MediaEventListener
   Connect(AbstractThread* aTarget, This* aThis, Method aMethod) {
-    return ConnectInternal(aTarget, aThis, aMethod);
+    return ConnectInternal(aTarget, aThis, aMethod,
+      typename detail::TakeArgs<Method>::Type());
   }
 
   template <typename This, typename Method>
   MediaEventListener
   Connect(nsIEventTarget* aTarget, This* aThis, Method aMethod) {
-    return ConnectInternal(aTarget, aThis, aMethod);
+    return ConnectInternal(aTarget, aThis, aMethod,
+      typename detail::TakeArgs<Method>::Type());
   }
 
 protected:
   MediaEventSource() : mMutex("MediaEventSource::mMutex") {}
 
-  template <typename T>
-  void NotifyInternal(T&& aEvent) {
+  void NotifyInternal(const ArgType& aEvent) {
     MutexAutoLock lock(mMutex);
     for (int32_t i = mListeners.Length() - 1; i >= 0; --i) {
       auto&& l = mListeners[i];
       // Remove disconnected listeners.
       // It is not optimal but is simple and works well.
       if (l->Token()->IsRevoked()) {
         mListeners.RemoveElementAt(i);
         continue;
       }
-      l->Dispatch(Forward<T>(aEvent));
+      l->Dispatch(aEvent);
     }
   }
 
 private:
   Mutex mMutex;
   nsTArray<UniquePtr<Listener>> mListeners;
 };
 
 /**
  * A class to separate the interface of event subject (MediaEventSource)
  * and event publisher. Mostly used as a member variable to publish events
  * to the listeners.
  */
 template <typename EventType, ListenerMode Mode = ListenerMode::NonExclusive>
 class MediaEventProducer : public MediaEventSource<EventType, Mode> {
 public:
-  template <typename T>
-  void Notify(T&& aEvent) {
-    this->NotifyInternal(Forward<T>(aEvent));
+  void Notify(const EventType& aEvent) {
+    this->NotifyInternal(aEvent);
   }
 };
 
 /**
  * Specialization for void type. A dummy bool is passed to NotifyInternal
  * since there is no way to pass a void value.
  */
 template <>
--- a/dom/media/gtest/TestMediaEventSource.cpp
+++ b/dom/media/gtest/TestMediaEventSource.cpp
@@ -1,17 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gtest/gtest.h"
 
 #include "mozilla/TaskQueue.h"
-#include "mozilla/UniquePtr.h"
 #include "MediaEventSource.h"
 #include "VideoUtils.h"
 
 using namespace mozilla;
 
 /*
  * Test if listeners receive the event data correctly.
  */
@@ -290,34 +289,8 @@ TEST(MediaEventSource, CopyEvent2)
   source.Notify(SomeEvent(i));
 
   queue->BeginShutdown();
   queue->AwaitShutdownAndIdle();
   EXPECT_EQ(i, 0);
   listener1.Disconnect();
   listener2.Disconnect();
 }
-
-/*
- * Test move-only types.
- */
-TEST(MediaEventSource, MoveOnly)
-{
-  nsRefPtr<TaskQueue> queue = new TaskQueue(
-    GetMediaThreadPool(MediaThreadType::PLAYBACK));
-
-  MediaEventProducer<UniquePtr<int>, ListenerMode::Exclusive> source;
-
-  auto func = [] (UniquePtr<int>&& aEvent) {
-    EXPECT_EQ(*aEvent, 20);
-  };
-  MediaEventListener listener = source.Connect(queue, func);
-
-  // It is OK to pass an rvalue which is move-only.
-  source.Notify(UniquePtr<int>(new int(20)));
-  // It is an error to pass an lvalue which is move-only.
-  // UniquePtr<int> event(new int(30));
-  // source.Notify(event);
-
-  queue->BeginShutdown();
-  queue->AwaitShutdownAndIdle();
-  listener.Disconnect();
-}