Bug 1551385 - part1 : let 'processCue()' handle cleaning cues div. r=heycam
☠☠ backed out by ca67f4681a4a ☠ ☠
authorAlastor Wu <alwu@mozilla.com>
Wed, 22 May 2019 05:39:40 +0000
changeset 475228 883151b42d88925215fb1212af70e6e425cd0f23
parent 475227 916a05c96ee29fd1b4dfa9e33c92d4e5a758dcc7
child 475229 fb20dcf3c07280de6c2b4d8b9a1edcdeec9d4d29
push id36058
push useraciure@mozilla.com
push dateFri, 24 May 2019 03:53:25 +0000
treeherdermozilla-central@c87317c41902 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1551385
milestone69.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 1551385 - part1 : let 'processCue()' handle cleaning cues div. r=heycam We can actually let `processCue()` to handle rendering cues or cleaning displayed cues, no need to use another way to clear the cue. The advantages is to make the code cleaner and easier to read, now we just need to know JS side would handle all rendering stuffs for us. We don't need to have different behavior when there is no showing cue. The way we clear displayed cues are intuitive, we would remove all child nodes under the overlay, which are used to display cues. Differential Revision: https://phabricator.services.mozilla.com/D31171
dom/html/TextTrackManager.cpp
dom/media/webvtt/vtt.jsm
--- a/dom/html/TextTrackManager.cpp
+++ b/dom/html/TextTrackManager.cpp
@@ -235,50 +235,49 @@ void TextTrackManager::DidSeek() {
   mHasSeeked = true;
 }
 
 void TextTrackManager::UpdateCueDisplay() {
   WEBVTT_LOG("UpdateCueDisplay");
   mUpdateCueDisplayDispatched = false;
 
   if (!mMediaElement || !mTextTracks || IsShutdown()) {
+    WEBVTT_LOG("Abort UpdateCueDisplay.");
     return;
   }
 
   nsIFrame* frame = mMediaElement->GetPrimaryFrame();
   nsVideoFrame* videoFrame = do_QueryFrame(frame);
   if (!videoFrame) {
+    WEBVTT_LOG("Abort UpdateCueDisplay, because of no video frame.");
     return;
   }
 
   nsCOMPtr<nsIContent> overlay = videoFrame->GetCaptionOverlay();
-  nsCOMPtr<nsIContent> controls = videoFrame->GetVideoControls();
   if (!overlay) {
+    WEBVTT_LOG("Abort UpdateCueDisplay, because of no overlay.");
     return;
   }
 
+  nsPIDOMWindowInner* window = mMediaElement->OwnerDoc()->GetInnerWindow();
+  if (!window) {
+    WEBVTT_LOG("Abort UpdateCueDisplay, because of no window.");
+  }
+
   nsTArray<RefPtr<TextTrackCue>> showingCues;
   mTextTracks->GetShowingCues(showingCues);
 
-  if (showingCues.Length() > 0) {
-    WEBVTT_LOG("UpdateCueDisplay, processCues, showingCuesNum=%zu",
-               showingCues.Length());
-    RefPtr<nsVariantCC> jsCues = new nsVariantCC();
-
-    jsCues->SetAsArray(nsIDataType::VTYPE_INTERFACE, &NS_GET_IID(EventTarget),
-                       showingCues.Length(),
-                       static_cast<void*>(showingCues.Elements()));
-    nsPIDOMWindowInner* window = mMediaElement->OwnerDoc()->GetInnerWindow();
-    if (window) {
-      sParserWrapper->ProcessCues(window, jsCues, overlay, controls);
-    }
-  } else if (overlay->Length() > 0) {
-    WEBVTT_LOG("UpdateCueDisplay EmptyString");
-    nsContentUtils::SetNodeTextContent(overlay, EmptyString(), true);
-  }
+  WEBVTT_LOG("UpdateCueDisplay, processCues, showingCuesNum=%zu",
+             showingCues.Length());
+  RefPtr<nsVariantCC> jsCues = new nsVariantCC();
+  jsCues->SetAsArray(nsIDataType::VTYPE_INTERFACE, &NS_GET_IID(EventTarget),
+                     showingCues.Length(),
+                     static_cast<void*>(showingCues.Elements()));
+  nsCOMPtr<nsIContent> controls = videoFrame->GetVideoControls();
+  sParserWrapper->ProcessCues(window, jsCues, overlay, controls);
 }
 
 void TextTrackManager::NotifyCueAdded(TextTrackCue& aCue) {
   WEBVTT_LOG("NotifyCueAdded, cue=%p", &aCue);
   if (mNewCues) {
     mNewCues->AddCue(aCue);
   }
   MaybeRunTimeMarchesOn();
--- a/dom/media/webvtt/vtt.jsm
+++ b/dom/media/webvtt/vtt.jsm
@@ -1079,24 +1079,35 @@ XPCOMUtils.defineLazyPreferenceGetter(th
 
   WebVTT.convertCueToDOMTree = function(window, cuetext) {
     if (!window) {
       return null;
     }
     return parseContent(window, cuetext, PARSE_CONTENT_MODE.DOCUMENT_FRAGMENT);
   };
 
+  function clearAllCuesDiv(overlay) {
+    while (overlay.firstChild) {
+      overlay.firstChild.remove();
+    }
+  }
+
   // Runs the processing model over the cues and regions passed to it.
-  // @param overlay A block level element (usually a div) that the computed cues
+  // Spec https://www.w3.org/TR/webvtt1/#processing-model
+  // @parem window : JS window
+  // @param cues : the VTT cues are going to be displayed.
+  // @param overlay : A block level element (usually a div) that the computed cues
   //                and regions will be placed into.
-  // @param controls  A Control bar element. Cues' position will be
+  // @param controls : A Control bar element. Cues' position will be
   //                 affected and repositioned according to it.
   WebVTT.processCues = function(window, cues, overlay, controls) {
-    if (!window || !cues || !overlay) {
-      return null;
+    if (!cues) {
+      LOG(`Abort processing because no cue.`);
+      clearAllCuesDiv(overlay);
+      return;
     }
 
     let controlBar, controlBarShown;
     if (controls) {
       // controls is a <div> that is the children of the UA Widget Shadow Root.
       controlBar = controls.parentNode.getElementById("controlBar");
       controlBarShown = controlBar ? !!controlBar.clientHeight : false;
     } else {
@@ -1118,24 +1129,22 @@ XPCOMUtils.defineLazyPreferenceGetter(th
           return true;
         }
       }
       return false;
     }
 
     // We don't need to recompute the cues' display states. Just reuse them.
     if (!shouldCompute(cues)) {
+      LOG(`Abort processing because no need to compute cues' display state.`);
       return;
     }
     overlay.lastControlBarShownStatus = controlBarShown;
 
-    // Remove all previous children.
-    while (overlay.firstChild) {
-      overlay.firstChild.remove();
-    }
+    clearAllCuesDiv(overlay);
     let rootOfCues = window.document.createElement("div");
     rootOfCues.style.position = "absolute";
     rootOfCues.style.left = "0";
     rootOfCues.style.right = "0";
     rootOfCues.style.top = "0";
     rootOfCues.style.bottom = "0";
     overlay.appendChild(rootOfCues);