Bug 1401461. P1 - protect access to ChannelMediaResource::Listener::mResource. r=gerald

MozReview-Commit-ID: 6G1x7cXNvAq

--HG--
extra : rebase_source : 2f6a8642930ec314621aab2ce6424e4f9f95e0c2
extra : source : eeb067bc2905dfa6fb5764fd42ebb78e1b71a61d

dom/media/ChannelMediaResource.cpp

@@ -68,6 +68,7 @@ nsresult
 ChannelMediaResource::Listener::OnStartRequest(nsIRequest* aRequest,
                                                nsISupports* aContext)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   if (!mResource)
     return NS_OK;
   return mResource->OnStartRequest(aRequest, mOffset);
@@ -78,6 +79,7 @@ ChannelMediaResource::Listener::OnStopRequest(nsIRequest* aRequest,
                                               nsISupports* aContext,
                                               nsresult aStatus)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   if (!mResource)
     return NS_OK;
   return mResource->OnStopRequest(aRequest, aStatus);
@@ -91,8 +93,14 @@ ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest,
                                                 uint32_t aCount)
 {
   // This might happen off the main thread.
-  MOZ_DIAGNOSTIC_ASSERT(mResource);
-  return mResource->OnDataAvailable(mLoadID, aStream, aCount);
+  RefPtr<ChannelMediaResource> 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;
 }
 
 nsresult
@@ -102,6 +110,8 @@ 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);
@@ -127,6 +137,14 @@ 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)
 {
@@ -415,8 +433,6 @@ 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<ChannelMediaResource> self = this;
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(

dom/media/ChannelMediaResource.h

@@ -116,7 +116,8 @@ public:
     ~Listener() {}
   public:
     Listener(ChannelMediaResource* aResource, int64_t aOffset, uint32_t aLoadID)
-      : mResource(aResource)
+      : mMutex("Listener.mMutex")
+      , mResource(aResource)
       , mOffset(aOffset)
       , mLoadID(aLoadID)
     {}
@@ -128,9 +129,13 @@ public:
     NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
 
-    void Revoke() { mResource = nullptr; }
+    void Revoke();
 
   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<ChannelMediaResource> mResource;
     const int64_t mOffset;
     const uint32_t mLoadID;