Bug 1344357 - Part 1: Move the MediaDecoder::mSeekDOMPromise to HTMLMediaElement. r=jwwang, a=gchang
authorKaku Kuo <kaku@mozilla.com>
Mon, 17 Apr 2017 18:25:26 +0800
changeset 396044 773644d29d67abebf17a4a94fc06e13ee9765d4a
parent 396043 e65cc09cdf877dc4a775b8606b386c13ba0d6e39
child 396045 a8b2d8cd033a69370c97d9221d7ddf10a48cab32
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang, gchang
bugs1344357
milestone54.0
Bug 1344357 - Part 1: Move the MediaDecoder::mSeekDOMPromise to HTMLMediaElement. r=jwwang, a=gchang There was a cycle amoung a window object -> a HTMLMediaElement -> a MediaDecoder -> a Promise (-> back to the window object). And we have no way to break the cycle since the MediaDecoder does not participate into the collection. By moving the Promise form MediaDecoder to HTMLMediaElement, we will be able to break the cycle since the HTMLMediaElement is in the collection. We'll implement the cycle collection in the next patch. MozReview-Commit-ID: CyVXBl6IMI3
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/MediaDecoderOwner.h
dom/media/gtest/MockMediaDecoderOwner.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -2696,16 +2696,19 @@ HTMLMediaElement::Seek(double aTime,
   nsresult rv = mDecoder->Seek(aTime, aSeekType, promise);
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
   }
 
   // We changed whether we're seeking so we need to AddRemoveSelfReference.
   AddRemoveSelfReference();
 
+  // Keep the DOM promise.
+  mSeekDOMPromise = promise;
+
   return promise.forget();
 }
 
 NS_IMETHODIMP HTMLMediaElement::SetCurrentTime(double aCurrentTime)
 {
   // Detect for a NaN and invalid values.
   if (mozilla::IsNaN(aCurrentTime)) {
     LOG(LogLevel::Debug, ("%p SetCurrentTime(%f) failed: bad time", this, aCurrentTime));
@@ -7362,10 +7365,38 @@ HTMLMediaElement::GetEMEInfo(nsString& a
 }
 
 bool HasDebuggerPrivilege(JSContext* aCx, JSObject* aObj)
 {
   return nsContentUtils::CallerHasPermission(aCx,
                                              NS_LITERAL_STRING("debugger"));
  }
 
+void
+HTMLMediaElement::AsyncResolveSeekDOMPromiseIfExists()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  if (mSeekDOMPromise) {
+    RefPtr<dom::Promise> promise = mSeekDOMPromise.forget();
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
+      promise->MaybeResolveWithUndefined();
+    });
+    mAbstractMainThread->Dispatch(r.forget());
+    mSeekDOMPromise = nullptr;
+  }
+}
+
+void
+HTMLMediaElement::AsyncRejectSeekDOMPromiseIfExists()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  if (mSeekDOMPromise) {
+    RefPtr<dom::Promise> promise = mSeekDOMPromise.forget();
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
+      promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
+    });
+    mAbstractMainThread->Dispatch(r.forget());
+    mSeekDOMPromise = nullptr;
+  }
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -757,16 +757,27 @@ public:
   enum class CallerAPI {
     DRAW_IMAGE,
     CREATE_PATTERN,
     CREATE_IMAGEBITMAP,
     CAPTURE_STREAM,
   };
   void MarkAsContentSource(CallerAPI aAPI);
 
+  // The promise resolving/rejection is queued as a "micro-task" which will be
+  // handled immediately after the current JS task and before any pending JS
+  // tasks.
+  // At the time we are going to resolve/reject a promise, the "seeking" event
+  // task should already be queued but might yet be processed, so we queue one
+  // more task to file the promise resolving/rejection micro-tasks
+  // asynchronously to make sure that the micro-tasks are processed after the
+  // "seeking" event task.
+  void AsyncResolveSeekDOMPromiseIfExists() override;
+  void AsyncRejectSeekDOMPromiseIfExists() override;
+
 protected:
   virtual ~HTMLMediaElement();
 
   class AudioChannelAgentCallback;
   class ChannelLoader;
   class ErrorSink;
   class MediaLoadListener;
   class MediaStreamTracksAvailableCallback;
@@ -1728,16 +1739,21 @@ private:
   nsTArray<RefPtr<Promise>> mPendingPlayPromises;
 
   // A list of already-dispatched but not yet run
   // nsResolveOrRejectPendingPlayPromisesRunners.
   // Runners whose Run() method is called remove themselves from this list.
   // We keep track of these because the load algorithm resolves/rejects all
   // already-dispatched pending play promises.
   nsTArray<nsResolveOrRejectPendingPlayPromisesRunner*> mPendingPlayPromisesRunners;
+
+  // A pending seek promise which is created at Seek() method call and is
+  // resolved/rejected at AsyncResolveSeekDOMPromiseIfExists()/
+  // AsyncRejectSeekDOMPromiseIfExists() methods.
+  RefPtr<dom::Promise> mSeekDOMPromise;
 };
 
 // Check if the context is chrome or has the debugger permission
 bool HasDebuggerPrivilege(JSContext* aCx, JSObject* aObj);
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -743,58 +743,29 @@ MediaDecoder::Seek(double aTime, SeekTar
   if (mPlayState == PLAY_STATE_ENDED) {
     PinForSeek();
     ChangeState(GetOwner()->GetPaused() ? PLAY_STATE_PAUSED : PLAY_STATE_PLAYING);
   }
   return NS_OK;
 }
 
 void
-MediaDecoder::AsyncResolveSeekDOMPromiseIfExists()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  if (mSeekDOMPromise) {
-    RefPtr<dom::Promise> promise = mSeekDOMPromise;
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-      promise->MaybeResolveWithUndefined();
-    });
-    mAbstractMainThread->Dispatch(r.forget());
-    mSeekDOMPromise = nullptr;
-  }
-}
-
-void
-MediaDecoder::AsyncRejectSeekDOMPromiseIfExists()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  if (mSeekDOMPromise) {
-    RefPtr<dom::Promise> promise = mSeekDOMPromise;
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
-      promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
-    });
-    mAbstractMainThread->Dispatch(r.forget());
-    mSeekDOMPromise = nullptr;
-  }
-}
-
-void
 MediaDecoder::DiscardOngoingSeekIfExists()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mSeekRequest.DisconnectIfExists();
-  AsyncRejectSeekDOMPromiseIfExists();
+  GetOwner()->AsyncRejectSeekDOMPromiseIfExists();
 }
 
 void
 MediaDecoder::CallSeek(const SeekTarget& aTarget, dom::Promise* aPromise)
 {
   MOZ_ASSERT(NS_IsMainThread());
   DiscardOngoingSeekIfExists();
 
-  mSeekDOMPromise = aPromise;
   mDecoderStateMachine->InvokeSeek(aTarget)
   ->Then(mAbstractMainThread, __func__, this,
          &MediaDecoder::OnSeekResolved, &MediaDecoder::OnSeekRejected)
   ->Track(mSeekRequest);
 }
 
 double
 MediaDecoder::GetCurrentTime()
@@ -1186,26 +1157,26 @@ MediaDecoder::OnSeekResolved()
     UnpinForSeek();
     mLogicallySeeking = false;
   }
 
   // Ensure logical position is updated after seek.
   UpdateLogicalPositionInternal();
 
   GetOwner()->SeekCompleted();
-  AsyncResolveSeekDOMPromiseIfExists();
+  GetOwner()->AsyncResolveSeekDOMPromiseIfExists();
 }
 
 void
 MediaDecoder::OnSeekRejected()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mSeekRequest.Complete();
   mLogicallySeeking = false;
-  AsyncRejectSeekDOMPromiseIfExists();
+  GetOwner()->AsyncRejectSeekDOMPromiseIfExists();
 }
 
 void
 MediaDecoder::SeekingStarted()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
   GetOwner()->SeekStarted();
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -615,31 +615,20 @@ private:
   RefPtr<MediaDecoderStateMachine> mDecoderStateMachine;
 
   RefPtr<ResourceCallback> mResourceCallback;
 
   MozPromiseHolder<CDMProxyPromise> mCDMProxyPromiseHolder;
   RefPtr<CDMProxyPromise> mCDMProxyPromise;
 
 protected:
-  // The promise resolving/rejection is queued as a "micro-task" which will be
-  // handled immediately after the current JS task and before any pending JS
-  // tasks.
-  // At the time we are going to resolve/reject a promise, the "seeking" event
-  // task should already be queued but might yet be processed, so we queue one
-  // more task to file the promise resolving/rejection micro-tasks
-  // asynchronously to make sure that the micro-tasks are processed after the
-  // "seeking" event task.
-  void AsyncResolveSeekDOMPromiseIfExists();
-  void AsyncRejectSeekDOMPromiseIfExists();
   void DiscardOngoingSeekIfExists();
   virtual void CallSeek(const SeekTarget& aTarget, dom::Promise* aPromise);
 
   MozPromiseRequestHolder<SeekPromise> mSeekRequest;
-  RefPtr<dom::Promise> mSeekDOMPromise;
 
   // True when seeking or otherwise moving the play position around in
   // such a manner that progress event data is inaccurate. This is set
   // during seek and duration operations to prevent the progress indicator
   // from jumping around. Read/Write on the main thread only.
   bool mIgnoreProgressData;
 
   // True if the stream is infinite (e.g. a webradio).
--- a/dom/media/MediaDecoderOwner.h
+++ b/dom/media/MediaDecoderOwner.h
@@ -152,14 +152,20 @@ public:
   // reference to the decoder to prevent further calls into the decoder.
   virtual void NotifyXPCOMShutdown() = 0;
 
   // Dispatches a "encrypted" event to the HTMLMediaElement, with the
   // provided init data. Actual dispatch may be delayed until HAVE_METADATA.
   // Main thread only.
   virtual void DispatchEncrypted(const nsTArray<uint8_t>& aInitData,
                                  const nsAString& aInitDataType) = 0;
+
+  // Called by the media decoder to notify the owner to resolve a seek promise.
+  virtual void AsyncResolveSeekDOMPromiseIfExists() = 0;
+
+  // Called by the media decoder to notify the owner to reject a seek promise.
+  virtual void AsyncRejectSeekDOMPromiseIfExists() = 0;
 };
 
 } // namespace mozilla
 
 #endif
 
--- a/dom/media/gtest/MockMediaDecoderOwner.h
+++ b/dom/media/gtest/MockMediaDecoderOwner.h
@@ -49,12 +49,14 @@ public:
   }
   void SetAudibleState(bool aAudible) override {}
   void NotifyXPCOMShutdown() override {}
   AbstractThread* AbstractMainThread() const override
   {
     // Non-DocGroup version for Mock.
     return AbstractThread::MainThread();
   }
+  void AsyncResolveSeekDOMPromiseIfExists() override {}
+  void AsyncRejectSeekDOMPromiseIfExists() override {}
 };
 }
 
 #endif