From b4ee0ecebf6f39f51113a331ef5ad4f532f02522 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 28 Apr 2017 11:28:13 +0800 Subject: [PATCH] Bug 1360423 - backout P5 and P2 from bug 1281090. r=gerald It turns out that sync notification is a bad idea which is easy to be misused and could results in unexpected reentrant call flow. Since it has no users after the mass media code refactoring, it is good to remove it now to prevent future users. Backed out changeset fb5b05298007 Backed out changeset 9e1fb308cf51 MozReview-Commit-ID: 9WGvRCbvJhQ --HG-- extra : rebase_source : 748cae9449636c68f3fffbaed0e08347fe63cd91 --- dom/media/MediaEventSource.h | 138 +++++++---------------------------- 1 file changed, 28 insertions(+), 110 deletions(-) diff --git a/dom/media/MediaEventSource.h b/dom/media/MediaEventSource.h index eb0451e493b1..3b5957752b84 100644 --- a/dom/media/MediaEventSource.h +++ b/dom/media/MediaEventSource.h @@ -58,11 +58,6 @@ enum class ListenerPolicy : int8_t { NonExclusive }; -enum class DispatchPolicy : int8_t { - Sync, // Events are passed synchronously to the listeners. - Async // Events are passed asynchronously to the listeners. -}; - namespace detail { /** @@ -97,10 +92,10 @@ public: template struct TakeArgs : public TakeArgsHelper::Type {}; -template struct EventTarget; +template struct EventTarget; template <> -struct EventTarget { +struct EventTarget { static void Dispatch(nsIEventTarget* aTarget, already_AddRefed aTask) { aTarget->Dispatch(Move(aTask), NS_DISPATCH_NORMAL); @@ -108,38 +103,13 @@ struct EventTarget { }; template <> -struct EventTarget { +struct EventTarget { static void Dispatch(AbstractThread* aTarget, already_AddRefed aTask) { aTarget->Dispatch(Move(aTask), AbstractThread::DontAssertDispatchSuccess); } }; -template <> -struct EventTarget { - static bool IsOnCurrentThread(nsIEventTarget* aTarget) { - bool current = false; - auto rv = aTarget->IsOnCurrentThread(¤t); - return NS_SUCCEEDED(rv) && current; - } - static void - Dispatch(nsIEventTarget* aTarget, already_AddRefed aTask) { - MOZ_ASSERT(IsOnCurrentThread(aTarget)); - nsCOMPtr r = aTask; - r->Run(); - } -}; - -template <> -struct EventTarget { - static void - Dispatch(AbstractThread* aTarget, already_AddRefed aTask) { - MOZ_ASSERT(aTarget->IsCurrentThreadIn()); - nsCOMPtr r = aTask; - r->Run(); - } -}; - /** * Encapsulate a raw pointer to be captured by a lambda without causing * static-analysis errors. @@ -157,7 +127,7 @@ private: * A helper class to pass event data to the listeners. Optimized to save * copy when Move is possible or |Function| takes no arguments. */ -template +template class ListenerHelper { // Define our custom runnable to minimize copy of the event data. // NS_NewRunnableFunction will result in 2 copies of the event data. @@ -204,7 +174,7 @@ public: DispatchHelper(const F& aFunc, Ts&&... aEvents) { nsCOMPtr r = new R(mToken, aFunc, Forward(aEvents)...); - EventTarget::Dispatch(mTarget.get(), r.forget()); + EventTarget::Dispatch(mTarget.get(), r.forget()); } // |F| takes no arguments. Don't bother passing aEvent. @@ -218,7 +188,7 @@ public: aFunc(); } }); - EventTarget::Dispatch(mTarget.get(), r.forget()); + EventTarget::Dispatch(mTarget.get(), r.forget()); } template @@ -279,8 +249,7 @@ public: * Store the registered target thread and function so it knows where and to * whom to send the event data. */ -template +template class ListenerImpl : public Listener { public: ListenerImpl(Target* aTarget, const Function& aFunction) @@ -289,11 +258,11 @@ public: mHelper.Dispatch(aEvents...); } private: - ListenerHelper mHelper; + ListenerHelper mHelper; }; -template -class ListenerImpl +template +class ListenerImpl : public Listener { public: ListenerImpl(Target* aTarget, const Function& aFunction) @@ -302,7 +271,7 @@ public: mHelper.Dispatch(Move(aEvents)...); } private: - ListenerHelper mHelper; + ListenerHelper mHelper; }; /** @@ -337,8 +306,7 @@ struct IsAnyReference { } // namespace detail -template -class MediaEventSourceImpl; +template class MediaEventSourceImpl; /** * Not thread-safe since this is not meant to be shared and therefore only @@ -347,7 +315,7 @@ class MediaEventSourceImpl; * listener from an event source. */ class MediaEventListener { - template + template friend class MediaEventSourceImpl; public: @@ -387,7 +355,7 @@ private: /** * A generic and thread-safe class to implement the observer pattern. */ -template +template class MediaEventSourceImpl { static_assert(!detail::IsAnyReference::value, "Ref-type not supported!"); @@ -402,7 +370,7 @@ class MediaEventSourceImpl { template using ListenerImpl = - detail::ListenerImpl...>; + detail::ListenerImpl...>; template using TakeArgs = detail::TakeArgs; @@ -496,9 +464,8 @@ public: protected: MediaEventSourceImpl() : mMutex("MediaEventSourceImpl::mMutex") {} - template - typename EnableIf

::Type - NotifyInternal(IntegralConstant, Ts&&... aEvents) { + template + void NotifyInternal(Ts&&... aEvents) { MutexAutoLock lock(mMutex); int32_t last = static_cast(mListeners.Length()) - 1; for (int32_t i = last; i >= 0; --i) { @@ -513,34 +480,6 @@ protected: } } - template - typename EnableIf

::Type - NotifyInternal(IntegralConstant, Ts&&... aEvents) { - // Move |mListeners| to a new container before iteration to prevent - // |mListeners| from being disrupted if the listener calls Connect() to - // modify |mListeners| in the callback function. - nsTArray> listeners; - listeners.SwapElements(mListeners); - for (auto&& l : listeners) { - l->Dispatch(Forward(aEvents)...); - } - PruneListeners(); - // Move remaining listeners back to |mListeners|. - for (auto&& l : listeners) { - if (!l->Token()->IsRevoked()) { - mListeners.AppendElement(Move(l)); - } - } - // Perform sanity checks. - MOZ_ASSERT(Lp == ListenerPolicy::NonExclusive || mListeners.Length() <= 1); - } - - template - void Notify(Ts&&... aEvents) { - NotifyInternal(IntegralConstant(), - Forward(aEvents)...); - } - private: Mutex mMutex; nsTArray> mListeners; @@ -548,12 +487,11 @@ private: template using MediaEventSource = - MediaEventSourceImpl; + MediaEventSourceImpl; template using MediaEventSourceExc = - MediaEventSourceImpl; + MediaEventSourceImpl; /** * A class to separate the interface of event subject (MediaEventSource) @@ -563,7 +501,10 @@ using MediaEventSourceExc = template class MediaEventProducer : public MediaEventSource { public: - using MediaEventSource::Notify; + template + void Notify(Ts&&... aEvents) { + this->NotifyInternal(Forward(aEvents)...); + } }; /** @@ -574,7 +515,7 @@ template <> class MediaEventProducer : public MediaEventSource { public: void Notify() { - MediaEventSource::Notify(false /* dummy */); + this->NotifyInternal(true /* dummy */); } }; @@ -584,33 +525,10 @@ public: template class MediaEventProducerExc : public MediaEventSourceExc { public: - using MediaEventSourceExc::Notify; -}; - -/** - * Events are passed directly to the callback function of the listeners without - * dispatching. Note this class is not thread-safe. Both Connect() and Notify() - * must be called on the same thread. - */ -template -class MediaCallback - : public MediaEventSourceImpl { -public: - using MediaEventSourceImpl::Notify; -}; - -/** - * A special version of MediaCallback which allows at most one listener. - */ -template -class MediaCallbackExc - : public MediaEventSourceImpl { -public: - using MediaEventSourceImpl::Notify; + template + void Notify(Ts&&... aEvents) { + this->NotifyInternal(Forward(aEvents)...); + } }; } // namespace mozilla