Backed out 4 changesets (bug 1412737)for frequent Windows talos-g4 timeouts. a=RyanVM
authorNarcis Beleuzu <nbeleuzu@mozilla.com>
Wed, 08 Nov 2017 18:52:15 +0200
changeset 444021 3f797c2aedfb7be631be963e1a402fa9054076c7
parent 443974 f63559d7e6a570e4e73ba367964099394248e18d
child 444022 75c04adb414bc00d39a0af80691a82af1ceebbd0
child 444063 ac109e443ac2facf708848a6f95a5a1e553596ed
child 444087 baa4067320eee09b4fd1d3cdbfeae78888b75bd0
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersRyanVM
bugs1412737
milestone58.0a1
backs out13b3569d56c4788967266f8f33a1e8860a92d8a5
b8ae4f1e89c92e7f3b0a5f0335497d54fc848e27
dd35b8813ca1e7ad8cbafcde88435b1ddc62ce9a
29e511fbcd627fecb2c3f5b4b99d0a747223bfd6
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
Backed out 4 changesets (bug 1412737)for frequent Windows talos-g4 timeouts. a=RyanVM Backed out changeset 13b3569d56c4 (bug 1412737) Backed out changeset b8ae4f1e89c9 (bug 1412737) Backed out changeset dd35b8813ca1 (bug 1412737) Backed out changeset 29e511fbcd62 (bug 1412737)
dom/media/MediaCache.cpp
dom/media/MediaResource.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -9,17 +9,16 @@
 #include "ChannelMediaResource.h"
 #include "FileBlockCache.h"
 #include "MediaBlockCacheBase.h"
 #include "MediaPrefs.h"
 #include "MediaResource.h"
 #include "MemoryBlockCache.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ClearOnShutdown.h"
-#include "mozilla/ErrorNames.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/ReentrantMonitor.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/Telemetry.h"
 #include "nsContentUtils.h"
@@ -31,21 +30,19 @@
 #include "nsThreadUtils.h"
 #include "prio.h"
 #include <algorithm>
 
 namespace mozilla {
 
 #undef LOG
 #undef LOGI
-#undef LOGE
 LazyLogModule gMediaCacheLog("MediaCache");
 #define LOG(...) MOZ_LOG(gMediaCacheLog, LogLevel::Debug, (__VA_ARGS__))
 #define LOGI(...) MOZ_LOG(gMediaCacheLog, LogLevel::Info, (__VA_ARGS__))
-#define LOGE(...) NS_DebugBreak(NS_DEBUG_WARNING, nsPrintfCString(__VA_ARGS__).get(), nullptr, __FILE__, __LINE__)
 
 // For HTTP seeking, if number of bytes needing to be
 // seeked forward is less than this value then a read is
 // done rather than a byte range request.
 //
 // If we assume a 100Mbit connection, and assume reissuing an HTTP seek causes
 // a delay of 200ms, then in that 200ms we could have simply read ahead 2MB. So
 // setting SEEK_VS_READ_THRESHOLD to 1MB sounds reasonable.
@@ -2013,19 +2010,16 @@ MediaCacheStream::NotifyDataReceived(uin
 
   if (mLoadID != aLoadID) {
     // mChannelOffset is updated to a new position when loading a new channel.
     // We should discard the data coming from the old channel so it won't be
     // stored to the wrong positoin.
     return;
   }
 
-  // True if we commit any blocks to the cache.
-  bool cacheUpdated = false;
-
   auto source = MakeSpan<const uint8_t>(aData, aCount);
 
   // We process the data one block (or part of a block) at a time
   while (!source.IsEmpty()) {
     // The data we've collected so far in the partial block.
     auto partial = MakeSpan<const uint8_t>(mPartialBlockBuffer.get(),
                                            OffsetInBlock(mChannelOffset));
 
@@ -2043,17 +2037,16 @@ MediaCacheStream::NotifyDataReceived(uin
       mMediaCache->AllocateAndWriteBlock(
         this,
         OffsetToBlockIndexUnchecked(mChannelOffset),
         mMetadataInPartialBlockBuffer ? MODE_METADATA : MODE_PLAYBACK,
         partial,
         source.First(remaining));
       source = source.From(remaining);
       mChannelOffset += remaining;
-      cacheUpdated = true;
     } else {
       // The buffer to be filled in the partial block.
       auto buf = MakeSpan<uint8_t>(mPartialBlockBuffer.get() + partial.Length(),
                                    remaining);
       memcpy(buf.Elements(), source.Elements(), source.Length());
       mChannelOffset += source.Length();
       break;
     }
@@ -2063,22 +2056,20 @@ MediaCacheStream::NotifyDataReceived(uin
   while (MediaCacheStream* stream = iter.Next()) {
     if (stream->mStreamLength >= 0) {
       // The stream is at least as long as what we've read
       stream->mStreamLength = std::max(stream->mStreamLength, mChannelOffset);
     }
     stream->mClient->CacheClientNotifyDataReceived();
   }
 
+  // Notify in case there's a waiting reader
   // XXX it would be fairly easy to optimize things a lot more to
   // avoid waking up reader threads unnecessarily
-  if (cacheUpdated) {
-    // Wake up the reader who is waiting for the committed blocks.
-    mon.NotifyAll();
-  }
+  mon.NotifyAll();
 }
 
 void
 MediaCacheStream::FlushPartialBlockInternal(bool aNotifyAll,
                                             ReentrantMonitorAutoEnter& aReentrantMonitor)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
@@ -2545,34 +2536,30 @@ MediaCacheStream::Tell()
 }
 
 nsresult
 MediaCacheStream::Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes)
 {
   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;
   // Read one block (or part of a block) at a time
   while (count < aCount) {
-    if (mClosed) {
-      return NS_ERROR_ABORT;
-    }
-
     int32_t streamBlock = OffsetToBlockIndex(streamOffset);
     if (streamBlock < 0) {
-      LOGE("Stream %p invalid offset=%" PRId64, this, streamOffset);
-      return NS_ERROR_ILLEGAL_VALUE;
+      break;
     }
-
     uint32_t offsetInStreamBlock = uint32_t(streamOffset - streamBlock*BLOCK_SIZE);
     int64_t size = std::min<int64_t>(aCount - count, BLOCK_SIZE - offsetInStreamBlock);
 
     if (mStreamLength >= 0) {
       // 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
@@ -2583,92 +2570,96 @@ MediaCacheStream::Read(char* aBuffer, ui
       size = std::min(size, int64_t(INT32_MAX));
     }
 
     int32_t cacheBlock =
       size_t(streamBlock) < mBlocks.Length() ? mBlocks[streamBlock] : -1;
     if (cacheBlock < 0) {
       // We don't have a complete cached block here.
 
+      if (count > 0) {
+        // Some data has been read, so return what we've got instead of
+        // blocking or trying to find a stream with a partial block.
+        break;
+      }
+
       // 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) {
+            streamOffset < stream->mChannelOffset) {
           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);
+        memcpy(aBuffer,
+          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.
+        count = bytes;
         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();
+      if (mClosed) {
+        // We may have successfully read some data, but let's just throw
+        // that out.
+        return NS_ERROR_FAILURE;
+      }
       continue;
     }
 
     mMediaCache->NoteBlockUsage(
       this, cacheBlock, streamOffset, mCurrentMode, TimeStamp::Now());
 
     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 (count == 0)
+        return rv;
+      // If we did successfully read some data, may as well return it
+      break;
     }
     streamOffset += bytes;
     count += bytes;
   }
 
-  *aBytes = count;
-  if (count == 0) {
-    return NS_OK;
+  if (count > 0) {
+    // Some data was read, so queue an update since block priorities may
+    // have changed
+    mMediaCache->QueueUpdate();
   }
-
-  // Some data was read, so queue an update since block priorities may
-  // have changed
-  mMediaCache->QueueUpdate();
-
   LOG("Stream %p Read at %" PRId64 " count=%d", this, streamOffset-count, count);
+  *aBytes = count;
   mStreamOffset = streamOffset;
   return NS_OK;
 }
 
 nsresult
 MediaCacheStream::ReadAt(int64_t aOffset, char* aBuffer,
                          uint32_t aCount, uint32_t* aBytes)
 {
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -409,42 +409,83 @@ MediaResourceIndex::CacheOrReadAt(int64_
 }
 
 nsresult
 MediaResourceIndex::UncachedReadAt(int64_t aOffset,
                                    char* aBuffer,
                                    uint32_t aCount,
                                    uint32_t* aBytes) const
 {
+  *aBytes = 0;
   if (aOffset < 0) {
     return NS_ERROR_ILLEGAL_VALUE;
   }
-  if (aCount == 0) {
-    *aBytes = 0;
-    return NS_OK;
+  if (aCount != 0) {
+    for (;;) {
+      uint32_t bytesRead = 0;
+      nsresult rv = mResource->ReadAt(aOffset, aBuffer, aCount, &bytesRead);
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
+      if (bytesRead == 0) {
+        break;
+      }
+      *aBytes += bytesRead;
+      aCount -= bytesRead;
+      if (aCount == 0) {
+        break;
+      }
+      aOffset += bytesRead;
+      if (aOffset < 0) {
+        // Very unlikely overflow.
+        return NS_ERROR_FAILURE;
+      }
+      aBuffer += bytesRead;
+    }
   }
-  return mResource->ReadAt(aOffset, aBuffer, aCount, aBytes);
+  return NS_OK;
 }
 
 nsresult
 MediaResourceIndex::UncachedRangedReadAt(int64_t aOffset,
                                          char* aBuffer,
                                          uint32_t aRequestedCount,
                                          uint32_t aExtraCount,
                                          uint32_t* aBytes) const
 {
+  *aBytes = 0;
   uint32_t count = aRequestedCount + aExtraCount;
   if (aOffset < 0 || count < aRequestedCount) {
     return NS_ERROR_ILLEGAL_VALUE;
   }
-  if (count == 0) {
-    *aBytes = 0;
-    return NS_OK;
+  if (count != 0) {
+    for (;;) {
+      uint32_t bytesRead = 0;
+      nsresult rv = mResource->ReadAt(aOffset, aBuffer, count, &bytesRead);
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
+      if (bytesRead == 0) {
+        break;
+      }
+      *aBytes += bytesRead;
+      count -= bytesRead;
+      if (count <= aExtraCount) {
+        // We have read at least aRequestedCount, don't loop anymore.
+        break;
+      }
+      aOffset += bytesRead;
+      if (aOffset < 0) {
+        // Very unlikely overflow.
+        return NS_ERROR_FAILURE;
+      }
+      aBuffer += bytesRead;
+    }
   }
-  return mResource->ReadAt(aOffset, aBuffer, count, aBytes);
+  return NS_OK;
 }
 
 nsresult
 MediaResourceIndex::Seek(int32_t aWhence, int64_t aOffset)
 {
   switch (aWhence) {
     case SEEK_SET:
       break;
@@ -470,27 +511,40 @@ MediaResourceIndex::Seek(int32_t aWhence
   mOffset = aOffset;
 
   return NS_OK;
 }
 
 already_AddRefed<MediaByteBuffer>
 MediaResourceIndex::MediaReadAt(int64_t aOffset, uint32_t aCount) const
 {
-  NS_ENSURE_TRUE(aOffset >= 0, nullptr);
   RefPtr<MediaByteBuffer> bytes = new MediaByteBuffer();
+  if (aOffset < 0) {
+    return bytes.forget();
+  }
   bool ok = bytes->SetLength(aCount, fallible);
   NS_ENSURE_TRUE(ok, nullptr);
-
-  uint32_t bytesRead = 0;
-  nsresult rv = mResource->ReadAt(
-    aOffset, reinterpret_cast<char*>(bytes->Elements()), aCount, &bytesRead);
-  NS_ENSURE_SUCCESS(rv, nullptr);
-
-  bytes->SetLength(bytesRead);
+  char* curr = reinterpret_cast<char*>(bytes->Elements());
+  const char* start = curr;
+  while (aCount > 0) {
+    uint32_t bytesRead;
+    nsresult rv = mResource->ReadAt(aOffset, curr, aCount, &bytesRead);
+    NS_ENSURE_SUCCESS(rv, nullptr);
+    if (!bytesRead) {
+      break;
+    }
+    aOffset += bytesRead;
+    if (aOffset < 0) {
+      // Very unlikely overflow.
+      break;
+    }
+    aCount -= bytesRead;
+    curr += bytesRead;
+  }
+  bytes->SetLength(curr - start);
   return bytes.forget();
 }
 
 already_AddRefed<MediaByteBuffer>
 MediaResourceIndex::CachedMediaReadAt(int64_t aOffset, uint32_t aCount) const
 {
   RefPtr<MediaByteBuffer> bytes = new MediaByteBuffer();
   bool ok = bytes->SetLength(aCount, fallible);