Issue #2452 - Don't use WebVTTListener, JS wrapper, MediaShutdownManager in stable state

https://bugzilla.mozilla.org/show_bug.cgi?id=1443429
https://bugzilla.mozilla.org/show_bug.cgi?id=1515021

dom/html/HTMLMediaElement.cpp

@@ -53,6 +53,7 @@
 #include "MediaDecoder.h"
 #include "MediaPrefs.h"
 #include "MediaResource.h"
+#include "MediaShutdownManager.h"
 
 #include "nsICategoryManager.h"
 #include "nsIContentPolicy.h"
@@ -3217,6 +3218,13 @@ HTMLMediaElement::HTMLMediaElement(already_AddRefed<mozilla::dom::NodeInfo>& aNo
   NotifyOwnerDocumentActivityChanged();
 
   MOZ_ASSERT(NS_IsMainThread());
+
+  // We initialize the MediaShutdownManager as the HTMLMediaElement is always
+  // constructed on the main thread, and not during stable state.
+  // (MediaShutdownManager make use of nsIAsyncShutdownClient which is written
+  // in JS)
+  MediaShutdownManager::InitStatics();
+
   mWatchManager.Watch(mDownloadSuspendedByCache, &HTMLMediaElement::UpdateReadyStateInternal);
   // Paradoxically, there is a self-edge whereby UpdateReadyStateInternal refuses
   // to run until mReadyState reaches at least HAVE_METADATA by some other means.

dom/html/HTMLTrackElement.cpp

@@ -264,14 +264,16 @@ void
 HTMLTrackElement::DispatchLoadResource()
 {
   if (!mLoadResourceDispatched) {
-    RefPtr<Runnable> r = NewRunnableMethod(this, &HTMLTrackElement::LoadResource);
+    RefPtr<WebVTTListener> listener = new WebVTTListener(this);
+    RefPtr<Runnable> r = NewRunnableMethod<RefPtr<WebVTTListener>>(this,
+        &HTMLTrackElement::LoadResource, std::move(listener));
     nsContentUtils::RunInStableState(r.forget());
     mLoadResourceDispatched = true;
   }
 }
 
 void
-HTMLTrackElement::LoadResource()
+HTMLTrackElement::LoadResource(RefPtr<WebVTTListener>&& aWebVTTListener)
 {
   mLoadResourceDispatched = false;
 
@@ -317,33 +319,46 @@ HTMLTrackElement::LoadResource()
     }
   }
 
-  nsCOMPtr<nsIChannel> channel;
-  nsCOMPtr<nsILoadGroup> loadGroup = OwnerDoc()->GetDocumentLoadGroup();
-  rv = NS_NewChannel(getter_AddRefs(channel),
-                     uri,
-                     static_cast<Element*>(this),
-                     secFlags,
-                     nsIContentPolicy::TYPE_INTERNAL_TRACK,
-                     loadGroup,
-                     nullptr,   // aCallbacks
-                     nsIRequest::LOAD_NORMAL | nsIChannel::LOAD_CLASSIFY_URI);
-
-  NS_ENSURE_TRUE_VOID(NS_SUCCEEDED(rv));
-
-  mListener = new WebVTTListener(this);
+  mListener = std::move(aWebVTTListener);
+  // This will do 6. Set the text track readiness state to loading.
   rv = mListener->LoadResource();
   NS_ENSURE_TRUE_VOID(NS_SUCCEEDED(rv));
-  channel->SetNotificationCallbacks(mListener);
 
-  LOG(LogLevel::Debug, ("opening webvtt channel"));
-  rv = channel->AsyncOpen2(mListener);
-
-  if (NS_FAILED(rv)) {
-    SetReadyState(TextTrackReadyState::FailedToLoad);
+  nsIDocument* doc = OwnerDoc();
+  if (!doc) {
     return;
   }
 
-  mChannel = channel;
+  // 9. End the synchronous section, continuing the remaining steps in parallel.
+  NS_DispatchToMainThread(NS_NewRunnableFunction(
+      [ self = RefPtr<HTMLTrackElement>(this), uri, secFlags ]() {
+        if (!self->mListener) {
+          // Shutdown got called, abort.
+          return;
+        }
+        nsCOMPtr<nsIChannel> channel;
+        nsCOMPtr<nsILoadGroup> loadGroup = self->OwnerDoc()->GetDocumentLoadGroup();
+        nsresult rv = NS_NewChannel(getter_AddRefs(channel),
+                           uri,
+                           static_cast<Element*>(self),
+                           secFlags,
+                           nsIContentPolicy::TYPE_INTERNAL_TRACK,
+                           loadGroup,
+                           nullptr,   // aCallbacks
+                           nsIRequest::LOAD_NORMAL | nsIChannel::LOAD_CLASSIFY_URI);
+        NS_ENSURE_TRUE_VOID(NS_SUCCEEDED(rv));
+
+        channel->SetNotificationCallbacks(self->mListener);
+
+        LOG(LogLevel::Debug, ("opening webvtt channel"));
+        rv = channel->AsyncOpen2(self->mListener);
+
+        if (NS_FAILED(rv)) {
+          self->SetReadyState(TextTrackReadyState::FailedToLoad);
+          return;
+        }
+        self->mChannel = channel;
+      }));
 }
 
 nsresult

dom/html/HTMLTrackElement.h

@@ -118,9 +118,6 @@ protected:
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
   void OnChannelRedirect(nsIChannel* aChannel, nsIChannel* aNewChannel,
                          uint32_t aFlags);
-  // Open a new channel to the HTMLTrackElement's src attribute and call
-  // mListener's LoadResource().
-  void LoadResource();
 
   friend class TextTrackCue;
   friend class WebVTTListener;
@@ -134,6 +131,9 @@ protected:
 
 private:
   void DispatchLoadResource();
+  // Open a new channel to the HTMLTrackElement's src attribute and call
+  // mListener's LoadResource().
+  void LoadResource(RefPtr<WebVTTListener>&& aWebVTTListener);
   bool mLoadResourceDispatched;
 
   RefPtr<WindowDestroyObserver> mWindowDestroyObserver;

dom/html/TextTrackManager.cpp

@@ -602,6 +602,17 @@ void
 TextTrackManager::TimeMarchesOn()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
+  CycleCollectedJSContext* context = CycleCollectedJSContext::Get();
+  if (context && context->IsInStableOrMetaStableState()) {
+    // FireTimeUpdate can be called while at stable state following a
+    // current position change which triggered a state watcher in MediaDecoder
+    // (see bug 1443429).
+    // TimeMarchesOn() will modify JS attributes which is forbidden while in
+    // stable state. So we dispatch a task to perform such operation later
+    // instead.
+    DispatchTimeMarchesOn();
+    return;
+  }
   WEBVTT_LOG("TimeMarchesOn");
   mTimeMarchesOnDispatched = false;
 

dom/media/MediaDecoder.cpp

@@ -457,8 +457,6 @@ MediaDecoder::MediaDecoder(MediaDecoderOwner* aOwner)
   mWatchManager.Watch(mLogicallySeeking, &MediaDecoder::SeekingChanged);
 
   mWatchManager.Watch(mIsAudioDataAudible, &MediaDecoder::NotifyAudibleStateChanged);
-
-  MediaShutdownManager::InitStatics();
 }
 
 #undef INIT_MIRROR

dom/media/WebVTTListener.cpp

@@ -4,6 +4,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebVTTListener.h"
+#include "mozilla/CycleCollectedJSContext.h"
 #include "mozilla/dom/TextTrackCue.h"
 #include "mozilla/dom/TextTrackRegion.h"
 #include "mozilla/dom/VTTRegionBinding.h"
@@ -34,9 +35,22 @@ LazyLogModule gTextTrackLog("TextTrack");
 
 WebVTTListener::WebVTTListener(HTMLTrackElement* aElement)
   : mElement(aElement)
+  , mParserWrapperError(NS_OK)
 {
   MOZ_ASSERT(mElement, "Must pass an element to the callback");
   VTT_LOG("WebVTTListener created.");
+  MOZ_DIAGNOSTIC_ASSERT(
+      CycleCollectedJSContext::Get() &&
+      !CycleCollectedJSContext::Get()->IsInStableOrMetaStableState());
+  mParserWrapper = do_CreateInstance(NS_WEBVTTPARSERWRAPPER_CONTRACTID,
+                                     &mParserWrapperError);
+  if (NS_SUCCEEDED(mParserWrapperError)) {
+    nsPIDOMWindowInner* window = mElement->OwnerDoc()->GetInnerWindow();
+    mParserWrapperError = mParserWrapper->LoadParser(window);
+  }
+  if (NS_SUCCEEDED(mParserWrapperError)) {
+    mParserWrapperError = mParserWrapper->Watch(this);
+  }
 }
 
 WebVTTListener::~WebVTTListener()
@@ -54,16 +68,8 @@ WebVTTListener::GetInterface(const nsIID &aIID,
 nsresult
 WebVTTListener::LoadResource()
 {
-  nsresult rv;
-  mParserWrapper = do_CreateInstance(NS_WEBVTTPARSERWRAPPER_CONTRACTID, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsPIDOMWindowInner* window = mElement->OwnerDoc()->GetInnerWindow();
-  rv = mParserWrapper->LoadParser(window);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = mParserWrapper->Watch(this);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // Exit if we failed to create the WebVTTParserWrapper (vtt.jsm)
+  NS_ENSURE_SUCCESS(mParserWrapperError, mParserWrapperError);
 
   mElement->SetReadyState(TextTrackReadyState::Loading);
   return NS_OK;

dom/media/WebVTTListener.h

@@ -57,6 +57,7 @@ private:
 
   RefPtr<HTMLTrackElement> mElement;
   nsCOMPtr<nsIWebVTTParserWrapper> mParserWrapper;
+  nsresult mParserWrapperError;
 };
 
 } // namespace dom