Bug 1344357 P1 - move the MediaDecoder::mSeekDOMPromise to HTMLMediaElement; r=jwwang
authorKaku Kuo <kaku@mozilla.com>
Mon, 17 Apr 2017 18:25:26 +0800
changeset 353715 8dc519a9f713e12b55ae48e8faba22ab2ddc084f
parent 353714 278c3bf8f7f4a1a8260e1563f18bf93ccb097d24
child 353716 dd1d45aa85253f95070c79149e423dc20e3df345
push id31675
push usercbook@mozilla.com
push dateWed, 19 Apr 2017 08:28:05 +0000
treeherdermozilla-central@245811243903 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1344357
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1344357 P1 - move the MediaDecoder::mSeekDOMPromise to HTMLMediaElement; r=jwwang 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
@@ -2726,16 +2726,19 @@ HTMLMediaElement::Seek(double aTime,
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return nullptr;
   }
 
   // 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));
@@ -7485,10 +7488,38 @@ HTMLMediaElement::MarkAsTainted()
 }
 
 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
@@ -777,16 +777,27 @@ public:
   nsIDocument* GetDocument() const override;
 
   void ConstructMediaTracks(const MediaInfo* aInfo) override;
 
   void RemoveMediaTracks() override;
 
   already_AddRefed<GMPCrashHelper> CreateGMPCrashHelper() override;
 
+  // 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;
@@ -1759,16 +1770,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
@@ -739,58 +739,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()
@@ -1161,26 +1132,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
@@ -636,31 +636,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
@@ -168,14 +168,20 @@ public:
   virtual void ConstructMediaTracks(const MediaInfo* aInfo) = 0;
 
   // Called by the media decoder to removes all audio/video tracks from its
   // owner's track list.
   virtual void RemoveMediaTracks() = 0;
 
   // Called by the media decoder to create a GMPCrashHelper.
   virtual already_AddRefed<GMPCrashHelper> CreateGMPCrashHelper() = 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
@@ -53,12 +53,14 @@ public:
   {
     // Non-DocGroup version for Mock.
     return AbstractThread::MainThread();
   }
   nsIDocument* GetDocument() const { return nullptr; }
   void ConstructMediaTracks(const MediaInfo* aInfo) {}
   void RemoveMediaTracks() {}
   already_AddRefed<GMPCrashHelper> CreateGMPCrashHelper() { return nullptr; }
+  void AsyncResolveSeekDOMPromiseIfExists() override {}
+  void AsyncRejectSeekDOMPromiseIfExists() override {}
 };
 }
 
 #endif