Bug 1399760. P4 - remove Listener::Revoke(). draft
authorJW Wang <jwwang@mozilla.com>
Fri, 15 Sep 2017 14:49:19 +0800
changeset 665334 f64dd2a664bb7828b9c1d4bd9c99d2cc27979ffc
parent 665333 2895ccffbe9d5290d1392bbfca5698409ca9ed84
child 731743 fe0793c3e60bab4cfa35e1a2ad3ecf4cb29ab2d0
push id80022
push userjwwang@mozilla.com
push dateFri, 15 Sep 2017 09:43:21 +0000
bugs1399760
milestone57.0a1
Bug 1399760. P4 - remove Listener::Revoke(). It would be a data race for Listener::OnDataAvailable() to read mResource off the main thread while Revoke() is happening on the main thread. We remove Revoke() and apply the same concept of P3 to check if the loading channels match and continue the call flow. For OnDataAvailable() which could happen off the main thread, it is not safe to check mResource->mChannel and continue the call flow. We need special handling to ensure data from the old channel will not update the principal or be written to the wrong position (mostly done in P3). MozReview-Commit-ID: CsJnWRrAKJV
dom/media/MediaResource.cpp
dom/media/MediaResource.h
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -117,52 +117,53 @@ NS_IMPL_ISUPPORTS(ChannelMediaResource::
                   nsIChannelEventSink,
                   nsIInterfaceRequestor,
                   nsIThreadRetargetableStreamListener)
 
 nsresult
 ChannelMediaResource::Listener::OnStartRequest(nsIRequest* aRequest,
                                                nsISupports* aContext)
 {
-  if (!mResource)
+  if (aRequest != mResource->mChannel) {
     return NS_OK;
+  }
   return mResource->OnStartRequest(aRequest, mOffset);
 }
 
 nsresult
 ChannelMediaResource::Listener::OnStopRequest(nsIRequest* aRequest,
                                               nsISupports* aContext,
                                               nsresult aStatus)
 {
-  if (!mResource)
+  if (aRequest != mResource->mChannel) {
     return NS_OK;
+  }
   return mResource->OnStopRequest(aRequest, aStatus);
 }
 
 nsresult
 ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest,
                                                 nsISupports* aContext,
                                                 nsIInputStream* aStream,
                                                 uint64_t aOffset,
                                                 uint32_t aCount)
 {
   // This might happen off the main thread.
-  MOZ_DIAGNOSTIC_ASSERT(mResource);
   return mResource->OnDataAvailable(aRequest, aStream, aCount);
 }
 
 nsresult
 ChannelMediaResource::Listener::AsyncOnChannelRedirect(
   nsIChannel* aOld,
   nsIChannel* aNew,
   uint32_t aFlags,
   nsIAsyncVerifyRedirectCallback* cb)
 {
   nsresult rv = NS_OK;
-  if (mResource) {
+  if (aOld == mResource->mChannel) {
     rv = mResource->OnChannelRedirect(aOld, aNew, aFlags, mOffset);
   }
 
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   cb->OnRedirectVerifyCallback(NS_OK);
@@ -644,17 +645,16 @@ void ChannelMediaResource::CloseChannel(
     // document load to think there was an error.
     // NS_ERROR_PARSED_DATA_CACHED is the best thing we have for that
     // at the moment.
     mChannel->Cancel(NS_ERROR_PARSED_DATA_CACHED);
     mChannel = nullptr;
   }
 
   if (mListener) {
-    mListener->Revoke();
     mListener = nullptr;
   }
 }
 
 nsresult ChannelMediaResource::ReadFromCache(char* aBuffer,
                                              int64_t aOffset,
                                              uint32_t aCount)
 {
--- a/dom/media/MediaResource.h
+++ b/dom/media/MediaResource.h
@@ -515,20 +515,18 @@ public:
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIREQUESTOBSERVER
     NS_DECL_NSISTREAMLISTENER
     NS_DECL_NSICHANNELEVENTSINK
     NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
 
-    void Revoke() { mResource = nullptr; }
-
   private:
-    RefPtr<ChannelMediaResource> mResource;
+    const RefPtr<ChannelMediaResource> mResource;
     const int64_t mOffset;
   };
   friend class Listener;
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override;
 
 protected:
   bool IsSuspendedByCache();