Bug 1416085 - use Span<> to replace low level pointer arithmetic in Read(). draft
authorJW Wang <jwwang@mozilla.com>
Fri, 03 Nov 2017 16:56:58 +0800
changeset 697485 446a0bf591ef46ed0ac7e580907e6233f0609837
parent 696951 6371e71153c8c3d32d31c8eb92ac296f353dd860
child 740140 cf39f09216992c5b2ba3f58186b8c4780a5acd1b
push id89025
push userjwwang@mozilla.com
push dateTue, 14 Nov 2017 05:53:59 +0000
bugs1416085
milestone59.0a1
Bug 1416085 - use Span<> to replace low level pointer arithmetic in Read(). MozReview-Commit-ID: 6l7cG2Xn0R7
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2457,17 +2457,19 @@ MediaCacheStream::ReadPartialBlock(int64
   // We have |source.Length() <= BLOCK_SIZE < INT32_MAX| to guarantee
   // that |bytesToRead| can fit into a uint32_t.
   uint32_t bytesToRead = std::min(aBuffer.Length(), source.Length());
   memcpy(aBuffer.Elements(), source.Elements(), bytesToRead);
   return bytesToRead;
 }
 
 Result<uint32_t, nsresult>
-MediaCacheStream::ReadBlockFromCache(int64_t aOffset, Span<char> aBuffer)
+MediaCacheStream::ReadBlockFromCache(int64_t aOffset,
+                                     Span<char> aBuffer,
+                                     bool aNoteBlockUsage)
 {
   mMediaCache->GetReentrantMonitor().AssertCurrentThreadIn();
   MOZ_ASSERT(IsOffsetAllowed(aOffset));
 
   // OffsetToBlockIndexUnchecked() is always non-negative.
   uint32_t index = OffsetToBlockIndexUnchecked(aOffset);
   int32_t cacheBlock = index < mBlocks.Length() ? mBlocks[index] : -1;
   if (cacheBlock < 0) {
@@ -2496,16 +2498,21 @@ MediaCacheStream::ReadBlockFromCache(int
 
   if (NS_FAILED(rv)) {
     nsCString name;
     GetErrorName(rv, name);
     LOGE("Stream %p ReadCacheFile failed, rv=%s", this, name.Data());
     return mozilla::Err(rv);
   }
 
+  if (aNoteBlockUsage) {
+    mMediaCache->NoteBlockUsage(
+      this, cacheBlock, aOffset, mCurrentMode, TimeStamp::Now());
+  }
+
   return bytesRead;
 }
 
 int64_t
 MediaCacheStream::Tell()
 {
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   return mStreamOffset;
@@ -2517,115 +2524,83 @@ MediaCacheStream::Read(char* aBuffer, ui
   NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
 
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
 
   // 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;
+  // The buffer we are about to fill.
+  auto buffer = MakeSpan<char>(aBuffer, aCount);
+
   // Read one block (or part of a block) at a time
-  while (count < aCount) {
+  while (!buffer.IsEmpty()) {
     if (mClosed) {
       return NS_ERROR_ABORT;
     }
 
-    int32_t streamBlock = OffsetToBlockIndex(streamOffset);
-    if (streamBlock < 0) {
+    if (!IsOffsetAllowed(streamOffset)) {
       LOGE("Stream %p invalid offset=%" PRId64, this, streamOffset);
       return NS_ERROR_ILLEGAL_VALUE;
     }
 
-    uint32_t offsetInStreamBlock = uint32_t(streamOffset - streamBlock*BLOCK_SIZE);
-    int64_t size = std::min<int64_t>(aCount - count, BLOCK_SIZE - offsetInStreamBlock);
-
-    if (mStreamLength >= 0) {
+    if (mStreamLength >= 0 && streamOffset >= mStreamLength) {
       // Don't try to read beyond the end of the stream
-      int64_t bytesRemaining = mStreamLength - streamOffset;
-      if (bytesRemaining <= 0) {
-        // Get out of here and return NS_OK
-        break;
-      }
-      size = std::min(size, bytesRemaining);
-      // Clamp size until 64-bit file size issues are fixed.
-      size = std::min(size, int64_t(INT32_MAX));
+      break;
     }
 
-    int32_t cacheBlock =
-      size_t(streamBlock) < mBlocks.Length() ? mBlocks[streamBlock] : -1;
-    if (cacheBlock < 0) {
-      // We don't have a complete cached block here.
+    Result<uint32_t, nsresult> rv =
+      ReadBlockFromCache(streamOffset, buffer, true /* aNoteBlockUsage */);
+    if (rv.isErr()) {
+      return rv.unwrapErr();
+    }
 
-      // See if the data is available in the partial cache block of any
-      // stream reading this resource. We need to do this in case there is
-      // another stream with this resource that has all the data to the end of
-      // the stream but the data doesn't end on a block boundary.
-      MediaCacheStream* streamWithPartialBlock = nullptr;
-      MediaCache::ResourceStreamIterator iter(mMediaCache, mResourceID);
-      while (MediaCacheStream* stream = iter.Next()) {
-        if (OffsetToBlockIndexUnchecked(stream->mChannelOffset) ==
-              streamBlock &&
-            streamOffset < stream->mChannelOffset &&
-            stream->mChannelOffset == stream->mStreamLength) {
-          streamWithPartialBlock = stream;
-          break;
-        }
-      }
-      if (streamWithPartialBlock) {
-        // We can just use the data in mPartialBlockBuffer. In fact we should
-        // use it rather than waiting for the block to fill and land in
-        // the cache.
-        int64_t bytes = std::min<int64_t>(size, streamWithPartialBlock->mChannelOffset - streamOffset);
-        // Clamp bytes until 64-bit file size issues are fixed.
-        bytes = std::min(bytes, int64_t(INT32_MAX));
-        MOZ_ASSERT(bytes >= 0 && bytes <= aCount, "Bytes out of range.");
-        memcpy(aBuffer + count,
-               streamWithPartialBlock->mPartialBlockBuffer.get() +
-                 offsetInStreamBlock,
-               bytes);
-        if (mCurrentMode == MODE_METADATA) {
-          streamWithPartialBlock->mMetadataInPartialBlockBuffer = true;
-        }
-        streamOffset += bytes;
-        count += bytes;
-        // Break for we've reached EOS and have nothing more to read.
-        break;
-      }
-
-      if (mStreamOffset != streamOffset) {
-        // Updat mStreamOffset before we drop the lock. We need to run
-        // Update() again since stream reading strategy might have changed.
-        mStreamOffset = streamOffset;
-        mMediaCache->QueueUpdate();
-      }
-
-      // No data has been read yet, so block
-      mon.Wait();
+    uint32_t bytes = rv.unwrap();
+    if (bytes > 0) {
+      // Got data from the cache successfully. Read next block.
+      streamOffset += bytes;
+      buffer = buffer.From(bytes);
       continue;
     }
 
-    mMediaCache->NoteBlockUsage(
-      this, cacheBlock, streamOffset, mCurrentMode, TimeStamp::Now());
+    // See if we can use the data in the partial block of any stream reading
+    // this resource. Note we use the partial block only when it is completed,
+    // that is reaching EOS.
+    bool foundDataInPartialBlock = false;
+    MediaCache::ResourceStreamIterator iter(mMediaCache, mResourceID);
+    while (MediaCacheStream* stream = iter.Next()) {
+      if (OffsetToBlockIndexUnchecked(stream->mChannelOffset) ==
+            OffsetToBlockIndexUnchecked(streamOffset) &&
+          stream->mChannelOffset == stream->mStreamLength) {
+        uint32_t bytes = stream->ReadPartialBlock(streamOffset, buffer);
+        streamOffset += bytes;
+        buffer = buffer.From(bytes);
+        foundDataInPartialBlock = true;
+        break;
+      }
+    }
+    if (foundDataInPartialBlock) {
+      // Break for we've reached EOS.
+      break;
+    }
 
-    int64_t offset = cacheBlock*BLOCK_SIZE + offsetInStreamBlock;
-    int32_t bytes;
-    MOZ_ASSERT(size >= 0 && size <= INT32_MAX, "Size out of range.");
-    nsresult rv = mMediaCache->ReadCacheFile(
-      offset, aBuffer + count, int32_t(size), &bytes);
-    if (NS_FAILED(rv)) {
-      nsCString name;
-      GetErrorName(rv, name);
-      LOGE("Stream %p ReadCacheFile failed, rv=%s", this, name.Data());
-      return rv;
+    if (mStreamOffset != streamOffset) {
+      // Update mStreamOffset before we drop the lock. We need to run
+      // Update() again since stream reading strategy might have changed.
+      mStreamOffset = streamOffset;
+      mMediaCache->QueueUpdate();
     }
-    streamOffset += bytes;
-    count += bytes;
+
+    // No data to read, so block
+    mon.Wait();
+    continue;
   }
 
+  uint32_t count = buffer.Elements() - aBuffer;
   *aBytes = count;
   if (count == 0) {
     return NS_OK;
   }
 
   // Some data was read, so queue an update since block priorities may
   // have changed
   mMediaCache->QueueUpdate();
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -423,17 +423,18 @@ private:
   // Read data from the partial block and return the number of bytes read
   // successfully. 0 if aOffset is not an offset in the partial block or there
   // is nothing to read.
   uint32_t ReadPartialBlock(int64_t aOffset, Span<char> aBuffer);
 
   // Read data from the cache block specified by aOffset. Return the number of
   // bytes read successfully or an error code if any failure.
   Result<uint32_t, nsresult> ReadBlockFromCache(int64_t aOffset,
-                                                Span<char> aBuffer);
+                                                Span<char> aBuffer,
+                                                bool aNoteBlockUsage = false);
 
   // Non-main thread only.
   nsresult Seek(int64_t aOffset);
 
   // Returns the end of the bytes starting at the given offset
   // which are in cache.
   // This method assumes that the cache monitor is held and can be called on
   // any thread.