Bug 1368952: P2. Remove monitor from SourceBufferResource. r?gerald draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 31 May 2017 11:08:06 +0200
changeset 586922 331c95344060c4afcba5739e83413e1365d7abcf
parent 586911 41cfcc3ad6c04d32e4b25a27c3384ba36233cdce
child 631156 5de05e72c28074b9479c16c7a4e213874374b056
push id61584
push userbmo:jyavenard@mozilla.com
push dateWed, 31 May 2017 09:41:16 +0000
reviewersgerald
bugs1368952
milestone55.0a1
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
dom/media/mediasource/SourceBufferResource.cpp
dom/media/mediasource/SourceBufferResource.h
dom/media/mediasource/TrackBuffersManager.cpp
--- a/dom/media/mediasource/SourceBufferResource.cpp
+++ b/dom/media/mediasource/SourceBufferResource.cpp
@@ -7,83 +7,65 @@
 #include "SourceBufferResource.h"
 
 #include <algorithm>
 
 #include "nsISeekableStream.h"
 #include "nsISupports.h"
 #include "mozilla/Logging.h"
 #include "mozilla/SizePrintfMacros.h"
+#include "mozilla/TaskQueue.h"
 #include "MediaData.h"
 
 mozilla::LogModule* GetSourceBufferResourceLog()
 {
   static mozilla::LazyLogModule sLogModule("SourceBufferResource");
   return sLogModule;
 }
 
 #define SBR_DEBUG(arg, ...) MOZ_LOG(GetSourceBufferResourceLog(), mozilla::LogLevel::Debug, ("SourceBufferResource(%p:%s)::%s: " arg, this, mType.OriginalString().Data(), __func__, ##__VA_ARGS__))
 #define SBR_DEBUGV(arg, ...) MOZ_LOG(GetSourceBufferResourceLog(), mozilla::LogLevel::Verbose, ("SourceBufferResource(%p:%s)::%s: " arg, this, mType.OriginalString().Data(), __func__, ##__VA_ARGS__))
 
 namespace mozilla {
 
 nsresult
 SourceBufferResource::Close()
 {
-  ReentrantMonitorAutoEnter mon(mMonitor);
+  MOZ_ASSERT(OnTaskQueue());
   SBR_DEBUG("Close");
-  //MOZ_ASSERT(!mClosed);
   mClosed = true;
-  mon.NotifyAll();
   return NS_OK;
 }
 
 nsresult
 SourceBufferResource::ReadAt(int64_t aOffset,
                              char* aBuffer,
                              uint32_t aCount,
                              uint32_t* aBytes)
 {
   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 ||
       uint64_t(aOffset) < mInputBuffer.GetOffset() ||
       aOffset > GetLength()) {
     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);
 
   // Keep the position of the last read to have Tell() approximately give us
   // the position we're up to in the stream.
   mOffset = aOffset + count;
 
   SBR_DEBUGV("offset=%" PRId64 " GetLength()=%" PRId64
@@ -107,94 +89,96 @@ SourceBufferResource::ReadAtInternal(int
 
 nsresult
 SourceBufferResource::ReadFromCache(char* aBuffer,
                                     int64_t aOffset,
                                     uint32_t aCount)
 {
   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.
   return bytesRead == aCount ? NS_OK : NS_ERROR_FAILURE;
 }
 
 uint32_t
 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()
 {
   SBR_DEBUG("");
 }
 
 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)
 {
   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
--- a/dom/media/mediasource/SourceBufferResource.h
+++ b/dom/media/mediasource/SourceBufferResource.h
@@ -6,39 +6,40 @@
 
 #ifndef MOZILLA_SOURCEBUFFERRESOURCE_H_
 #define MOZILLA_SOURCEBUFFERRESOURCE_H_
 
 #include "MediaCache.h"
 #include "MediaResource.h"
 #include "ResourceQueue.h"
 #include "mozilla/Attributes.h"
-#include "mozilla/ReentrantMonitor.h"
 #include "nsCOMPtr.h"
 #include "nsError.h"
 #include "nsIPrincipal.h"
 #include "nsTArray.h"
 #include "nscore.h"
 #include "mozilla/Logging.h"
 
 #define UNIMPLEMENTED() { /* Logging this is too spammy to do by default */ }
 
 class nsIStreamListener;
 
 namespace mozilla {
 
 class MediaDecoder;
 class MediaByteBuffer;
+class TaskQueue;
 
 namespace dom {
 
 class SourceBuffer;
 
 } // namespace dom
 
+// SourceBufferResource is not thread safe.
 class SourceBufferResource final : public MediaResource
 {
 public:
   explicit SourceBufferResource(const MediaContainerType& aType);
   nsresult Close() override;
   void Suspend(bool aCloseImmediately) override { UNIMPLEMENTED(); }
   void Resume() override { UNIMPLEMENTED(); }
   already_AddRefed<nsIPrincipal> GetCurrentPrincipal() override
@@ -67,17 +68,17 @@ public:
   {
     UNIMPLEMENTED();
     *aIsReliable = false;
     return 0;
   }
   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();
     } else if (aOffset == GetLength()) {
       return -1;
     }
     return aOffset;
   }
@@ -108,29 +109,29 @@ public:
   nsresult Open(nsIStreamListener** aStreamListener) override
   {
     UNIMPLEMENTED();
     return NS_ERROR_FAILURE;
   }
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     if (mInputBuffer.GetLength()) {
       aRanges += MediaByteRange(mInputBuffer.GetOffset(),
                                 mInputBuffer.GetLength());
     }
     return NS_OK;
   }
 
   const MediaContainerType& GetContentType() const override { return mType; }
 
   size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const override
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
 
     size_t size = MediaResource::SizeOfExcludingThis(aMallocSizeOf);
     size += mType.SizeOfExcludingThis(aMallocSizeOf);
     size += mInputBuffer.SizeOfExcludingThis(aMallocSizeOf);
 
     return size;
   }
 
@@ -144,59 +145,58 @@ public:
     return false;
   }
 
   // Used by SourceBuffer.
   void AppendData(MediaByteBuffer* aData);
   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
   // the given number of bytes. Returns amount evicted.
   uint32_t EvictData(uint64_t aPlaybackOffset, int64_t aThresholdReduct,
                      ErrorResult& aRv);
 
   // Remove data from resource before the given offset.
   void EvictBefore(uint64_t aOffset, ErrorResult& aRv);
 
   // Remove all data from the resource
   uint32_t EvictAll();
 
   // Returns the amount of data currently retained by this resource.
   int64_t GetSize()
   {
-    ReentrantMonitorAutoEnter mon(mMonitor);
+    MOZ_ASSERT(OnTaskQueue());
     return mInputBuffer.GetLength() - mInputBuffer.GetOffset();
   }
 
 #if defined(DEBUG)
   void Dump(const char* aPath)
   {
     mInputBuffer.Dump(aPath);
   }
 #endif
 
 private:
   virtual ~SourceBufferResource();
   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;
 
   uint64_t mOffset;
   bool mClosed;
   bool mEnded;
 };
--- a/dom/media/mediasource/TrackBuffersManager.cpp
+++ b/dom/media/mediasource/TrackBuffersManager.cpp
@@ -197,16 +197,17 @@ TrackBuffersManager::ProcessTasks()
       break;
     case Type::Abort:
       // not handled yet, and probably never.
       break;
     case Type::Reset:
       CompleteResetParserState();
       break;
     case Type::Detach:
+      mCurrentInputBuffer = nullptr;
       mTaskQueue = nullptr;
       MOZ_DIAGNOSTIC_ASSERT(mQueue.Length() == 0,
                             "Detach task must be the last");
       return;
     default:
       NS_WARNING("Invalid Task");
   }
   GetTaskQueue()->Dispatch(NewRunnableMethod(this, &TrackBuffersManager::ProcessTasks));