Bug 1401461. P1 - remove ChannelMediaResource::Listener::Revoke(). r=gerald
☠☠ backed out by 104d47ebd0d9 ☠ ☠
authorJW Wang <jwwang@mozilla.com>
Wed, 20 Sep 2017 14:37:18 +0800
changeset 431671 0ceb7e5789f4e88468c9b2c3ed43ba762aa87b16
parent 431670 279ea05e310ad4d2bd028b26b80116c1128dc414
child 431672 ddfa978c27f7125395975b1e3f6b6acfd12a753f
push id7785
push userryanvm@gmail.com
push dateThu, 21 Sep 2017 13:39:55 +0000
treeherdermozilla-beta@06d4034a8a03 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1401461
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1401461. P1 - remove ChannelMediaResource::Listener::Revoke(). r=gerald See comment 0 for the rationale. We check |aRequest != mResource->mChannel| to know if a new channel is being loaded and the call should be aborted. MozReview-Commit-ID: 6G1x7cXNvAq
dom/media/ChannelMediaResource.cpp
dom/media/ChannelMediaResource.h
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -63,52 +63,56 @@ 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);
+  // Don't check |aRequest != mResource->mChannel| for it is a data race to read
+  // mResource->mChannel off the main thread. Instead we check the load ID to
+  // determine if the data is from an old channel.
   return mResource->OnDataAvailable(mLoadID, 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);
@@ -410,18 +414,16 @@ ChannelMediaResource::CopySegmentToCache
 }
 
 nsresult
 ChannelMediaResource::OnDataAvailable(uint32_t aLoadID,
                                       nsIInputStream* aStream,
                                       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(
     "ChannelMediaResource::OnDataAvailable",
     [self, aCount]() { self->mChannelStatistics.AddBytes(aCount); });
   mCallback->AbstractMainThread()->Dispatch(r.forget());
 
   Closure closure{ aLoadID, this };
@@ -582,17 +584,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/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -123,20 +123,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;
     const uint32_t mLoadID;
   };
   friend class Listener;
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override;
 
 protected: