Backed out 4 changesets (bug 1531863) for mochitest failures at test_videocontrols_vtt.html. CLOSED TREE
authorBrindusan Cristian <cbrindusan@mozilla.com>
Fri, 08 Mar 2019 02:34:45 +0200
changeset 520932 b653c9d4de17e17b4f7273ed192d10414222f88b
parent 520931 be438ca69c7e957273a358cf2891207eaf2f2d53
child 520933 686e97ccf53ab544398a63a3734b5a806a536173
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1531863
milestone67.0a1
backs oute2e2b325344fcda75fe015b3de5ba25bd53613ba
5be609c166652d5861cc5a8bac0265c3bf98d242
21bafd01b4389ff72d26f78f28079a6585708b4c
c9628b2b89b23bbbf49d9854f9c1569c614558e2
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
Backed out 4 changesets (bug 1531863) for mochitest failures at test_videocontrols_vtt.html. CLOSED TREE Backed out changeset e2e2b325344f (bug 1531863) Backed out changeset 5be609c16665 (bug 1531863) Backed out changeset 21bafd01b438 (bug 1531863) Backed out changeset c9628b2b89b2 (bug 1531863)
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/media/TextTrack.cpp
dom/media/TextTrack.h
dom/media/TextTrackList.cpp
dom/media/TextTrackList.h
testing/web-platform/meta/html/semantics/embedded-content/media-elements/track/track-element/track-change-event.html.ini
testing/web-platform/tests/html/semantics/embedded-content/media-elements/track/track-element/track-change-event.html
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -2705,20 +2705,16 @@ already_AddRefed<Promise> HTMLMediaEleme
                                                  ErrorResult& aRv) {
   // Note: Seek is called both by synchronous code that expects errors thrown in
   // aRv, as well as asynchronous code that expects a promise. Make sure all
   // synchronous errors are returned using aRv, not promise rejections.
 
   // aTime should be non-NaN.
   MOZ_ASSERT(!mozilla::IsNaN(aTime));
 
-  // Seeking step1, Set the media element's show poster flag to false.
-  // https://html.spec.whatwg.org/multipage/media.html#dom-media-seek
-  mShowPoster = false;
-
   RefPtr<Promise> promise = CreateDOMPromise(aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   // Detect if user has interacted with element by seeking so that
   // play will not be blocked when initiated by a script.
   if (EventStateManager::IsHandlingUserInput()) {
@@ -3502,18 +3498,17 @@ HTMLMediaElement::HTMLMediaElement(
       mMainThreadEventTarget(OwnerDoc()->EventTargetFor(TaskCategory::Other)),
       mAbstractMainThread(
           OwnerDoc()->AbstractMainThreadFor(TaskCategory::Other)),
       mShutdownObserver(new ShutdownObserver),
       mPlayed(new TimeRanges(ToSupports(OwnerDoc()))),
       mPaused(true, "HTMLMediaElement::mPaused"),
       mErrorSink(new ErrorSink(this)),
       mAudioChannelWrapper(new AudioChannelAgentCallback(this)),
-      mSink(MakePair(nsString(), RefPtr<AudioDeviceInfo>())),
-      mShowPoster(IsVideo()) {
+      mSink(MakePair(nsString(), RefPtr<AudioDeviceInfo>())) {
   MOZ_ASSERT(mMainThreadEventTarget);
   MOZ_ASSERT(mAbstractMainThread);
   // Please don't add anything to this constructor or the initialization
   // list that can cause AddRef to be called. This prevents subclasses
   // from overriding AddRef in a way that works with our refcount
   // logging mechanisms. Put these things inside of the ::Init method
   // instead.
 }
@@ -3808,22 +3803,16 @@ void HTMLMediaElement::PlayInternal(bool
   if (oldPaused) {
     // 6.1. Change the value of paused to false. (Already done.)
     // This step is uplifted because the "block-media-playback" feature needs
     // the mPaused to be false before UpdateAudioChannelPlayingState() being
     // called.
 
     // 6.2. If the show poster flag is true, set the element's show poster flag
     //      to false and run the time marches on steps.
-    if (mShowPoster) {
-      mShowPoster = false;
-      if (mTextTrackManager) {
-        mTextTrackManager->TimeMarchesOn();
-      }
-    }
 
     // 6.3. Queue a task to fire a simple event named play at the element.
     DispatchAsyncEvent(NS_LITERAL_STRING("play"));
 
     // 6.4. If the media element's readyState attribute has the value
     //      HAVE_NOTHING, HAVE_METADATA, or HAVE_CURRENT_DATA, queue a task to
     //      fire a simple event named waiting at the element.
     //      Otherwise, the media element's readyState attribute has the value
@@ -5589,23 +5578,16 @@ void HTMLMediaElement::ChangeNetworkStat
   if (mNetworkState == NETWORK_LOADING) {
     // Start progress notification when entering NETWORK_LOADING.
     StartProgress();
   } else if (mNetworkState == NETWORK_IDLE && !mErrorSink->mError) {
     // Fire 'suspend' event when entering NETWORK_IDLE and no error presented.
     DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
   }
 
-  // According to the resource selection (step2, step9-18), dedicated media
-  // source failure step (step4) and aborting existing load (step4), set show
-  // poster flag to true. https://html.spec.whatwg.org/multipage/media.html
-  if (mNetworkState == NETWORK_NO_SOURCE || mNetworkState == NETWORK_EMPTY) {
-    mShowPoster = true;
-  }
-
   // Changing mNetworkState affects AddRemoveSelfReference().
   AddRemoveSelfReference();
 }
 
 bool HTMLMediaElement::CanActivateAutoplay() {
   // For stream inputs, we activate autoplay on HAVE_NOTHING because
   // this element itself might be blocking the stream from making progress by
   // being paused. We only check that it has data by checking its active state.
@@ -5681,24 +5663,16 @@ void HTMLMediaElement::CheckAutoplayData
       mCurrentPlayRangeStart = CurrentTime();
     }
     MOZ_ASSERT(!mPausedForInactiveDocumentOrChannel);
     mDecoder->Play();
   } else if (mSrcStream) {
     SetPlayedOrSeeked(true);
   }
 
-  // https://html.spec.whatwg.org/multipage/media.html#ready-states:show-poster-flag
-  if (mShowPoster) {
-    mShowPoster = false;
-    if (mTextTrackManager) {
-      mTextTrackManager->TimeMarchesOn();
-    }
-  }
-
   // For blocked media, the event would be pending until it is resumed.
   DispatchAsyncEvent(NS_LITERAL_STRING("play"));
 
   DispatchAsyncEvent(NS_LITERAL_STRING("playing"));
 }
 
 bool HTMLMediaElement::IsActive() const {
   Document* ownerDoc = OwnerDoc();
@@ -7396,33 +7370,13 @@ already_AddRefed<Promise> HTMLMediaEleme
               }
             }
           });
 
   aRv = NS_OK;
   return promise.forget();
 }
 
-void HTMLMediaElement::NotifyTextTrackModeChanged() {
-  if (mPendingTextTrackChanged) {
-    return;
-  }
-  mPendingTextTrackChanged = true;
-  mAbstractMainThread->Dispatch(
-      NS_NewRunnableFunction("HTMLMediaElement::NotifyTextTrackModeChanged",
-                             [this, self = RefPtr<HTMLMediaElement>(this)]() {
-                               mPendingTextTrackChanged = false;
-                               if (!mTextTrackManager) {
-                                 return;
-                               }
-                               GetTextTracks()->CreateAndDispatchChangeEvent();
-                               // https://html.spec.whatwg.org/multipage/media.html#text-track-model:show-poster-flag
-                               if (!mShowPoster) {
-                                 mTextTrackManager->TimeMarchesOn();
-                               }
-                             }));
-}
-
 }  // namespace dom
 }  // namespace mozilla
 
 #undef LOG
 #undef LOG_EVENT
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -1669,26 +1669,16 @@ class HTMLMediaElement : public nsGeneri
   // audio.
   MozPromiseHolder<GenericNonExclusivePromise> mAllowedToPlayPromise;
 
   // True if media has ever been blocked for autoplay, it's used to notify front
   // end to show the correct blocking icon when the document goes back from
   // bfcache.
   bool mHasEverBeenBlockedForAutoplay = false;
 
-  // True if we have dispatched a task for text track changed, will be unset
-  // when we starts processing text track changed.
-  // https://html.spec.whatwg.org/multipage/media.html#pending-text-track-change-notification-flag
-  bool mPendingTextTrackChanged = false;
-
- public:
-  // This function will be called whenever a text track that is in a media
-  // element's list of text tracks has its text track mode change value
-  void NotifyTextTrackModeChanged();
-
  public:
   // Helper class to measure times for playback telemetry stats
   class TimeDurationAccumulator {
    public:
     TimeDurationAccumulator() : mCount(0) {}
     void Start() {
       if (IsStarted()) {
         return;
@@ -1826,21 +1816,16 @@ class HTMLMediaElement : public nsGeneri
 
   // Contains the unique id of the sink device and the device info.
   // The initial value is ("", nullptr) and the default output device is used.
   // It can contain an invalid id and info if the device has been
   // unplugged. It can be set to ("", nullptr). It follows the spec attribute:
   // https://w3c.github.io/mediacapture-output/#htmlmediaelement-extensions
   // Read/Write from the main thread only.
   Pair<nsString, RefPtr<AudioDeviceInfo>> mSink;
-
-  // This flag is used to control when the user agent is to show a poster frame
-  // for a video element instead of showing the video contents.
-  // https://html.spec.whatwg.org/multipage/media.html#show-poster-flag
-  bool mShowPoster;
 };
 
 // Check if the context is chrome or has the debugger or tabs permission
 bool HasDebuggerOrTabsPrivilege(JSContext* aCx, JSObject* aObj);
 
 }  // namespace dom
 }  // namespace mozilla
 
--- a/dom/media/TextTrack.cpp
+++ b/dom/media/TextTrack.cpp
@@ -68,38 +68,47 @@ void TextTrack::SetDefaultSettings() {
 }
 
 JSObject* TextTrack::WrapObject(JSContext* aCx,
                                 JS::Handle<JSObject*> aGivenProto) {
   return TextTrack_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 void TextTrack::SetMode(TextTrackMode aValue) {
-  if (mMode == aValue) {
-    return;
-  }
-  mMode = aValue;
-
-  HTMLMediaElement* mediaElement = GetMediaElement();
-  if (aValue == TextTrackMode::Disabled) {
-    for (size_t i = 0; i < mCueList->Length() && mediaElement; ++i) {
-      mediaElement->NotifyCueRemoved(*(*mCueList)[i]);
+  if (mMode != aValue) {
+    mMode = aValue;
+    if (aValue == TextTrackMode::Disabled) {
+      // Remove all the cues in MediaElement.
+      if (mTextTrackList) {
+        HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
+        if (mediaElement) {
+          for (size_t i = 0; i < mCueList->Length(); ++i) {
+            mediaElement->NotifyCueRemoved(*(*mCueList)[i]);
+          }
+        }
+      }
+      SetCuesInactive();
+    } else {
+      // Add all the cues into MediaElement.
+      if (mTextTrackList) {
+        HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
+        if (mediaElement) {
+          for (size_t i = 0; i < mCueList->Length(); ++i) {
+            mediaElement->NotifyCueAdded(*(*mCueList)[i]);
+          }
+        }
+      }
     }
-    SetCuesInactive();
-  } else {
-    for (size_t i = 0; i < mCueList->Length() && mediaElement; ++i) {
-      mediaElement->NotifyCueAdded(*(*mCueList)[i]);
+    if (mTextTrackList) {
+      mTextTrackList->CreateAndDispatchChangeEvent();
     }
+    // Ensure the TimeMarchesOn is called in case that the mCueList
+    // is empty.
+    NotifyCueUpdated(nullptr);
   }
-  if (mediaElement) {
-    mediaElement->NotifyTextTrackModeChanged();
-  }
-  // Ensure the TimeMarchesOn is called in case that the mCueList
-  // is empty.
-  NotifyCueUpdated(nullptr);
 }
 
 void TextTrack::GetId(nsAString& aId) const {
   // If the track has a track element then its id should be the same as the
   // track element's id.
   if (mTrackElement) {
     mTrackElement->GetAttribute(NS_LITERAL_STRING("id"), aId);
   }
@@ -108,46 +117,54 @@ void TextTrack::GetId(nsAString& aId) co
 void TextTrack::AddCue(TextTrackCue& aCue) {
   TextTrack* oldTextTrack = aCue.GetTrack();
   if (oldTextTrack) {
     ErrorResult dummy;
     oldTextTrack->RemoveCue(aCue, dummy);
   }
   mCueList->AddCue(aCue);
   aCue.SetTrack(this);
-  HTMLMediaElement* mediaElement = GetMediaElement();
-  if (mediaElement && (mMode != TextTrackMode::Disabled)) {
-    mediaElement->NotifyCueAdded(aCue);
+  if (mTextTrackList) {
+    HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
+    if (mediaElement && (mMode != TextTrackMode::Disabled)) {
+      mediaElement->NotifyCueAdded(aCue);
+    }
   }
   SetDirty();
 }
 
 void TextTrack::RemoveCue(TextTrackCue& aCue, ErrorResult& aRv) {
   // Bug1304948, check the aCue belongs to the TextTrack.
   mCueList->RemoveCue(aCue, aRv);
   if (aRv.Failed()) {
     return;
   }
   aCue.SetActive(false);
   aCue.SetTrack(nullptr);
-  HTMLMediaElement* mediaElement = GetMediaElement();
-  if (mediaElement) {
-    mediaElement->NotifyCueRemoved(aCue);
+  if (mTextTrackList) {
+    HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
+    if (mediaElement) {
+      mediaElement->NotifyCueRemoved(aCue);
+    }
   }
   SetDirty();
 }
 
 void TextTrack::SetCuesDirty() {
   for (uint32_t i = 0; i < mCueList->Length(); i++) {
     ((*mCueList)[i])->Reset();
   }
 }
 
 void TextTrack::UpdateActiveCueList() {
-  HTMLMediaElement* mediaElement = GetMediaElement();
+  if (!mTextTrackList) {
+    return;
+  }
+
+  HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
   if (!mediaElement) {
     return;
   }
 
   // If we are dirty, i.e. an event happened that may cause the sorted mCueList
   // to have changed like a seek or an insert for a cue, than we need to rebuild
   // the active cue list from scratch.
   if (mDirty) {
@@ -195,17 +212,22 @@ TextTrackReadyState TextTrack::ReadyStat
 void TextTrack::SetReadyState(uint32_t aReadyState) {
   if (aReadyState <= TextTrackReadyState::FailedToLoad) {
     SetReadyState(static_cast<TextTrackReadyState>(aReadyState));
   }
 }
 
 void TextTrack::SetReadyState(TextTrackReadyState aState) {
   mReadyState = aState;
-  HTMLMediaElement* mediaElement = GetMediaElement();
+
+  if (!mTextTrackList) {
+    return;
+  }
+
+  HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
   if (mediaElement && (mReadyState == TextTrackReadyState::Loaded ||
                        mReadyState == TextTrackReadyState::FailedToLoad)) {
     mediaElement->RemoveTextTrack(this, true);
     mediaElement->UpdateReadyState();
   }
 }
 
 TextTrackList* TextTrack::GetTextTrackList() { return mTextTrackList; }
@@ -219,19 +241,21 @@ HTMLTrackElement* TextTrack::GetTrackEle
 void TextTrack::SetTrackElement(HTMLTrackElement* aTrackElement) {
   mTrackElement = aTrackElement;
 }
 
 void TextTrack::SetCuesInactive() { mCueList->SetCuesInactive(); }
 
 void TextTrack::NotifyCueUpdated(TextTrackCue* aCue) {
   mCueList->NotifyCueUpdated(aCue);
-  HTMLMediaElement* mediaElement = GetMediaElement();
-  if (mediaElement) {
-    mediaElement->NotifyCueUpdated(aCue);
+  if (mTextTrackList) {
+    HTMLMediaElement* mediaElement = mTextTrackList->GetMediaElement();
+    if (mediaElement) {
+      mediaElement->NotifyCueUpdated(aCue);
+    }
   }
   SetDirty();
 }
 
 void TextTrack::GetLabel(nsAString& aLabel) const {
   if (mTrackElement) {
     mTrackElement->GetLabel(aLabel);
   } else {
@@ -269,14 +293,10 @@ bool TextTrack::IsLoaded() {
     nsAutoString src;
     if (!(mTrackElement->GetAttr(kNameSpaceID_None, nsGkAtoms::src, src))) {
       return true;
     }
   }
   return (mReadyState >= Loaded);
 }
 
-HTMLMediaElement* TextTrack::GetMediaElement() {
-  return mTextTrackList ? mTextTrackList->GetMediaElement() : nullptr;
-}
-
 }  // namespace dom
 }  // namespace mozilla
--- a/dom/media/TextTrack.h
+++ b/dom/media/TextTrack.h
@@ -15,17 +15,16 @@
 
 namespace mozilla {
 namespace dom {
 
 class TextTrackList;
 class TextTrackCue;
 class TextTrackCueList;
 class HTMLTrackElement;
-class HTMLMediaElement;
 
 enum TextTrackSource { Track, AddTextTrack, MediaResourceSpecific };
 
 // Constants for numeric readyState property values.
 enum TextTrackReadyState {
   NotLoaded = 0U,
   Loading = 1U,
   Loaded = 2U,
@@ -98,18 +97,16 @@ class TextTrack final : public DOMEventT
 
   void DispatchAsyncTrustedEvent(const nsString& aEventName);
 
   bool IsLoaded();
 
  private:
   ~TextTrack();
 
-  HTMLMediaElement* GetMediaElement();
-
   RefPtr<TextTrackList> mTextTrackList;
 
   TextTrackKind mKind;
   nsString mLabel;
   nsString mLanguage;
   nsString mType;
   TextTrackMode mMode;
 
--- a/dom/media/TextTrackList.cpp
+++ b/dom/media/TextTrackList.cpp
@@ -119,35 +119,49 @@ class TrackEventRunner : public Runnable
   NS_IMETHOD Run() override { return mList->DispatchTrackEvent(mEvent); }
 
   RefPtr<TextTrackList> mList;
 
  private:
   RefPtr<Event> mEvent;
 };
 
+class ChangeEventRunner final : public TrackEventRunner {
+ public:
+  ChangeEventRunner(TextTrackList* aList, Event* aEvent)
+      : TrackEventRunner(aList, aEvent) {}
+
+  NS_IMETHOD Run() override {
+    mList->mPendingTextTrackChange = false;
+    return TrackEventRunner::Run();
+  }
+};
+
 nsresult TextTrackList::DispatchTrackEvent(Event* aEvent) {
   return DispatchTrustedEvent(aEvent);
 }
 
 void TextTrackList::CreateAndDispatchChangeEvent() {
   MOZ_ASSERT(NS_IsMainThread());
-  nsPIDOMWindowInner* win = GetOwner();
-  if (!win) {
-    return;
-  }
-
-  RefPtr<Event> event = NS_NewDOMEvent(this, nullptr, nullptr);
+  if (!mPendingTextTrackChange) {
+    nsPIDOMWindowInner* win = GetOwner();
+    if (!win) {
+      return;
+    }
 
-  event->InitEvent(NS_LITERAL_STRING("change"), false, false);
-  event->SetTrusted(true);
+    mPendingTextTrackChange = true;
+    RefPtr<Event> event = NS_NewDOMEvent(this, nullptr, nullptr);
 
-  nsCOMPtr<nsIRunnable> eventRunner = new TrackEventRunner(this, event);
-  nsGlobalWindowInner::Cast(win)->Dispatch(TaskCategory::Other,
-                                           eventRunner.forget());
+    event->InitEvent(NS_LITERAL_STRING("change"), false, false);
+    event->SetTrusted(true);
+
+    nsCOMPtr<nsIRunnable> eventRunner = new ChangeEventRunner(this, event);
+    nsGlobalWindowInner::Cast(win)->Dispatch(TaskCategory::Other,
+                                             eventRunner.forget());
+  }
 }
 
 void TextTrackList::CreateAndDispatchTrackEventRunner(
     TextTrack* aTrack, const nsAString& aEventName) {
   DebugOnly<nsresult> rv;
   nsCOMPtr<nsIEventTarget> target = GetMainThreadEventTarget();
   if (!target) {
     // If we are not able to get the main-thread object we are shutting down.
--- a/dom/media/TextTrackList.h
+++ b/dom/media/TextTrackList.h
@@ -61,16 +61,18 @@ class TextTrackList final : public DOMEv
 
   bool AreTextTracksLoaded();
   nsTArray<RefPtr<TextTrack>>& GetTextTrackArray();
 
   IMPL_EVENT_HANDLER(change)
   IMPL_EVENT_HANDLER(addtrack)
   IMPL_EVENT_HANDLER(removetrack)
 
+  bool mPendingTextTrackChange = false;
+
  private:
   ~TextTrackList();
 
   nsTArray<RefPtr<TextTrack>> mTextTracks;
   RefPtr<TextTrackManager> mTextTrackManager;
 
   void CreateAndDispatchTrackEventRunner(TextTrack* aTrack,
                                          const nsAString& aEventName);
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/html/semantics/embedded-content/media-elements/track/track-element/track-change-event.html.ini
@@ -0,0 +1,4 @@
+[track-change-event.html]
+  [A 'change' event is fired when a TextTrack's mode changes]
+    expected: FAIL
+
--- a/testing/web-platform/tests/html/semantics/embedded-content/media-elements/track/track-element/track-change-event.html
+++ b/testing/web-platform/tests/html/semantics/embedded-content/media-elements/track/track-element/track-change-event.html
@@ -7,15 +7,15 @@ async_test(function(t) {
     var video = document.createElement('video');
     var track = video.addTextTrack('subtitles', 'test', 'en');
 
     // addTextTrack() defaults to "hidden", so settings
     // mode to "showing" should trigger a "change" event.
     track.mode = 'showing';
     assert_equals(video.textTracks.length, 1);
 
-    video.textTracks.onchange = t.step_func_done(function(event) {
+    video.textTracks.onchange = t.step_func_done(function() {
         assert_equals(event.target, video.textTracks);
         assert_true(event instanceof Event, 'instanceof');
         assert_false(event.hasOwnProperty('track'), 'unexpected property found: "track"');
     });
 });
 </script>