Bug 1395802. P4 - we don't need lock since mChannelStatistics is always accessed on the main thread. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 01 Sep 2017 14:32:00 +0800
changeset 660058 4cfc0a9c2b4c514e772600cd94d0cc63e1fc275c
parent 660057 6fa91ade351fcd520a21147d35bfb223a0b50047
child 730131 6740e6b1d1d510d0d6c9b03fa7653a1a4fc78a0c
push id78285
push userjwwang@mozilla.com
push dateWed, 06 Sep 2017 15:49:42 +0000
bugs1395802
milestone57.0a1
Bug 1395802. P4 - we don't need lock since mChannelStatistics is always accessed on the main thread. MozReview-Commit-ID: KHnhPaSTSFr
dom/media/MediaResource.cpp
dom/media/MediaResource.h
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -79,33 +79,31 @@ ChannelMediaResource::ChannelMediaResour
                                            nsIChannel* aChannel,
                                            nsIURI* aURI,
                                            bool aIsPrivateBrowsing)
   : BaseMediaResource(aCallback, aChannel, aURI)
   , mOffset(0)
   , mReopenOnError(false)
   , mIgnoreClose(false)
   , mCacheStream(this, aIsPrivateBrowsing)
-  , mLock("ChannelMediaResource.mLock")
   , mIgnoreResume(false)
   , mSuspendAgent(mChannel)
 {
 }
 
 ChannelMediaResource::ChannelMediaResource(
   MediaResourceCallback* aCallback,
   nsIChannel* aChannel,
   nsIURI* aURI,
   const MediaChannelStatistics& aStatistics)
   : BaseMediaResource(aCallback, aChannel, aURI)
   , mOffset(0)
   , mReopenOnError(false)
   , mIgnoreClose(false)
   , mCacheStream(this, /* aIsPrivateBrowsing = */ false)
-  , mLock("ChannelMediaResource.mLock")
   , mChannelStatistics(aStatistics)
   , mIgnoreResume(false)
   , mSuspendAgent(mChannel)
 {
 }
 
 ChannelMediaResource::~ChannelMediaResource()
 {
@@ -318,22 +316,17 @@ ChannelMediaResource::OnStartRequest(nsI
     // the server gave us a range which is not quite what we asked for
 
     // If we get an HTTP_OK_CODE response to our byte range request,
     // and the server isn't sending Accept-Ranges:bytes then we don't
     // support seeking. We also can't seek in compressed streams.
     seekable = !isCompressed && acceptsRanges;
   }
   mCacheStream.SetTransportSeekable(seekable);
-
-  {
-    MutexAutoLock lock(mLock);
-    mChannelStatistics.Start();
-  }
-
+  mChannelStatistics.Start();
   mReopenOnError = false;
   mIgnoreClose = false;
 
   mSuspendAgent.UpdateSuspendedStatusIfNeeded();
 
   // Fires an initial progress event.
   owner->DownloadProgressed();
 
@@ -395,20 +388,17 @@ ChannelMediaResource::ParseContentRangeH
 
 nsresult
 ChannelMediaResource::OnStopRequest(nsIRequest* aRequest, nsresult aStatus)
 {
   NS_ASSERTION(mChannel.get() == aRequest, "Wrong channel!");
   NS_ASSERTION(!mSuspendAgent.IsSuspended(),
                "How can OnStopRequest fire while we're suspended?");
 
-  {
-    MutexAutoLock lock(mLock);
-    mChannelStatistics.Stop();
-  }
+  mChannelStatistics.Stop();
 
   // Note that aStatus might have succeeded --- this might be a normal close
   // --- even in situations where the server cut us off because we were
   // suspended. So we need to "reopen on error" in that case too. The only
   // cases where we don't need to reopen are when *we* closed the stream.
   // But don't reopen if we need to seek and we don't think we can... that would
   // cause us to just re-read the stream, which would be really bad.
   if (mReopenOnError && aStatus != NS_ERROR_PARSED_DATA_CACHED &&
@@ -673,20 +663,17 @@ ChannelMediaResource::CloneData(MediaRes
   }
   return resource.forget();
 }
 
 void ChannelMediaResource::CloseChannel()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
-  {
-    MutexAutoLock lock(mLock);
-    mChannelStatistics.Stop();
-  }
+  mChannelStatistics.Stop();
 
   if (mChannel) {
     mSuspendAgent.NotifyChannelClosing();
     // The status we use here won't be passed to the decoder, since
     // we've already revoked the listener. It can however be passed
     // to nsDocumentViewer::LoadComplete if our channel is the one
     // that kicked off creation of a video document. We don't want that
     // document load to think there was an error.
@@ -758,20 +745,17 @@ void ChannelMediaResource::Suspend(bool 
     // Kill off our channel right now, but don't tell anyone about it.
     mIgnoreClose = true;
     CloseChannel();
     element->DownloadSuspended();
   }
 
   if (mSuspendAgent.Suspend()) {
     if (mChannel) {
-      {
-        MutexAutoLock lock(mLock);
-        mChannelStatistics.Stop();
-      }
+      mChannelStatistics.Stop();
       element->DownloadSuspended();
     }
   }
 }
 
 void ChannelMediaResource::Resume()
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on non-main thread");
@@ -785,20 +769,17 @@ void ChannelMediaResource::Resume()
   if (!element) {
     // Shutting down; do nothing.
     return;
   }
 
   if (mSuspendAgent.Resume()) {
     if (mChannel) {
       // Just wake up our existing channel
-      {
-        MutexAutoLock lock(mLock);
-        mChannelStatistics.Start();
-      }
+      mChannelStatistics.Start();
       // if an error occurs after Resume, assume it's because the server
       // timed out the connection and we should reopen it.
       mReopenOnError = true;
       element->DownloadResumed();
     } else {
       int64_t totalLength = mCacheStream.GetLength();
       // If mOffset is at the end of the stream, then we shouldn't try to
       // seek to it. The seek will fail and be wasted anyway. We can leave
@@ -1003,17 +984,16 @@ ChannelMediaResource::Unpin()
 {
   mCacheStream.Unpin();
 }
 
 double
 ChannelMediaResource::GetDownloadRate(bool* aIsReliable)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MutexAutoLock lock(mLock);
   return mChannelStatistics.GetRate(aIsReliable);
 }
 
 int64_t
 ChannelMediaResource::GetLength()
 {
   return mCacheStream.GetLength();
 }
--- a/dom/media/MediaResource.h
+++ b/dom/media/MediaResource.h
@@ -576,18 +576,16 @@ protected:
   bool               mReopenOnError;
   // When this flag is set, we should not report the next close of the
   // channel.
   bool               mIgnoreClose;
 
   // Any thread access
   MediaCacheStream mCacheStream;
 
-  // This lock protects mChannelStatistics
-  Mutex               mLock;
   MediaChannelStatistics mChannelStatistics;
 
   // True if we couldn't suspend the stream and we therefore don't want
   // to resume later. This is usually due to the channel not being in the
   // isPending state at the time of the suspend request.
   bool mIgnoreResume;
 
   ChannelSuspendAgent mSuspendAgent;