From 1112382b61c21ee0512bf23edc54aeed5b9f059c Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Thu, 21 Sep 2017 20:29:39 -0700 Subject: [PATCH] Backed out 2 changesets (bug 1401461) for being the wrong patches Backed out changeset 20a0000f97bc (bug 1401461) Backed out changeset e3c36a62b5b1 (bug 1401461) MozReview-Commit-ID: BJeIuoGJwjb --- dom/media/ChannelMediaResource.cpp | 33 ++++++------------------------ dom/media/ChannelMediaResource.h | 9 ++------ 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/dom/media/ChannelMediaResource.cpp b/dom/media/ChannelMediaResource.cpp index ba4fd51636fb..dbb27b0bb375 100644 --- a/dom/media/ChannelMediaResource.cpp +++ b/dom/media/ChannelMediaResource.cpp @@ -68,7 +68,6 @@ nsresult ChannelMediaResource::Listener::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) { - MOZ_ASSERT(NS_IsMainThread()); if (!mResource) return NS_OK; return mResource->OnStartRequest(aRequest, mOffset); @@ -79,7 +78,6 @@ ChannelMediaResource::Listener::OnStopRequest(nsIRequest* aRequest, nsISupports* aContext, nsresult aStatus) { - MOZ_ASSERT(NS_IsMainThread()); if (!mResource) return NS_OK; return mResource->OnStopRequest(aRequest, aStatus); @@ -93,14 +91,8 @@ ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest, uint32_t aCount) { // This might happen off the main thread. - RefPtr res; - { - MutexAutoLock lock(mMutex); - res = mResource; - } - // Note Rekove() might happen at the same time to reset mResource. We check - // the load ID to determine if the data is from an old channel. - return res ? res->OnDataAvailable(mLoadID, aStream, aCount) : NS_OK; + MOZ_DIAGNOSTIC_ASSERT(mResource); + return mResource->OnDataAvailable(mLoadID, aStream, aCount); } nsresult @@ -110,8 +102,6 @@ ChannelMediaResource::Listener::AsyncOnChannelRedirect( uint32_t aFlags, nsIAsyncVerifyRedirectCallback* cb) { - MOZ_ASSERT(NS_IsMainThread()); - nsresult rv = NS_OK; if (mResource) { rv = mResource->OnChannelRedirect(aOld, aNew, aFlags, mOffset); @@ -137,14 +127,6 @@ ChannelMediaResource::Listener::GetInterface(const nsIID& aIID, void** aResult) return QueryInterface(aIID, aResult); } -void -ChannelMediaResource::Listener::Revoke() -{ - MOZ_ASSERT(NS_IsMainThread()); - MutexAutoLock lock(mMutex); - mResource = nullptr; -} - static bool IsPayloadCompressed(nsIHttpChannel* aChannel) { @@ -433,16 +415,13 @@ ChannelMediaResource::OnDataAvailable(uint32_t aLoadID, uint32_t aCount) { // This might happen off the main thread. + // Don't assert |mChannel.get() == aRequest| since reading mChannel here off + // the main thread is a data race. RefPtr self = this; nsCOMPtr r = NS_NewRunnableFunction( - "ChannelMediaResource::OnDataAvailable", [self, aCount, aLoadID]() { - if (aLoadID != self->mLoadID) { - // Ignore data from the old channel. - return; - } - self->mChannelStatistics.AddBytes(aCount); - }); + "ChannelMediaResource::OnDataAvailable", + [self, aCount]() { self->mChannelStatistics.AddBytes(aCount); }); mCallback->AbstractMainThread()->Dispatch(r.forget()); Closure closure{ aLoadID, this }; diff --git a/dom/media/ChannelMediaResource.h b/dom/media/ChannelMediaResource.h index f8ee12248d16..f7221de8007c 100644 --- a/dom/media/ChannelMediaResource.h +++ b/dom/media/ChannelMediaResource.h @@ -116,8 +116,7 @@ public: ~Listener() {} public: Listener(ChannelMediaResource* aResource, int64_t aOffset, uint32_t aLoadID) - : mMutex("Listener.mMutex") - , mResource(aResource) + : mResource(aResource) , mOffset(aOffset) , mLoadID(aLoadID) {} @@ -129,13 +128,9 @@ public: NS_DECL_NSIINTERFACEREQUESTOR NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER - void Revoke(); + void Revoke() { mResource = nullptr; } private: - Mutex mMutex; - // mResource should only be modified on the main thread with the lock. - // So it can be read without lock on the main thread or on other threads - // with the lock. RefPtr mResource; const int64_t mOffset; const uint32_t mLoadID;