From 3592b4fbbaf22abf2c9bc3a975d9309dc90f7cb2 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Sun, 17 Aug 2014 02:09:21 -0400 Subject: [PATCH 1/3] Bug 1054166: Mirror Add/RemoveListener in Add/RemoveDirectListener r=roc --- content/media/MediaStreamGraph.cpp | 38 +++++++++++++++++++++++------- content/media/MediaStreamGraph.h | 7 ++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/content/media/MediaStreamGraph.cpp b/content/media/MediaStreamGraph.cpp index 16c9ec6fd268..0e6d65e422b3 100644 --- a/content/media/MediaStreamGraph.cpp +++ b/content/media/MediaStreamGraph.cpp @@ -2471,6 +2471,32 @@ SourceMediaStream::NotifyDirectConsumers(TrackData *aTrack, } } +// These handle notifying all the listeners of an event +void +SourceMediaStream::NotifyListenersEventImpl(MediaStreamListener::MediaStreamGraphEvent aEvent) +{ + for (uint32_t j = 0; j < mListeners.Length(); ++j) { + MediaStreamListener* l = mListeners[j]; + l->NotifyEvent(GraphImpl(), aEvent); + } +} + +void +SourceMediaStream::NotifyListenersEvent(MediaStreamListener::MediaStreamGraphEvent aNewEvent) +{ + class Message : public ControlMessage { + public: + Message(SourceMediaStream* aStream, MediaStreamListener::MediaStreamGraphEvent aEvent) : + ControlMessage(aStream), mEvent(aEvent) {} + virtual void Run() + { + mStream->AsSourceStream()->NotifyListenersEventImpl(mEvent); + } + MediaStreamListener::MediaStreamGraphEvent mEvent; + }; + GraphImpl()->AppendMessage(new Message(this, aNewEvent)); +} + void SourceMediaStream::AddDirectListener(MediaStreamDirectListener* aListener) { @@ -2482,10 +2508,8 @@ SourceMediaStream::AddDirectListener(MediaStreamDirectListener* aListener) } if (wasEmpty) { - for (uint32_t j = 0; j < mListeners.Length(); ++j) { - MediaStreamListener* l = mListeners[j]; - l->NotifyEvent(GraphImpl(), MediaStreamListener::EVENT_HAS_DIRECT_LISTENERS); - } + // Async + NotifyListenersEvent(MediaStreamListener::EVENT_HAS_DIRECT_LISTENERS); } } @@ -2500,10 +2524,8 @@ SourceMediaStream::RemoveDirectListener(MediaStreamDirectListener* aListener) } if (isEmpty) { - for (uint32_t j = 0; j < mListeners.Length(); ++j) { - MediaStreamListener* l = mListeners[j]; - l->NotifyEvent(GraphImpl(), MediaStreamListener::EVENT_HAS_NO_DIRECT_LISTENERS); - } + // Async + NotifyListenersEvent(MediaStreamListener::EVENT_HAS_NO_DIRECT_LISTENERS); } } diff --git a/content/media/MediaStreamGraph.h b/content/media/MediaStreamGraph.h index 15f74f9cf8de..6d02d7bf8730 100644 --- a/content/media/MediaStreamGraph.h +++ b/content/media/MediaStreamGraph.h @@ -716,6 +716,13 @@ public: */ void SetPullEnabled(bool aEnabled); + /** + * These add/remove DirectListeners, which allow bypassing the graph and any + * synchronization delays for e.g. PeerConnection, which wants the data ASAP + * and lets the far-end handle sync and playout timing. + */ + void NotifyListenersEventImpl(MediaStreamListener::MediaStreamGraphEvent aEvent); + void NotifyListenersEvent(MediaStreamListener::MediaStreamGraphEvent aEvent); void AddDirectListener(MediaStreamDirectListener* aListener); void RemoveDirectListener(MediaStreamDirectListener* aListener); From 5a0921a9b9ec0255107aca7877bd52a3dd207cda Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Fri, 15 Aug 2014 12:12:37 -0500 Subject: [PATCH 2/3] Bug 874437 - Only enable ipc sync wait deferred Windows message handling for plugin protocols, everything else should use standard blocking waits. r=bsmedberg --- dom/plugins/ipc/PluginModuleChild.cpp | 5 ++++ dom/plugins/ipc/PluginModuleParent.cpp | 5 ++++ ipc/glue/MessageChannel.cpp | 3 ++- ipc/glue/MessageChannel.h | 20 ++++++++++++++-- ipc/glue/WindowsMessageLoop.cpp | 33 ++++++++++++-------------- 5 files changed, 45 insertions(+), 21 deletions(-) diff --git a/dom/plugins/ipc/PluginModuleChild.cpp b/dom/plugins/ipc/PluginModuleChild.cpp index 122fccafc708..4a32396803d0 100644 --- a/dom/plugins/ipc/PluginModuleChild.cpp +++ b/dom/plugins/ipc/PluginModuleChild.cpp @@ -143,6 +143,11 @@ PluginModuleChild::Init(const std::string& aPluginFilename, GetIPCChannel()->SetAbortOnError(true); + // Request Windows message deferral behavior on our channel. This + // applies to the top level and all sub plugin protocols since they + // all share the same channel. + GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_DEFERRED_MESSAGE_PROTECTION); + #ifdef XP_WIN COMMessageFilter::Initialize(this); #endif diff --git a/dom/plugins/ipc/PluginModuleParent.cpp b/dom/plugins/ipc/PluginModuleParent.cpp index b56f00c86334..0ff64570fe11 100755 --- a/dom/plugins/ipc/PluginModuleParent.cpp +++ b/dom/plugins/ipc/PluginModuleParent.cpp @@ -103,6 +103,11 @@ PluginModuleParent::LoadModule(const char* aFilePath) parent->Open(parent->mSubprocess->GetChannel(), parent->mSubprocess->GetChildProcessHandle()); + // Request Windows message deferral behavior on our channel. This + // applies to the top level and all sub plugin protocols since they + // all share the same channel. + parent->GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_DEFERRED_MESSAGE_PROTECTION); + TimeoutChanged(CHILD_TIMEOUT_PREF, parent); #ifdef MOZ_CRASHREPORTER diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index 96e52817a825..e449a839a300 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -234,7 +234,8 @@ MessageChannel::MessageChannel(MessageListener *aListener) mRemoteStackDepthGuess(false), mSawInterruptOutMsg(false), mAbortOnError(false), - mBlockScripts(false) + mBlockScripts(false), + mFlags(REQUIRE_DEFAULT) { MOZ_COUNT_CTOR(ipc::MessageChannel); diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index 960f5ff83780..2a6db9568459 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -63,13 +63,13 @@ class MessageChannel : HasResultCodes // "Open" from the perspective of the transport layer; the underlying // socketpair/pipe should already be created. // - // Returns true iff the transport layer was successfully connected, + // Returns true if the transport layer was successfully connected, // i.e., mChannelState == ChannelConnected. bool Open(Transport* aTransport, MessageLoop* aIOLoop=0, Side aSide=UnknownSide); // "Open" a connection to another thread in the same process. // - // Returns true iff the transport layer was successfully connected, + // Returns true if the transport layer was successfully connected, // i.e., mChannelState == ChannelConnected. // // For more details on the process of opening a channel between @@ -89,6 +89,19 @@ class MessageChannel : HasResultCodes mAbortOnError = true; } + // Misc. behavioral traits consumers can request for this channel + enum ChannelFlags { + REQUIRE_DEFAULT = 0, + // Windows: if this channel operates on the UI thread, indicates + // WindowsMessageLoop code should enable deferred native message + // handling to prevent deadlocks. Should only be used for protocols + // that manage child processes which might create native UI, like + // plugins. + REQUIRE_DEFERRED_MESSAGE_PROTECTION = 1 << 0 + }; + void SetChannelFlags(ChannelFlags aFlags) { mFlags = aFlags; } + ChannelFlags GetChannelFlags() { return mFlags; } + void BlockScripts(); bool ShouldBlockScripts() const @@ -649,6 +662,9 @@ class MessageChannel : HasResultCodes // Should we prevent scripts from running while dispatching urgent messages? bool mBlockScripts; + + // See SetChannelFlags + ChannelFlags mFlags; }; bool diff --git a/ipc/glue/WindowsMessageLoop.cpp b/ipc/glue/WindowsMessageLoop.cpp index fbc9c102993c..8cda66fc8368 100644 --- a/ipc/glue/WindowsMessageLoop.cpp +++ b/ipc/glue/WindowsMessageLoop.cpp @@ -657,6 +657,7 @@ InitUIThread() } // namespace ipc } // namespace mozilla +// See SpinInternalEventLoop below MessageChannel::SyncStackFrame::SyncStackFrame(MessageChannel* channel, bool interrupt) : mInterrupt(interrupt) , mSpinNestedEvents(false) @@ -665,8 +666,9 @@ MessageChannel::SyncStackFrame::SyncStackFrame(MessageChannel* channel, bool int , mPrev(mChannel->mTopFrame) , mStaticPrev(sStaticTopFrame) { - // Only track stack frames when we are on the gui thread. - if (GetCurrentThreadId() != gUIThreadId) { + // Only track stack frames when Windows message deferral behavior + // is request for the channel. + if (!(mChannel->GetChannelFlags() & REQUIRE_DEFERRED_MESSAGE_PROTECTION)) { return; } @@ -682,7 +684,7 @@ MessageChannel::SyncStackFrame::SyncStackFrame(MessageChannel* channel, bool int MessageChannel::SyncStackFrame::~SyncStackFrame() { - if (GetCurrentThreadId() != gUIThreadId) { + if (!(mChannel->GetChannelFlags() & REQUIRE_DEFERRED_MESSAGE_PROTECTION)) { return; } @@ -809,11 +811,9 @@ MessageChannel::WaitForSyncNotify() MOZ_ASSERT(gUIThreadId, "InitUIThread was not called!"); - // We jump through a lot of unique hoops in dealing with Windows message - // trapping to prevent re-entrancy when we are blocked waiting on a sync - // reply or new rpc in-call. However none of this overhead is needed when - // we aren't on the main (gui) thread. - if (GetCurrentThreadId() != gUIThreadId) { + // Use a blocking wait if this channel does not require + // Windows message deferral behavior. + if (!(mFlags & REQUIRE_DEFERRED_MESSAGE_PROTECTION)) { PRIntervalTime timeout = (kNoTimeout == mTimeoutMs) ? PR_INTERVAL_NO_TIMEOUT : PR_MillisecondsToInterval(mTimeoutMs); @@ -837,9 +837,8 @@ MessageChannel::WaitForSyncNotify() false : IsTimeoutExpired(waitStart, timeout)); } - NS_ASSERTION(GetCurrentThreadId() == gUIThreadId, - "Shouldn't be on a non-main thread in here!"); - + NS_ASSERTION(mFlags & REQUIRE_DEFERRED_MESSAGE_PROTECTION, + "Shouldn't be here for channels that don't use message deferral!"); NS_ASSERTION(mTopFrame && !mTopFrame->mInterrupt, "Top frame is not a sync frame!"); @@ -961,21 +960,19 @@ MessageChannel::WaitForInterruptNotify() MOZ_ASSERT(gUIThreadId, "InitUIThread was not called!"); - // Re-use sync notification wait code when we aren't on the - // gui thread, which bypasses the gui thread hoops we jump - // through in dealing with Windows messaging. - if (GetCurrentThreadId() != gUIThreadId) { + // Re-use sync notification wait code if this channel does not require + // Windows message deferral behavior. + if (!(mFlags & REQUIRE_DEFERRED_MESSAGE_PROTECTION)) { return WaitForSyncNotify(); } - NS_ASSERTION(GetCurrentThreadId() == gUIThreadId, - "Shouldn't be on a non-main thread in here!"); - if (!InterruptStackDepth()) { // There is currently no way to recover from this condition. NS_RUNTIMEABORT("StackDepth() is 0 in call to MessageChannel::WaitForNotify!"); } + NS_ASSERTION(mFlags & REQUIRE_DEFERRED_MESSAGE_PROTECTION, + "Shouldn't be here for channels that don't use message deferral!"); NS_ASSERTION(mTopFrame && mTopFrame->mInterrupt, "Top frame is not a sync frame!"); From eb9d8f131b8ce147baaa059b6a9b3c4f34b08009 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Sun, 17 Aug 2014 15:09:27 -0400 Subject: [PATCH 3/3] Bug 874437 - Increase B2G mochitest leak threshold to 5012 bytes. rs=khuey --- testing/mochitest/mochitest_options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/mochitest/mochitest_options.py b/testing/mochitest/mochitest_options.py index 64922175130e..aba020baaf88 100644 --- a/testing/mochitest/mochitest_options.py +++ b/testing/mochitest/mochitest_options.py @@ -756,7 +756,7 @@ class B2GOptions(MochitestOptions): defaults["testPath"] = "" defaults["extensionsToExclude"] = ["specialpowers"] # See dependencies of bug 1038943. - defaults["leakThreshold"] = 4964 + defaults["leakThreshold"] = 5012 self.set_defaults(**defaults) def verifyRemoteOptions(self, options):