Convert video-controls-[captions, overlay, track]* tests to testharness.js

third_party/WebKit/LayoutTests/media/media-controls.js

 var captionsButtonElement;
 var captionsButtonCoordinates;
 
 // As specified in mediaControls.css, this is how long it takes to fade out controls
 const controlsFadeOutDurationMs = 300;
 
 // The timeout for the hide-after-no-mouse-movement behavior. Defined (and
 // should mirror) the value 'timeWithoutMouseMovementBeforeHidingMediaControls'
 // in MediaControls.cpp.
 const controlsMouseMovementTimeoutMs = 3000;
@@ skipping 50 matching lines @@
     return elementCoordinates(button);
 }
 
 function mediaControlsButtonDimensions(element, id)
 {
     var button = mediaControlsButton(element, id);
     var buttonBoundingRect = button.getBoundingClientRect();
     return new Array(buttonBoundingRect.width, buttonBoundingRect.height);
 }
 
+function textTrackContainerElement(parentElement) {
+    return mediaControlsElement(internals.shadowRoot(parentElement).firstChild,
+        "-webkit-media-text-track-container");
+}
+
+function textTrackCueElementById(parentElement, id) {

[foolip, 2016/07/05 13:37:00]

It looks like you're only using this in one place and there you're using
"display" for the id, isn't that the same as textTrackDisplayElement(video,
'display')?

[Srirama, 2016/07/05 18:33:28]

Just to return null and check for null instead of checking for exception.
Anyway, i have changed it as per below suggestion.
Right now i have named it "textTrackCueDisplayElement", once we remove all the
uses of existing "textTrackDisplayElement", i will change it.

+    var containerElement = textTrackContainerElement(parentElement);
+    if (!containerElement)
+        return null;
+
+    var controlID = id;
+    if (controlID != 'cue')
+        controlID = "-webkit-media-text-track-" + id;
+
+    return textTrackCueElement(containerElement, controlID);
+}
+
+function textTrackCueElementByIndex(displayElement, cueNumber) {
+    for (i = 0; i < cueNumber; i++)
+        displayElement = displayElement.nextSibling;
+
+    return displayElement;
+}
+
+function textTrackCueElement(containerElement, controlID){

[foolip, 2016/07/05 13:37:00]

This helper doesn't seem to do much, going back to using .firstchild where used
seems OK.

[Srirama, 2016/07/05 18:33:28]

Done.

+    return mediaControlsElement(containerElement.firstChild, controlID);
+}
+
 function textTrackDisplayElement(parentElement, id, cueNumber)
 {
-    var textTrackContainerID = "-webkit-media-text-track-container";
-    var containerElement = mediaControlsElement(internals.shadowRoot(parentElement).firstChild, textTrackContainerID);
+    var containerElement = textTrackContainerElement(parentElement);
 
     if (!containerElement)
         throw "Failed to find text track container element";
 
     if (!id)
         return containerElement;
 
-    if (arguments[1] != 'cue')
-        var controlID = "-webkit-media-text-track-" + arguments[1];
-    else
-        var controlID = arguments[1];
+    var controlID = id;
+    if (controlID != 'cue')

[foolip, 2016/07/05 13:37:00]

This special casing still seems silly. "cue" is never used together with
cueNumber, it would seem better to just have a separate function for getting to
that element. Overall, I think it'd be an improvement if existing uses of
textTrackDisplayElement() were replaced with more specific functions and this
were removed.

Maybe:

textTrackDisplayElement(video); => textTrackContainerElement(video)

textTrackDisplayElement(video, 'cue') => textTrackCueElementByIndex(video, 0) //
I think?

textTrackDisplayElement(video, 'display') => textTrackDisplayElement(video) //
old name, new meaning

textTrackDisplayElement(video, 'display', i) =>
textTrackCueElementByIndex(video, i)

textTrackDisplayElement(video, 'region') => textTrackRegionElement(video)

textTrackDisplayElement(video, 'region-container') =>
textTrackRegionContainerElement(video)

Would that work?

[Srirama, 2016/07/05 18:33:28]

Yes, i have added the required functions for this CL, and a TODO to implement
remaining later.

+        controlID = "-webkit-media-text-track-" + id;
 
-    var displayElement = mediaControlsElement(containerElement.firstChild, controlID);
+    var displayElement = textTrackCueElement(containerElement, controlID);
     if (!displayElement)
         throw "No text track cue with display id '" + controlID + "' is currently visible";
 
     if (cueNumber) {
-        for (i = 0; i < cueNumber; i++)
-            displayElement = displayElement.nextSibling;
+        displayElement = textTrackCueElementByIndex(displayElement, cueNumber);
 
         if (!displayElement)
             throw "There are not " + cueNumber + " text track cues visible";
     }
 
     return displayElement;
 }
 
 function isClosedCaptionsButtonVisible(currentMediaElement)
 {
@@ skipping 63 matching lines @@
 }
 
 function selectTextTrack(video, index)
 {
     clickCCButton();
     var trackListItemElement = textTrackListItemAtIndex(video, index);
     var trackListItemCoordinates = elementCoordinates(trackListItemElement);
     clickAtCoordinates(trackListItemCoordinates[0], trackListItemCoordinates[1]);
 }
 
+function clickTextTrackAtIndex(video, index)
+{
+    var captionsButtonCoordinates = mediaControlsButtonCoordinates(video, "toggle-closed-captions-button");
+    clickAtCoordinates(captionsButtonCoordinates[0], captionsButtonCoordinates[1]);
+    var trackListItemElement = textTrackListItemAtIndex(video, index);
+    var trackListItemCoordinates = elementCoordinates(trackListItemElement);
+    clickAtCoordinates(trackListItemCoordinates[0], trackListItemCoordinates[1]);
+}
+
 function turnClosedCaptionsOff(video)
 {
-    selectTextTrack(video, -1);
+    clickTextTrackAtIndex(video, -1);
 }
 
 function runAfterHideMediaControlsTimerFired(func, mediaElement)
 {
     if (mediaElement.paused)
         throw "The media element is not playing";
 
     // Compute the time it'll take until the controls will be invisible -
     // assuming playback has been started prior to invoking this
     // function. Allow 500ms slack.
     var hideTimeoutMs = controlsMouseMovementTimeoutMs + controlsFadeOutDurationMs + 500;
 
     if (!mediaElement.loop && hideTimeoutMs >= 1000 * (mediaElement.duration - mediaElement.currentTime))
         throw "The media will end before the controls have been hidden";
 
     setTimeout(func, hideTimeoutMs);
 }