Bug 1374441 - Backed out bug 1374173 (MediaCacheStream Seek&Read refactoring) - r?jwwang draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 23 Jun 2017 13:18:07 +1200
changeset 599442 248be0a5c49ba022bfd28a76f9db488e015b68d7
parent 599416 7455c74d833a9db4e02be17eda14588c7ef0de76
child 634775 828288da2972e97350bf460e848efe583d757a07
push id65524
push usergsquelart@mozilla.com
push dateFri, 23 Jun 2017 05:03:27 +0000
reviewersjwwang
bugs1374441, 1374173
milestone56.0a1
Bug 1374441 - Backed out bug 1374173 (MediaCacheStream Seek&Read refactoring) - r?jwwang Bug 1374173 seems to be the cause of all MediaCache intermittent assertion failures. It's not an important bug, so let's back-out and verify that intermittents disappear. MozReview-Commit-ID: 2X8iW1hWu99
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2238,30 +2238,46 @@ MediaCacheStream::SetPlaybackRate(uint32
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   if (aBytesPerSecond == mPlaybackBytesPerSecond)
     return;
   mPlaybackBytesPerSecond = aBytesPerSecond;
   mMediaCache->QueueUpdate();
 }
 
 nsresult
-MediaCacheStream::SeekInternal(int64_t aOffset)
+MediaCacheStream::Seek(int32_t aWhence, int64_t aOffset)
 {
-  if (aOffset < 0) {
+  NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
+
+  ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
+  if (mClosed)
+    return NS_ERROR_FAILURE;
+
+  int64_t oldOffset = mStreamOffset;
+  int64_t newOffset = mStreamOffset;
+  switch (aWhence) {
+  case PR_SEEK_END:
+    if (mStreamLength < 0)
+      return NS_ERROR_FAILURE;
+    newOffset = mStreamLength + aOffset;
+    break;
+  case PR_SEEK_CUR:
+    newOffset += aOffset;
+    break;
+  case PR_SEEK_SET:
+    newOffset = aOffset;
+    break;
+  default:
+    NS_ERROR("Unknown whence");
     return NS_ERROR_FAILURE;
   }
 
-  mMediaCache->GetReentrantMonitor().AssertCurrentThreadIn();
-
-  if (mClosed) {
+  if (newOffset < 0)
     return NS_ERROR_FAILURE;
-  }
-
-  int64_t oldOffset = mStreamOffset;
-  mStreamOffset = aOffset;
+  mStreamOffset = newOffset;
 
   LOG("Stream %p Seek to %" PRId64, this, mStreamOffset);
   mMediaCache->NoteSeek(this, oldOffset);
 
   mMediaCache->QueueUpdate();
   return NS_OK;
 }
 
@@ -2280,20 +2296,21 @@ MediaCacheStream::ThrottleReadahead(bool
 int64_t
 MediaCacheStream::Tell()
 {
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   return mStreamOffset;
 }
 
 nsresult
-MediaCacheStream::ReadInternal(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
+MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
 {
-  mMediaCache->GetReentrantMonitor().AssertCurrentThreadIn();
+  NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
 
+  ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   if (mClosed)
     return NS_ERROR_FAILURE;
 
   // Cache the offset in case it is changed again when we are waiting for the
   // monitor to be notified to avoid reading at the wrong position.
   auto streamOffset = mStreamOffset;
 
   uint32_t count = 0;
@@ -2352,17 +2369,17 @@ MediaCacheStream::ReadInternal(char* aBu
           streamWithPartialBlock->mMetadataInPartialBlockBuffer = true;
         }
         streamOffset += bytes;
         count = bytes;
         break;
       }
 
       // No data has been read yet, so block
-      mMediaCache->GetReentrantMonitor().Wait();
+      mon.Wait();
       if (mClosed) {
         // We may have successfully read some data, but let's just throw
         // that out.
         return NS_ERROR_FAILURE;
       }
       continue;
     }
 
@@ -2397,19 +2414,19 @@ MediaCacheStream::ReadInternal(char* aBu
 
 nsresult
 MediaCacheStream::ReadAt(int64_t aOffset, char* aBuffer,
                          uint32_t aCount, uint32_t* aBytes)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
 
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
-  nsresult rv = SeekInternal(aOffset);
+  nsresult rv = Seek(nsISeekableStream::NS_SEEK_SET, aOffset);
   if (NS_FAILED(rv)) return rv;
-  return ReadInternal(aBuffer, aCount, aBytes);
+  return Read(aBuffer, aCount, aBytes);
 }
 
 nsresult
 MediaCacheStream::ReadFromCache(char* aBuffer, int64_t aOffset, int64_t aCount)
 {
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
 
   // Read one block (or part of a block) at a time
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -330,22 +330,27 @@ public:
 
   // Returns true when all streams for this resource are suspended or their
   // channel has ended.
   bool AreAllStreamsForResourceSuspended();
 
   // These methods must be called on a different thread from the main
   // thread. They should always be called on the same thread for a given
   // stream.
+  // This can fail when aWhence is NS_SEEK_END and no stream length
+  // is known.
+  nsresult Seek(int32_t aWhence, int64_t aOffset);
   int64_t Tell();
-  // Seeks to aOffset in the stream then performs a Read operation.
   // *aBytes gets the number of bytes that were actually read. This can
   // be less than aCount. If the first byte of data is not in the cache,
   // this will block until the data is available or the stream is
   // closed, otherwise it won't block.
+  nsresult Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes);
+  // Seeks to aOffset in the stream then performs a Read operation. See
+  // 'Read' for argument and return details.
   nsresult ReadAt(int64_t aOffset, char* aBuffer,
                   uint32_t aCount, uint32_t* aBytes);
 
   void ThrottleReadahead(bool bThrottle);
 
   size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;
 
 private:
@@ -429,23 +434,16 @@ private:
   // that the cache monitor is held. Main thread only.
   // aReentrantMonitor is the nsAutoReentrantMonitor wrapper holding the cache monitor.
   // This is used to NotifyAll to wake up threads that might be
   // blocked on reading from this stream.
   void CloseInternal(ReentrantMonitorAutoEnter& aReentrantMonitor);
   // Update mPrincipal given that data has been received from aPrincipal
   bool UpdatePrincipal(nsIPrincipal* aPrincipal);
 
-  nsresult SeekInternal(int64_t aOffset);
-  // *aBytes gets the number of bytes that were actually read. This can
-  // be less than aCount. If the first byte of data is not in the cache,
-  // this will block until the data is available or the stream is
-  // closed, otherwise it won't block.
-  nsresult ReadInternal(char* aBuffer, uint32_t aCount, uint32_t* aBytes);
-
   // Instance of MediaCache to use with this MediaCacheStream.
   RefPtr<MediaCache> mMediaCache;
 
   // These fields are main-thread-only.
   ChannelMediaResource*  mClient;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   // Set to true when MediaCache::Update() has finished while this stream
   // was present.