Changeset 211374 in webkit


Ignore:
Timestamp:
Jan 30, 2017 10:16:57 AM (7 years ago)
Author:
graouts@webkit.org
Message:

LayoutTest media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html is flaky
​https://bugs.webkit.org/show_bug.cgi?id=167254
<rdar://problem/30259293>

Reviewed by Dean Jackson.

Source/WebCore:

When we would identify that we need to prolong an existing auto-hide timer, when the previous
auto-hide timer was marked as non-cancelable, calling _cancelAutoHideTimer() would not actually
cancel the previous timer, which would let it fire and hide the controls bar. We now have two
methods, _cancelAutoHideTimer() which always cancels the current auto-hide timer, and
_cancelNonEnforcedAutoHideTimer() which is used from all other existing call sites, which only
cancels the current auto-hide timer if it was marked as cancelable. This, and revised timing in
the test itself, make media/modern-media-controls/media-controller/media-controller-auto-hide-
mouse-leave-after-play.html a lot more reliable.

We also make a small drive-by fix where we ensure the "autoHideDelay" property is set first so
that setting other members which may set a timer do not used an undefined value for the auto-hide
timer delay.

  • Modules/modern-media-controls/controls/controls-bar.js:

(ControlsBar.prototype.set visible):
(ControlsBar.prototype.handleEvent):
(ControlsBar.prototype._cancelNonEnforcedAutoHideTimer):
(ControlsBar.prototype._cancelAutoHideTimer):

LayoutTests:

We improve the test by setting off timers when the actual "play" and "pause" events are
triggered rather than when we call .play() or .pause() on the media element. This matches
when the auto-hide timer are set in ControlsBar and makes the test more robust. Combined
with the modern-media-controls WebCore module source changes, we can now stop marking this
test as flaky.

We apply the same change to media/modern-media-controls/media-controller/media-controller-auto-hide-pause.html
since it also sets off a timer based on the media being paused.

  • media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play-expected.txt:
  • media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html:
  • media/modern-media-controls/media-controller/media-controller-auto-hide-pause.html:
  • platform/mac/TestExpectations:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r211373 r211374  
     12017-01-30  Antoine Quint  <graouts@apple.com>
     2
     3        LayoutTest media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html is flaky
     4        https://bugs.webkit.org/show_bug.cgi?id=167254
     5        <rdar://problem/30259293>
     6
     7        Reviewed by Dean Jackson.
     8
     9        We improve the test by setting off timers when the actual "play" and "pause" events are
     10        triggered rather than when we call .play() or .pause() on the media element. This matches
     11        when the auto-hide timer are set in ControlsBar and makes the test more robust. Combined
     12        with the modern-media-controls WebCore module source changes, we can now stop marking this
     13        test as flaky.
     14
     15        We apply the same change to media/modern-media-controls/media-controller/media-controller-auto-hide-pause.html
     16        since it also sets off a timer based on the media being paused.
     17
     18        * media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play-expected.txt:
     19        * media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html:
     20        * media/modern-media-controls/media-controller/media-controller-auto-hide-pause.html:
     21        * platform/mac/TestExpectations:
     22
    1232017-01-30  Daniel Bates  <dabates@apple.com>
    224

  • trunk/LayoutTests/media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play-expected.txt

    r209430 r211374  
    77PASS controlsBar.classList.contains('faded') is false
    88
    9 Pausing media, this stops the auto-hide timer.
     9Pausing media.
     10Media playback has paused, this stops the auto-hide timer.
    1011
    11 Resuming media playback, this should rewind the auto-hide timer and ensure that entering and leaving the media doesn't hide the controls bar until the auto-hide timer has expired after playing.
     12Resuming media playback.
     13Media playback has resumed, this should rewind the auto-hide timer and ensure that entering and leaving the media doesn't hide the controls bar until the auto-hide timer has expired after playing.
    1214
    1315Mouse entering the media.

  • trunk/LayoutTests/media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html

    r210959 r211374  
    1010
    1111let controlsBar;
    12 let played = false;
    1312
    14 document.querySelector("video").addEventListener("play", (event) => {
    15     if (played)
    16         return;
     13const media = document.querySelector("video");
     14media.addEventListener("play", mediaStartedPlaying);
    1715
    18     played = true;
    19 
     16function mediaStartedPlaying()
     17{
     18    media.removeEventListener("play", mediaStartedPlaying);
     19    baseTime = Date.now();
     20    controlsBar = window.internals.shadowRoot(media).querySelector(".controls-bar");
     21   
    2022    window.requestAnimationFrame(() => {
    21         const media = event.target;
    22         controlsBar = window.internals.shadowRoot(media).querySelector(".controls-bar");
    2323
    2424        debug("Video started playing, controls bar is visible by default.");
    … …  
    2626
    2727        debug("");
    28         debug("Pausing media, this stops the auto-hide timer.");
     28        debug("Pausing media.");
     29
     30        media.addEventListener("pause", mediaPaused);
    2931        media.pause();
     32    });
     33}
     34
     35function mediaPaused()
     36{
     37    debug("Media playback has paused, this stops the auto-hide timer.");
     38    debug("");
     39    debug("Resuming media playback.");
     40    media.addEventListener("play", mediaResumedPlaying);
     41    media.play();
     42}
     43
     44function mediaResumedPlaying()
     45{
     46    debug("Media playback has resumed, this should rewind the auto-hide timer and ensure that entering and leaving the media doesn't hide the controls bar until the auto-hide timer has expired after playing.");
     47    setTimeout(() => {
     48        debug("");
     49        debug("Mouse entering the media.");
     50        window.eventSender.mouseMoveTo(100, 100);
    3051
    3152        setTimeout(() => {
    3253            debug("");
    33             debug("Resuming media playback, this should rewind the auto-hide timer and ensure that entering and leaving the media doesn't hide the controls bar until the auto-hide timer has expired after playing.");
    34             media.play();
     54            debug("Mouse leaving the media.");
     55            window.eventSender.mouseMoveTo(400, 400);
    3556
    36             setTimeout(() => {
     57            window.requestAnimationFrame(() => {
    3758                debug("");
    38                 debug("Mouse entering the media.");
    39                 eventSender.mouseMoveTo(100, 100);
     59                debug("The initial auto-hide timer started when we resumed playback should not have expired or be overriden by the mouse entering and leaving the media, the controls should remain visible.");
     60                shouldBeFalse("controlsBar.classList.contains('faded')");
     61            });
     62        }, 50);
     63    }, 50);
    4064
    41                 setTimeout(() => {
    42                     debug("");
    43                     debug("Mouse leaving the media.");
    44                     eventSender.mouseMoveTo(400, 400);
     65    setTimeout(() => {
     66        debug("");
     67        debug("The initial auto-hide timer started when we resumed playback should now have expired and the controls should be faded.");
     68        shouldBeTrue("controlsBar.classList.contains('faded')");
    4569
    46                     window.requestAnimationFrame(() => {
    47                         debug("");
    48                         debug("The initial auto-hide timer started when we resumed playback should not have expired or be overriden by the mouse entering and leaving the media, the controls should remain visible.");
    49                         shouldBeFalse("controlsBar.classList.contains('faded')");
    50 
    51                     });
    52                 }, 50);
    53             }, 50);
    54 
    55             setTimeout(() => {
    56                 debug("");
    57                 debug("The initial auto-hide timer started when we resumed playback should now have expired and the controls should be faded.");
    58                 shouldBeTrue("controlsBar.classList.contains('faded')");
    59 
    60                 debug("");
    61                 media.remove();
    62                 finishJSTest();
    63             }, 300);
    64         });
    65     });
    66 });
     70        debug("");
     71        media.remove();
     72        finishJSTest();
     73    }, 300);
     74}
    6775
    6876</script>

  • trunk/LayoutTests/media/modern-media-controls/media-controller/media-controller-auto-hide-pause.html

    r210959 r211374  
    1111let controlsBar;
    1212
    13 document.querySelector("video").addEventListener("play", (event) => {
     13document.querySelector("video").addEventListener("play", event => {
    1414    window.requestAnimationFrame(() => {
    1515        const media = event.target;
    … …  
    2020
    2121        media.pause();
     22        media.addEventListener("pause", event => {
     23            setTimeout(() => {
     24                debug("");
     25                debug("Auto-hide timer would have elapsed, but media was paused so controls bar should remain visible.");
     26                shouldBeFalse("controlsBar.classList.contains('faded')");
    2227
    23         setTimeout(() => {
    24             debug("");
    25             debug("Auto-hide timer would have elapsed, but media was paused so controls bar should remain visible.");
    26             shouldBeFalse("controlsBar.classList.contains('faded')");
    27 
    28             debug("");
    29             media.remove();
    30             finishJSTest();
    31         }, 300);
     28                debug("");
     29                media.remove();
     30                finishJSTest();
     31            }, 300);
     32        });
    3233    });
    3334});

  • trunk/LayoutTests/platform/mac/TestExpectations

    r211302 r211374  
    14701470webkit.org/b/165290 media/modern-media-controls/tracks-panel/tracks-panel-hide-click-outside.html [ Pass Timeout ]
    14711471
    1472 webkit.org/b/167254 media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html [ Pass Failure ]
    1473 
    14741472webkit.org/b/167263 media/modern-media-controls/media-controller/media-controller-auto-hide.html [ Pass Failure ]
    14751473

  • trunk/Source/WebCore/ChangeLog

    r211372 r211374  
     12017-01-30  Antoine Quint  <graouts@apple.com>
     2
     3        LayoutTest media/modern-media-controls/media-controller/media-controller-auto-hide-mouse-leave-after-play.html is flaky
     4        https://bugs.webkit.org/show_bug.cgi?id=167254
     5        <rdar://problem/30259293>
     6
     7        Reviewed by Dean Jackson.
     8
     9        When we would identify that we need to prolong an existing auto-hide timer, when the previous
     10        auto-hide timer was marked as non-cancelable, calling _cancelAutoHideTimer() would not actually
     11        cancel the previous timer, which would let it fire and hide the controls bar. We now have two
     12        methods, _cancelAutoHideTimer() which always cancels the current auto-hide timer, and
     13        _cancelNonEnforcedAutoHideTimer() which is used from all other existing call sites, which only
     14        cancels the current auto-hide timer if it was marked as cancelable. This, and revised timing in
     15        the test itself, make media/modern-media-controls/media-controller/media-controller-auto-hide-
     16        mouse-leave-after-play.html a lot more reliable.
     17
     18        We also make a small drive-by fix where we ensure the "autoHideDelay" property is set first so
     19        that setting other members which may set a timer do not used an undefined value for the auto-hide
     20        timer delay.
     21
     22        * Modules/modern-media-controls/controls/controls-bar.js:
     23        (ControlsBar.prototype.set visible):
     24        (ControlsBar.prototype.handleEvent):
     25        (ControlsBar.prototype._cancelNonEnforcedAutoHideTimer):
     26        (ControlsBar.prototype._cancelAutoHideTimer):
     27
    1282017-01-30  Carlos Garcia Campos  <cgarcia@igalia.com>
    229

  • trunk/Source/WebCore/Modules/modern-media-controls/controls/controls-bar.js

    r211209 r211374  
    3434        this._mediaControls = mediaControls;
    3535
     36        this.autoHideDelay = ControlsBar.DefaultAutoHideDelay;
     37
    3638        this.fadesWhileIdle = false;
    3739        this.userInteractionEnabled = true;
    38 
    39         this.autoHideDelay = ControlsBar.DefaultAutoHideDelay;
    4040
    4141        if (GestureRecognizer.SupportsTouches)
    … …  
    119119            this.faded = false;
    120120        else if (!flag)
    121             this._cancelAutoHideTimer();
     121            this._cancelNonEnforcedAutoHideTimer();
    122122
    123123        super.visible = flag;
    … …  
    158158            if (event.type === "mouseenter") {
    159159                this._disableAutoHiding = true;
    160                 this._cancelAutoHideTimer();
     160                this._cancelNonEnforcedAutoHideTimer();
    161161            } else if (event.type === "mouseleave") {
    162162                delete this._disableAutoHiding;
    … …  
    196196    // Private
    197197
     198    _cancelNonEnforcedAutoHideTimer()
     199    {
     200        if (!this._enforceAutoHideTimer)
     201            this._cancelAutoHideTimer();
     202
     203    }
     204
    198205    _cancelAutoHideTimer()
    199206    {
    200         if (this._enforceAutoHideTimer)
    201             return;
    202 
    203207        window.clearTimeout(this._autoHideTimer);
    204208        delete this._autoHideTimer;
Note: See TracChangeset for help on using the changeset viewer.