Bug 1395802. P4 - we don't need lock since mChannelStatistics is always accessed on the main thread. r=cpearce
authorJW Wang <jwwang@mozilla.com>
Fri, 01 Sep 2017 14:32:00 +0800
changeset 428771 6469a2cb7df662afb23493b5566f99b6802fcb5a
parent 428770 a70f47912fbe20ae0fdba26c0123c32b0356b439
child 428911 37b95547f0d27565452136d16b2df2857be840f6
child 428914 0db93d594abc61e144653e9338c821d9a5b1574b
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1395802
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 1395802. P4 - we don't need lock since mChannelStatistics is always accessed on the main thread. r=cpearce 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
@@ -575,18 +575,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;