Bug 1368952: P2. Remove monitor from SourceBufferResource. r=gerald

A SourceBufferResource is only ever used on the same taskqueue. It doesn't require to be thread-safe.

MozReview-Commit-ID: HREUvzaHyD9

--HG--
extra : rebase_source : dfa9d5223e892f95091fb522a1d768167d874a0a

dom/media/mediasource/SourceBufferResource.cpp

@@ -12,6 +12,7 @@
 #include "nsISupports.h"
 #include "mozilla/Logging.h"
 #include "mozilla/SizePrintfMacros.h"
+#include "mozilla/TaskQueue.h"
 #include "MediaData.h"
 
 mozilla::LogModule* GetSourceBufferResourceLog()
@@ -28,11 +29,9 @@ namespace mozilla {
 nsresult
 SourceBufferResource::Close()
 {
-  ReentrantMonitorAutoEnter mon(mMonitor);
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("Close");
-  //MOZ_ASSERT(!mClosed);
   mClosed = true;
-  mon.NotifyAll();
   return NS_OK;
 }
 
@@ -44,21 +43,16 @@ SourceBufferResource::ReadAt(int64_t aOffset,
 {
   SBR_DEBUG("ReadAt(aOffset=%" PRId64 ", aBuffer=%p, aCount=%u, aBytes=%p)",
             aOffset, aBytes, aCount, aBytes);
-  ReentrantMonitorAutoEnter mon(mMonitor);
-  return ReadAtInternal(
-    aOffset, aBuffer, aCount, aBytes, /* aMayBlock = */ true);
+  return ReadAtInternal(aOffset, aBuffer, aCount, aBytes);
 }
 
 nsresult
 SourceBufferResource::ReadAtInternal(int64_t aOffset,
                                      char* aBuffer,
                                      uint32_t aCount,
-                                     uint32_t* aBytes,
-                                     bool aMayBlock)
+                                     uint32_t* aBytes)
 {
-  mMonitor.AssertCurrentThreadIn();
-
-  MOZ_ASSERT_IF(!aMayBlock, aBytes);
+  MOZ_ASSERT(OnTaskQueue());
 
   if (mClosed ||
       aOffset < 0 ||
@@ -67,18 +61,6 @@ SourceBufferResource::ReadAtInternal(int64_t aOffset,
     return NS_ERROR_FAILURE;
   }
 
-  while (aMayBlock &&
-         !mEnded &&
-         aOffset + aCount > GetLength()) {
-    SBR_DEBUGV("waiting for data");
-    mMonitor.Wait();
-    // The callers of this function should have checked this, but it's
-    // possible that we had an eviction while waiting on the monitor.
-    if (uint64_t(aOffset) < mInputBuffer.GetOffset()) {
-      return NS_ERROR_FAILURE;
-    }
-  }
-
   uint32_t available = GetLength() - aOffset;
   uint32_t count = std::min(aCount, available);
 
@@ -112,10 +94,8 @@ SourceBufferResource::ReadFromCache(char* aBuffer,
 {
   SBR_DEBUG("ReadFromCache(aBuffer=%p, aOffset=%" PRId64 ", aCount=%u)",
             aBuffer, aOffset, aCount);
-  ReentrantMonitorAutoEnter mon(mMonitor);
   uint32_t bytesRead;
-  nsresult rv = ReadAtInternal(
-    aOffset, aBuffer, aCount, &bytesRead, /* aMayBlock = */ false);
+  nsresult rv = ReadAtInternal(aOffset, aBuffer, aCount, &bytesRead);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // ReadFromCache return failure if not all the data is cached.
@@ -127,57 +107,46 @@ SourceBufferResource::EvictData(uint64_t aPlaybackOffset,
                                 int64_t aThreshold,
                                 ErrorResult& aRv)
 {
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("EvictData(aPlaybackOffset=%" PRIu64 ","
             "aThreshold=%" PRId64 ")", aPlaybackOffset, aThreshold);
-  ReentrantMonitorAutoEnter mon(mMonitor);
   uint32_t result = mInputBuffer.Evict(aPlaybackOffset, aThreshold, aRv);
-  if (result > 0) {
-    // Wake up any waiting threads in case a ReadInternal call
-    // is now invalid.
-    mon.NotifyAll();
-  }
   return result;
 }
 
 void
 SourceBufferResource::EvictBefore(uint64_t aOffset, ErrorResult& aRv)
 {
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("EvictBefore(aOffset=%" PRIu64 ")", aOffset);
-  ReentrantMonitorAutoEnter mon(mMonitor);
 
   mInputBuffer.EvictBefore(aOffset, aRv);
-
-  // Wake up any waiting threads in case a ReadInternal call
-  // is now invalid.
-  mon.NotifyAll();
 }
 
 uint32_t
 SourceBufferResource::EvictAll()
 {
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("EvictAll()");
-  ReentrantMonitorAutoEnter mon(mMonitor);
   return mInputBuffer.EvictAll();
 }
 
 void
 SourceBufferResource::AppendData(MediaByteBuffer* aData)
 {
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("AppendData(aData=%p, aLength=%" PRIuSIZE ")",
             aData->Elements(), aData->Length());
-  ReentrantMonitorAutoEnter mon(mMonitor);
   mInputBuffer.AppendItem(aData);
   mEnded = false;
-  mon.NotifyAll();
 }
 
 void
 SourceBufferResource::Ended()
 {
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("");
-  ReentrantMonitorAutoEnter mon(mMonitor);
   mEnded = true;
-  mon.NotifyAll();
 }
 
 SourceBufferResource::~SourceBufferResource()
@@ -187,7 +156,9 @@ SourceBufferResource::~SourceBufferResource()
 
 SourceBufferResource::SourceBufferResource(const MediaContainerType& aType)
   : mType(aType)
-  , mMonitor("mozilla::SourceBufferResource::mMonitor")
+#if defined(DEBUG)
+  , mTaskQueue(AbstractThread::GetCurrent()->AsTaskQueue())
+#endif
   , mOffset(0)
   , mClosed(false)
   , mEnded(false)
@@ -195,6 +166,19 @@ SourceBufferResource::SourceBufferResource(const MediaContainerType& aType)
   SBR_DEBUG("");
 }
 
+#if defined(DEBUG)
+AbstractThread*
+SourceBufferResource::GetTaskQueue() const
+{
+  return mTaskQueue;
+}
+bool
+SourceBufferResource::OnTaskQueue() const
+{
+  return !GetTaskQueue() || GetTaskQueue()->IsCurrentThreadIn();
+}
+#endif
+
 #undef SBR_DEBUG
 #undef SBR_DEBUGV
 } // namespace mozilla

dom/media/mediasource/SourceBufferResource.h

@@ -11,7 +11,6 @@
 #include "MediaResource.h"
 #include "ResourceQueue.h"
 #include "mozilla/Attributes.h"
-#include "mozilla/ReentrantMonitor.h"
 #include "nsCOMPtr.h"
 #include "nsError.h"
 #include "nsIPrincipal.h"
@@ -27,6 +26,7 @@ namespace mozilla {
 
 class MediaDecoder;
 class MediaByteBuffer;
+class TaskQueue;
 
 namespace dom {
 
@@ -34,6 +34,7 @@ class SourceBuffer;
 
 } // namespace dom
 
+// SourceBufferResource is not thread safe.
 class SourceBufferResource final : public MediaResource
 {
 public:
@@ -72,7 +73,7 @@ public:
   int64_t GetLength() override { return mInputBuffer.GetLength(); }
   int64_t GetNextCachedData(int64_t aOffset) override
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     MOZ_ASSERT(aOffset >= 0);
     if (uint64_t(aOffset) < mInputBuffer.GetOffset()) {
       return mInputBuffer.GetOffset();
@@ -113,7 +114,7 @@ public:
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     if (mInputBuffer.GetLength()) {
       aRanges += MediaByteRange(mInputBuffer.GetOffset(),
                                 mInputBuffer.GetLength());
@@ -125,7 +126,7 @@ public:
 
   size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const override
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
 
     size_t size = MediaResource::SizeOfExcludingThis(aMallocSizeOf);
     size += mType.SizeOfExcludingThis(aMallocSizeOf);
@@ -149,7 +150,7 @@ public:
   void Ended();
   bool IsEnded()
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     return mEnded;
   }
   // Remove data from resource if it holds more than the threshold reduced by
@@ -166,7 +167,7 @@ public:
   // Returns the amount of data currently retained by this resource.
   int64_t GetSize()
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     return mInputBuffer.GetLength() - mInputBuffer.GetOffset();
   }
 
@@ -182,16 +183,15 @@ private:
   nsresult ReadAtInternal(int64_t aOffset,
                           char* aBuffer,
                           uint32_t aCount,
-                          uint32_t* aBytes,
-                          bool aMayBlock);
-
+                          uint32_t* aBytes);
   const MediaContainerType mType;
 
-  // Provides synchronization between SourceBuffers and InputAdapters.
-  // Protects all of the member variables below.  Read() will await a
-  // Notify() (from Seek, AppendData, Ended, or Close) when insufficient
-  // data is available in mData.
-  mutable ReentrantMonitor mMonitor;
+#if defined(DEBUG)
+  const RefPtr<TaskQueue> mTaskQueue;
+  // TaskQueue methods and objects.
+  AbstractThread* GetTaskQueue() const;
+  bool OnTaskQueue() const;
+#endif
 
   // The buffer holding resource data.
   ResourceQueue mInputBuffer;

dom/media/mediasource/TrackBuffersManager.cpp

@@ -202,6 +202,7 @@ TrackBuffersManager::ProcessTasks()
       CompleteResetParserState();
       break;
     case Type::Detach:
+      mCurrentInputBuffer = nullptr;
       mTaskQueue = nullptr;
       MOZ_DIAGNOSTIC_ASSERT(mQueue.Length() == 0,
                             "Detach task must be the last");