Bug 1400166. P1 - move re-initialization code to Flush() for it is confusing to call Init() twice. draft
authorJW Wang <jwwang@mozilla.com>
Mon, 18 Sep 2017 10:52:17 +0800
changeset 666188 ba49d265551a889519ebea1b37e05cd8b328e575
parent 666127 ae39864562c6048fdc2950c5dfedb48e247c3300
child 666189 9b140bb729b974f0146c25704c09173ce63a0de2
push id80301
push userjwwang@mozilla.com
push dateMon, 18 Sep 2017 06:00:41 +0000
bugs1400166
milestone57.0a1
Bug 1400166. P1 - move re-initialization code to Flush() for it is confusing to call Init() twice. MozReview-Commit-ID: 6KolHyGkqXo
dom/media/FileBlockCache.cpp
dom/media/FileBlockCache.h
dom/media/MediaBlockCacheBase.h
dom/media/MediaCache.cpp
dom/media/MemoryBlockCache.cpp
dom/media/MemoryBlockCache.h
--- a/dom/media/FileBlockCache.cpp
+++ b/dom/media/FileBlockCache.cpp
@@ -71,28 +71,19 @@ FileBlockCache::SetCacheFile(PRFileDesc*
     CloseFD(mFD);
     mFD = nullptr;
   }
 }
 
 nsresult
 FileBlockCache::Init()
 {
+  LOG("Init()");
   MutexAutoLock mon(mDataMutex);
-  if (mThread) {
-    LOG("Init() again");
-    // Just discard pending changes, assume MediaCache won't read from
-    // blocks it hasn't written to.
-    mChangeIndexList.clear();
-    mBlockChanges.Clear();
-    return NS_OK;
-  }
-
-  LOG("Init()");
-
+  MOZ_ASSERT(!mThread);
   nsresult rv = NS_NewNamedThread("FileBlockCache",
                                   getter_AddRefs(mThread),
                                   nullptr,
                                   SharedThreadPool::kStackSize);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
@@ -119,16 +110,28 @@ FileBlockCache::Init()
 
   if (NS_FAILED(rv)) {
     Close();
   }
 
   return rv;
 }
 
+void
+FileBlockCache::Flush()
+{
+  LOG("Flush()");
+  MutexAutoLock mon(mDataMutex);
+  MOZ_ASSERT(mThread);
+  // Just discard pending changes, assume MediaCache won't read from
+  // blocks it hasn't written to.
+  mChangeIndexList.clear();
+  mBlockChanges.Clear();
+}
+
 int32_t
 FileBlockCache::GetMaxBlocks() const
 {
   // We look up the cache size every time. This means dynamic changes
   // to the pref are applied.
   const uint32_t cacheSizeKb =
     std::min(MediaPrefs::MediaCacheSizeKb(), uint32_t(INT32_MAX) * 2);
   // Ensure we can divide BLOCK_SIZE by 1024.
--- a/dom/media/FileBlockCache.h
+++ b/dom/media/FileBlockCache.h
@@ -57,19 +57,21 @@ class FileBlockCache : public MediaBlock
 public:
   FileBlockCache();
 
 protected:
   virtual ~FileBlockCache();
 
 public:
   // Launch thread and open temporary file.
-  // If re-initializing, just discard pending writes if any.
   nsresult Init() override;
 
+  // Will discard pending changes if any.
+  void Flush() override;
+
   // Maximum number of blocks allowed in this block cache.
   // Calculated from "media.cache_size" pref.
   int32_t GetMaxBlocks() const override;
 
   // Can be called on any thread. This defers to a non-main thread.
   nsresult WriteBlock(uint32_t aBlockIndex,
                       Span<const uint8_t> aData1,
                       Span<const uint8_t> aData2) override;
--- a/dom/media/MediaBlockCacheBase.h
+++ b/dom/media/MediaBlockCacheBase.h
@@ -44,19 +44,22 @@ public:
     "MediaCacheStream::BLOCK_SIZE should fit in 31 bits");
   static const int32_t BLOCK_SIZE = MediaCacheStream::BLOCK_SIZE;
 
 protected:
   virtual ~MediaBlockCacheBase() {}
 
 public:
   // Initialize this cache.
-  // If called again, re-initialize cache with minimal chance of failure.
   virtual nsresult Init() = 0;
 
+  // Erase data and discard pending changes to reset the cache to its pristine
+  // state as it was after Init().
+  virtual void Flush() = 0;
+
   // Maximum number of blocks expected in this block cache. (But allow overflow
   // to accomodate incoming traffic before MediaCache can handle it.)
   virtual int32_t GetMaxBlocks() const = 0;
 
   // Can be called on any thread. This defers to a non-main thread.
   virtual nsresult WriteBlock(uint32_t aBlockIndex,
                               Span<const uint8_t> aData1,
                               Span<const uint8_t> aData2) = 0;
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -662,19 +662,18 @@ MediaCache::Flush()
 
   for (uint32_t blockIndex = 0; blockIndex < mIndex.Length(); ++blockIndex) {
     FreeBlock(blockIndex);
   }
 
   // Truncate index array.
   Truncate();
   NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
-  // Re-initialize block cache.
-  nsresult rv = mBlockCache->Init();
-  NS_ENSURE_SUCCESS_VOID(rv);
+  // Reset block cache to its pristine state.
+  mBlockCache->Flush();
 }
 
 void
 MediaCache::CloseStreamsForPrivateBrowsing()
 {
   MOZ_ASSERT(NS_IsMainThread());
   for (MediaCacheStream* s : mStreams) {
     if (s->mIsPrivateBrowsing) {
--- a/dom/media/MemoryBlockCache.cpp
+++ b/dom/media/MemoryBlockCache.cpp
@@ -254,35 +254,37 @@ MemoryBlockCache::EnsureBufferCanContain
       watermark);
   mHasGrown = true;
   return true;
 }
 
 nsresult
 MemoryBlockCache::Init()
 {
+  LOG("Init()");
   MutexAutoLock lock(mMutex);
-  if (mBuffer.IsEmpty()) {
-    LOG("Init()");
-    // Attempt to pre-allocate buffer for expected content length.
-    if (!EnsureBufferCanContain(mInitialContentLength)) {
-      LOG("Init() MEMORYBLOCKCACHE_ERRORS='InitAllocation'");
-      Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
-                            InitAllocation);
-      return NS_ERROR_FAILURE;
-    }
-  } else {
-    LOG("Init() again");
-    // Re-initialization - Just erase data.
-    MOZ_ASSERT(mBuffer.Length() >= mInitialContentLength);
-    memset(mBuffer.Elements(), 0, mBuffer.Length());
+  MOZ_ASSERT(mBuffer.IsEmpty());
+  // Attempt to pre-allocate buffer for expected content length.
+  if (!EnsureBufferCanContain(mInitialContentLength)) {
+    LOG("Init() MEMORYBLOCKCACHE_ERRORS='InitAllocation'");
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          InitAllocation);
+    return NS_ERROR_FAILURE;
   }
-  // Ignore initial growth.
+  return NS_OK;
+}
+
+void
+MemoryBlockCache::Flush()
+{
+  LOG("Flush()");
+  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(mBuffer.Length() >= mInitialContentLength);
+  memset(mBuffer.Elements(), 0, mBuffer.Length());
   mHasGrown = false;
-  return NS_OK;
 }
 
 nsresult
 MemoryBlockCache::WriteBlock(uint32_t aBlockIndex,
                              Span<const uint8_t> aData1,
                              Span<const uint8_t> aData2)
 {
   MutexAutoLock lock(mMutex);
--- a/dom/media/MemoryBlockCache.h
+++ b/dom/media/MemoryBlockCache.h
@@ -37,16 +37,18 @@ public:
 protected:
   virtual ~MemoryBlockCache();
 
 public:
   // Allocate initial buffer.
   // If re-initializing, clear buffer.
   virtual nsresult Init() override;
 
+  void Flush() override;
+
   // Maximum number of blocks allowed in this block cache.
   // Based on initial content length, and minimum usable block cache.
   int32_t GetMaxBlocks() const override { return mMaxBlocks; }
 
   // Can be called on any thread.
   virtual nsresult WriteBlock(uint32_t aBlockIndex,
                               Span<const uint8_t> aData1,
                               Span<const uint8_t> aData2) override;
@@ -77,14 +79,14 @@ private:
 
   // Maximum number of blocks that this MemoryBlockCache expects.
   const int32_t mMaxBlocks;
 
   // Mutex which controls access to all members below.
   Mutex mMutex;
 
   nsTArray<uint8_t> mBuffer;
-  bool mHasGrown;
+  bool mHasGrown = false;
 };
 
 } // End namespace mozilla.
 
 #endif /* MEMORY_BLOCK_CACHE_H_ */