Bug 1371882 - Remove MediaBlockCacheBase::Close() - r=cpearce

The only external use of Close was always followed by an implicit destruction
(by resetting the RefPtr), so we don't need to expose it, and it can be done
from the destructor.

FileBlockCache keeps its Close() function for internal use.
Also, FileBlockCache::mIsOpen is redundant, as it's true iff mThread is not
null.

MozReview-Commit-ID: LV7YVrwJvGG

--HG--
extra : rebase_source : 23decadf249b9e63190b3e19d81edc4a090afcef

dom/media/FileBlockCache.cpp

@@ -33,7 +33,7 @@ CloseFD(PRFileDesc* aFD)
 void
 FileBlockCache::SetCacheFile(PRFileDesc* aFD)
 {
-  LOG("SetFD(aFD=%p) mIsOpen=%d", aFD, mIsOpen);
+  LOG("SetFD(aFD=%p) mThread=%p", aFD, mThread.get());
 
   if (!aFD) {
     // Failed to get a temporary file. Shutdown.
@@ -46,7 +46,7 @@ FileBlockCache::SetCacheFile(PRFileDesc* aFD)
   }
   {
     MutexAutoLock lock(mDataMutex);
-    if (mIsOpen) {
+    if (mThread) {
       // Still open, complete the initialization.
       mInitialized = true;
       if (mIsWriteScheduled) {
@@ -83,7 +83,6 @@ FileBlockCache::Init()
   if (NS_FAILED(rv)) {
     return rv;
   }
-  mIsOpen = true;
 
   if (XRE_IsParentProcess()) {
     RefPtr<FileBlockCache> self = this;
@@ -111,32 +110,28 @@ FileBlockCache::Init()
 }
 
 FileBlockCache::FileBlockCache()
-  : mFileMutex("MediaCache.Writer.IO.Mutex"),
-    mFD(nullptr),
-    mFDCurrentPos(0),
-    mDataMutex("MediaCache.Writer.Data.Mutex"),
-    mIsWriteScheduled(false),
-    mIsReading(false),
-    mIsOpen(false)
+  : mFileMutex("MediaCache.Writer.IO.Mutex")
+  , mFD(nullptr)
+  , mFDCurrentPos(0)
+  , mDataMutex("MediaCache.Writer.Data.Mutex")
+  , mIsWriteScheduled(false)
+  , mIsReading(false)
 {
 }
 
 FileBlockCache::~FileBlockCache()
 {
-  NS_ASSERTION(!mIsOpen, "Should Close() FileBlockCache before destroying");
+  Close();
 }
 
-void FileBlockCache::Close()
+void
+FileBlockCache::Close()
 {
   LOG("Close()");
 
   nsCOMPtr<nsIThread> thread;
   {
     MutexAutoLock mon(mDataMutex);
-    if (!mIsOpen) {
-      return;
-    }
-    mIsOpen = false;
     if (!mThread) {
       return;
     }
@@ -183,8 +178,9 @@ FileBlockCache::WriteBlock(uint32_t aBlockIndex,
 {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mIsOpen)
+  if (!mThread) {
     return NS_ERROR_FAILURE;
+  }
 
   // Check if we've already got a pending write scheduled for this block.
   mBlockChanges.EnsureLengthAtLeast(aBlockIndex + 1);
@@ -210,7 +206,7 @@ FileBlockCache::WriteBlock(uint32_t aBlockIndex,
 void FileBlockCache::EnsureWriteScheduled()
 {
   mDataMutex.AssertCurrentThreadOwns();
-  MOZ_ASSERT(mIsOpen);
+  MOZ_ASSERT(mThread);
 
   if (mIsWriteScheduled || mIsReading) {
     return;
@@ -311,10 +307,10 @@ FileBlockCache::PerformBlockIOs()
   NS_ASSERTION(!mChangeIndexList.empty(), "Only dispatch when there's work to do");
   NS_ASSERTION(mIsWriteScheduled, "Should report write running or scheduled.");
 
-  LOG("Run() mFD=%p mIsOpen=%d", mFD, mIsOpen);
+  LOG("Run() mFD=%p mThread=%p", mFD, mThread.get());
 
   while (!mChangeIndexList.empty()) {
-    if (!mIsOpen) {
+    if (!mThread) {
       // We've been closed, abort, discarding unwritten changes.
       mIsWriteScheduled = false;
       return;
@@ -375,8 +371,9 @@ nsresult FileBlockCache::Read(int64_t aOffset,
 {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mIsOpen || (aOffset / BLOCK_SIZE) > INT32_MAX)
+  if (!mThread || (aOffset / BLOCK_SIZE) > INT32_MAX) {
     return NS_ERROR_FAILURE;
+  }
 
   mIsReading = true;
   auto exitRead = MakeScopeExit([&] {
@@ -444,8 +441,9 @@ nsresult FileBlockCache::MoveBlock(int32_t aSourceBlockIndex, int32_t aDestBlock
 {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mIsOpen)
+  if (!mThread) {
     return NS_ERROR_FAILURE;
+  }
 
   mBlockChanges.EnsureLengthAtLeast(std::max(aSourceBlockIndex, aDestBlockIndex) + 1);
 

dom/media/FileBlockCache.h

@@ -63,9 +63,6 @@ protected:
 public:
   nsresult Init() override;
 
-  // Closes writer, shuts down thread.
-  void Close() override;
-
   // Can be called on any thread. This defers to a non-main thread.
   nsresult WriteBlock(uint32_t aBlockIndex,
                       Span<const uint8_t> aData1,
@@ -139,6 +136,9 @@ private:
 
   void SetCacheFile(PRFileDesc* aFD);
 
+  // Close file in thread and terminate thread.
+  void Close();
+
   // Performs block writes and block moves on its own thread.
   void PerformBlockIOs();
 
@@ -191,8 +191,6 @@ private:
   // True when a read is happening. Pending writes may be postponed, to give
   // higher priority to reads (which may be blocking the caller).
   bool mIsReading;
-  // True if the writer is ready to enqueue writes.
-  bool mIsOpen;
   // True if we've got a temporary file descriptor. Note: we don't use mFD
   // directly as that's synchronized via mFileMutex and we need to make
   // decisions about whether we can write while holding mDataMutex.

dom/media/MediaBlockCacheBase.h

@@ -50,9 +50,6 @@ protected:
 public:
   virtual nsresult Init() = 0;
 
-  // Closes writer, shuts down thread.
-  virtual void Close() = 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,

dom/media/MediaCache.cpp

@@ -279,10 +279,6 @@ protected:
     NS_ASSERTION(mStreams.IsEmpty(), "Stream(s) still open!");
     Truncate();
     NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
-    if (mBlockCache) {
-      mBlockCache->Close();
-      mBlockCache = nullptr;
-    }
     if (mContentLength <= 0) {
       // Only gather "MEDIACACHE" telemetry for the file-based cache.
       LOG("MediaCache::~MediaCache(this=%p) MEDIACACHE_WATERMARK_KB=%u",
@@ -708,10 +704,7 @@ MediaCache::Flush()
   // Truncate file, close it, and reopen
   Truncate();
   NS_ASSERTION(mIndex.Length() == 0, "Blocks leaked?");
-  if (mBlockCache) {
-    mBlockCache->Close();
-    mBlockCache = nullptr;
-  }
+  mBlockCache = nullptr;
   Init();
 }
 

dom/media/MemoryBlockCache.cpp

@@ -46,7 +46,6 @@ MemoryBlockCache::MemoryBlockCache(int64_t aContentLength)
 
 MemoryBlockCache::~MemoryBlockCache()
 {
-  MOZ_ASSERT(mBuffer.IsEmpty());
 }
 
 bool
@@ -86,14 +85,6 @@ MemoryBlockCache::Init()
   return NS_OK;
 }
 
-void
-MemoryBlockCache::Close()
-{
-  LOG("@%p Close()", this);
-  MutexAutoLock lock(mMutex);
-  mBuffer.SetLength(0);
-}
-
 nsresult
 MemoryBlockCache::WriteBlock(uint32_t aBlockIndex,
                              Span<const uint8_t> aData1,

dom/media/MemoryBlockCache.h

@@ -41,9 +41,6 @@ public:
   // Allocate initial buffer.
   virtual nsresult Init() override;
 
-  // Empty buffer.
-  virtual void Close() override;
-
   // Can be called on any thread.
   virtual nsresult WriteBlock(uint32_t aBlockIndex,
                               Span<const uint8_t> aData1,