Bug 1256533 - Use std::deque<int32_t> instead of nsDeque - r?cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Sun, 20 Mar 2016 16:52:34 +1100
changeset 342674 6d92cbb23761a3b82994bf75eefad66442662fef
parent 342671 f14898695ee0dd14615914f3e1401f17df57fdd7
child 516598 ab2ed91ba9e77bb0099b2c468b085daa31f7edfb
push id13433
push usergsquelart@mozilla.com
push dateSun, 20 Mar 2016 05:53:05 +0000
reviewerscpearce
bugs1256533
milestone48.0a1
Bug 1256533 - Use std::deque<int32_t> instead of nsDeque - r?cpearce The queue of pending block indexes was implemented using nsDeque where item pointers were perverted into pure 32-bit numbers, causing a size mismatch on 64-bit platforms, which was picked by VS2015. Note: We're using a deque because we need to implement a 'contains' method, which would be unreasonably-difficult with a pure queue. MozReview-Commit-ID: HpDBIwgSs9
dom/media/FileBlockCache.cpp
dom/media/FileBlockCache.h
--- a/dom/media/FileBlockCache.cpp
+++ b/dom/media/FileBlockCache.cpp
@@ -82,38 +82,46 @@ void FileBlockCache::Close()
       // we're on Mainthread already, *and* the event queues are already
       // shut down, so no events should occur - certainly not creations of
       // new streams.
       mThread->Shutdown();
     }
   }
 }
 
+template<typename Container, typename Value>
+bool
+ContainerContains(const Container& aContainer, const Value& value)
+{
+  return std::find(aContainer.begin(), aContainer.end(), value)
+         != aContainer.end();
+}
+
 nsresult FileBlockCache::WriteBlock(uint32_t aBlockIndex, const uint8_t* aData)
 {
   MonitorAutoLock mon(mDataMonitor);
 
   if (!mIsOpen)
     return NS_ERROR_FAILURE;
 
   // Check if we've already got a pending write scheduled for this block.
   mBlockChanges.EnsureLengthAtLeast(aBlockIndex + 1);
   bool blockAlreadyHadPendingChange = mBlockChanges[aBlockIndex] != nullptr;
   mBlockChanges[aBlockIndex] = new BlockChange(aData);
 
-  if (!blockAlreadyHadPendingChange || !mChangeIndexList.Contains(aBlockIndex)) {
+  if (!blockAlreadyHadPendingChange || !ContainerContains(mChangeIndexList, aBlockIndex)) {
     // We either didn't already have a pending change for this block, or we
     // did but we didn't have an entry for it in mChangeIndexList (we're in the process
     // of writing it and have removed the block's index out of mChangeIndexList
     // in Run() but not finished writing the block to file yet). Add the blocks
     // index to the end of mChangeIndexList to ensure the block is written as
     // as soon as possible.
-    mChangeIndexList.PushBack(aBlockIndex);
+    mChangeIndexList.push_back(aBlockIndex);
   }
-  NS_ASSERTION(mChangeIndexList.Contains(aBlockIndex), "Must have entry for new block");
+  NS_ASSERTION(ContainerContains(mChangeIndexList, aBlockIndex), "Must have entry for new block");
 
   EnsureWriteScheduled();
 
   return NS_OK;
 }
 
 void FileBlockCache::EnsureWriteScheduled()
 {
@@ -191,20 +199,20 @@ nsresult FileBlockCache::MoveBlockInFile
   }
   return WriteBlockToFile(aDestBlockIndex, buf);
 }
 
 nsresult FileBlockCache::Run()
 {
   MonitorAutoLock mon(mDataMonitor);
   NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
-  NS_ASSERTION(!mChangeIndexList.IsEmpty(), "Only dispatch when there's work to do");
+  NS_ASSERTION(!mChangeIndexList.empty(), "Only dispatch when there's work to do");
   NS_ASSERTION(mIsWriteScheduled, "Should report write running or scheduled.");
 
-  while (!mChangeIndexList.IsEmpty()) {
+  while (!mChangeIndexList.empty()) {
     if (!mIsOpen) {
       // We've been closed, abort, discarding unwritten changes.
       mIsWriteScheduled = false;
       return NS_ERROR_FAILURE;
     }
 
     // Process each pending change. We pop the index out of the change
     // list, but leave the BlockChange in mBlockChanges until the change
@@ -212,17 +220,18 @@ nsresult FileBlockCache::Run()
     // we drop mDataMonitor to write will refer to the data's source in
     // memory, rather than the not-yet up to date data written to file.
     // This also ensures we will insert a new index into mChangeIndexList
     // when this happens.
 
     // Hold a reference to the change, in case another change
     // overwrites the mBlockChanges entry for this block while we drop
     // mDataMonitor to take mFileMonitor.
-    int32_t blockIndex = mChangeIndexList.PopFront();
+    int32_t blockIndex = mChangeIndexList.front();
+    mChangeIndexList.pop_front();
     RefPtr<BlockChange> change = mBlockChanges[blockIndex];
     MOZ_ASSERT(change,
                "Change index list should only contain entries for blocks "
                "with changes");
     {
       MonitorAutoUnlock unlock(mDataMonitor);
       MonitorAutoLock lock(mFileMonitor);
       if (change->IsWrite()) {
@@ -321,35 +330,35 @@ nsresult FileBlockCache::MoveBlock(int32
   int32_t sourceIndex = aSourceBlockIndex;
   BlockChange* sourceBlock = nullptr;
   while ((sourceBlock = mBlockChanges[sourceIndex]) &&
           sourceBlock->IsMove()) {
     sourceIndex = sourceBlock->mSourceBlockIndex;
   }
 
   if (mBlockChanges[aDestBlockIndex] == nullptr ||
-      !mChangeIndexList.Contains(aDestBlockIndex)) {
+      !ContainerContains(mChangeIndexList, aDestBlockIndex)) {
     // Only add another entry to the change index list if we don't already
     // have one for this block. We won't have an entry when either there's
     // no pending change for this block, or if there is a pending change for
     // this block and we're in the process of writing it (we've popped the
     // block's index out of mChangeIndexList in Run() but not finished writing
     // the block to file yet.
-    mChangeIndexList.PushBack(aDestBlockIndex);
+    mChangeIndexList.push_back(aDestBlockIndex);
   }
 
   // If the source block hasn't yet been written to file then the dest block
   // simply contains that same write. Resolve this as a write instead.
   if (sourceBlock && sourceBlock->IsWrite()) {
     mBlockChanges[aDestBlockIndex] = new BlockChange(sourceBlock->mData.get());
   } else {
     mBlockChanges[aDestBlockIndex] = new BlockChange(sourceIndex);
   }
 
   EnsureWriteScheduled();
 
-  NS_ASSERTION(mChangeIndexList.Contains(aDestBlockIndex),
+  NS_ASSERTION(ContainerContains(mChangeIndexList, aDestBlockIndex),
     "Should have scheduled block for change");
 
   return NS_OK;
 }
 
 } // End namespace mozilla.
--- a/dom/media/FileBlockCache.h
+++ b/dom/media/FileBlockCache.h
@@ -9,16 +9,17 @@
 
 #include "mozilla/Attributes.h"
 #include "mozilla/Monitor.h"
 #include "mozilla/UniquePtr.h"
 #include "nsTArray.h"
 #include "MediaCache.h"
 #include "nsDeque.h"
 #include "nsThreadUtils.h"
+#include <deque>
 
 struct PRFileDesc;
 
 namespace mozilla {
 
 // Manages file I/O for the media cache. Data comes in over the network
 // via callbacks on the main thread, however we don't want to write the
 // incoming data to the media cache on the main thread, as this could block
@@ -119,48 +120,16 @@ public:
 
   private:
     // Private destructor, to discourage deletion outside of Release():
     ~BlockChange()
     {
     }
   };
 
-  class Int32Queue : private nsDeque {
-  public:
-    int32_t PopFront() {
-      int32_t front = ObjectAt(0);
-      nsDeque::PopFront();
-      return front;
-    }
-
-    void PushBack(int32_t aValue) {
-      nsDeque::Push(reinterpret_cast<void*>(aValue));
-    }
-
-    bool Contains(int32_t aValue) {
-      for (size_t i = 0; i < GetSize(); ++i) {
-        if (ObjectAt(i) == aValue) {
-          return true;
-        }
-      }
-      return false;
-    }
-
-    bool IsEmpty() {
-      return nsDeque::GetSize() == 0;
-    }
-
-  private:
-    int32_t ObjectAt(size_t aIndex) {
-      void* v = nsDeque::ObjectAt(aIndex);
-      return reinterpret_cast<uintptr_t>(v);
-    }
-  };
-
 private:
   int64_t BlockIndexToOffset(int32_t aBlockIndex) {
     return static_cast<int64_t>(aBlockIndex) * BLOCK_SIZE;
   }
 
   // Monitor which controls access to mFD and mFDCurrentPos. Don't hold
   // mDataMonitor while holding mFileMonitor! mFileMonitor must be owned
   // while accessing any of the following data fields or methods.
@@ -197,18 +166,17 @@ private:
   // cached in memory waiting to be written, or this block is the target of a
   // block move.
   nsTArray< RefPtr<BlockChange> > mBlockChanges;
   // Thread upon which block writes and block moves are performed. This is
   // created upon open, and shutdown (asynchronously) upon close (on the
   // main thread).
   nsCOMPtr<nsIThread> mThread;
   // Queue of pending block indexes that need to be written or moved.
-  //AutoTArray<int32_t, 8> mChangeIndexList;
-  Int32Queue mChangeIndexList;
+  std::deque<int32_t> mChangeIndexList;
   // True if we've dispatched an event to commit all pending block changes
   // to file on mThread.
   bool mIsWriteScheduled;
   // True if the writer is ready to write data to file.
   bool mIsOpen;
 };
 
 } // End namespace mozilla.