Changeset 231950 in webkit


Ignore:
Timestamp:
May 18, 2018 7:08:45 AM (6 years ago)
Author:
graouts@webkit.org
Message:

[modern-media-controls] AirPlaySupport should be disabled by default
​https://bugs.webkit.org/show_bug.cgi?id=185658
<rdar://problem/40272213>

Reviewed by Simon Fraser.

Source/WebCore:

We now only enable AirplaySupport if the controls are visible to the user _and_ media has played.

Test: media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html

  • Modules/modern-media-controls/media/airplay-support.js:

(AirplaySupport.prototype.enable):
(AirplaySupport.prototype.controlsUserVisibilityDidChange):
(AirplaySupport.prototype._shouldBeEnabled):
(AirplaySupport):

LayoutTests:

Add a new test that checks that we create an AirplaySupport object only after media has started playing.
We also modify a couple of existing tests so they are no longer flaky, as a first pass of this patch
revealed some flakiness.

  • media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt: Added.
  • media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html: Added.
  • media/modern-media-controls/scrubber-support/scrubber-support-drag-expected.txt:
  • media/modern-media-controls/scrubber-support/scrubber-support-drag.html:
  • media/modern-media-controls/volume-support/volume-support-click-expected.txt:
  • media/modern-media-controls/volume-support/volume-support-click.html:
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r231948 r231950  
     12018-05-18  Antoine Quint  <graouts@apple.com>
     2
     3        [modern-media-controls] AirPlaySupport should be disabled by default
     4        https://bugs.webkit.org/show_bug.cgi?id=185658
     5        <rdar://problem/40272213>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Add a new test that checks that we create an AirplaySupport object only after media has started playing.
     10        We also modify a couple of existing tests so they are no longer flaky, as a first pass of this patch
     11        revealed some flakiness.
     12
     13        * media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play-expected.txt: Added.
     14        * media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html: Added.
     15        * media/modern-media-controls/scrubber-support/scrubber-support-drag-expected.txt:
     16        * media/modern-media-controls/scrubber-support/scrubber-support-drag.html:
     17        * media/modern-media-controls/volume-support/volume-support-click-expected.txt:
     18        * media/modern-media-controls/volume-support/volume-support-click.html:
     19
    1202018-05-18  Antoine Quint  <graouts@apple.com>
    221

  • trunk/LayoutTests/media/modern-media-controls/scrubber-support/scrubber-support-drag-expected.txt

    r210959 r231950  
    55
    66PASS media.currentTime is within 0.2 of 3.0136
    7 PASS media.currentTime is 0
     7PASS media.currentTime became 0
    88PASS successfullyParsed is true
    99

  • trunk/LayoutTests/media/modern-media-controls/scrubber-support/scrubber-support-drag.html

    r228097 r231950  
    2525const mediaController = createControls(container, media, null);
    2626
    27 let numberOfEvents = 0;
    2827scheduler.frameDidFire = function() {
    2928    if (media.paused)
    … …  
    4342
    4443    media.addEventListener("timeupdate", () => {
    45         numberOfEvents++;
     44        shouldBeCloseTo("media.currentTime", media.duration / 2, 0.2);
     45        const delta = dragEndX - dragStartX;
     46        const iterations = Math.abs(delta);
    4647
    47         if (numberOfEvents == 1) {
    48             shouldBeCloseTo("media.currentTime", media.duration / 2, 0.2);
    49             const delta = dragEndX - dragStartX;
    50             const iterations = Math.abs(delta);
    51             for (let i = 1; i <= iterations; ++i)
    52                 eventSender.mouseMoveTo(dragStartX + i * Math.sign(delta), centerY);
     48        for (let i = 1; i <= iterations; ++i)
     49            eventSender.mouseMoveTo(dragStartX + i * Math.sign(delta), centerY);
    5350
    54             eventSender.mouseUp();
    55         } else {
    56             shouldBe("media.currentTime", "0");
     51        eventSender.mouseUp();
     52
     53        shouldBecomeEqual("media.currentTime", "0", () => {
    5754            container.remove();
    5855            media.remove();
    5956            finishJSTest();
    60         }
    61    
    62     });
     57        });
     58    }, { once: true });
    6359
    6460    // Start dragging.

  • trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-click-expected.txt

    r228017 r231950  
    44
    55
     6PASS media.paused became false
     7PASS !!muteButtonElement.getBoundingClientRect().width became true
     8PASS !!volumeSliderElement.getBoundingClientRect().width became true
    69PASS mediaController.controls.volumeSlider.parent.visible is true
    7 PASS media.volume is 0.51
     10PASS media.volume became 0.51
    811PASS successfullyParsed is true
    912

  • trunk/LayoutTests/media/modern-media-controls/volume-support/volume-support-click.html

    r228017 r231950  
    2525const mediaController = createControls(container, media, null);
    2626
    27 let numberOfFrames = 0;
    28 scheduler.frameDidFire = function() {
    29     const muteButtonElement = mediaController.controls.muteButton.element;
    30     const muteButtonBounds = muteButtonElement.getBoundingClientRect();
     27const muteButtonElement = mediaController.controls.muteButton.element;
     28const volumeSliderElement = mediaController.controls.volumeSlider.element;
    3129
    32     if (media.paused || muteButtonBounds.width === 0)
    33         return;
     30shouldBecomeEqual("media.paused", "false", () => {
     31    shouldBecomeEqual("!!muteButtonElement.getBoundingClientRect().width", "true", () => {
     32        // Controls are now visible, let's hover over the mute button to make the volume control visible.
     33        const muteButtonBounds = muteButtonElement.getBoundingClientRect();
     34        eventSender.mouseMoveTo(muteButtonBounds.left + muteButtonBounds.width / 2, muteButtonBounds.top + muteButtonBounds.height / 2);
    3435
    35     numberOfFrames++;
     36        shouldBecomeEqual("!!volumeSliderElement.getBoundingClientRect().width", "true", () => {
     37            // Volume slider is visible, let's click in the middle of it.
     38            const volumeSliderBounds = volumeSliderElement.getBoundingClientRect();
     39            eventSender.mouseMoveTo(volumeSliderBounds.left + volumeSliderBounds.width / 2, volumeSliderBounds.top + volumeSliderBounds.height / 2);
     40            eventSender.mouseDown();
     41            eventSender.mouseUp();
    3642
    37     if (numberOfFrames == 1) {
    38         // Controls are now visible, let's hover over the mute button to make the volume control visible.
    39         eventSender.mouseMoveTo(muteButtonBounds.left + muteButtonBounds.width / 2, muteButtonBounds.top + muteButtonBounds.height / 2);
    40     } else if (numberOfFrames == 2) {
    41         // Volume slider is visible, let's click in the middle of it.
    42         const volumeSliderBounds = mediaController.controls.volumeSlider.element.getBoundingClientRect();
    43         eventSender.mouseMoveTo(volumeSliderBounds.left + volumeSliderBounds.width / 2, volumeSliderBounds.top + volumeSliderBounds.height / 2);
    44         eventSender.mouseDown();
    45         eventSender.mouseUp();
     43            // Ensure the volume slider remains visible.
     44            shouldBeTrue("mediaController.controls.volumeSlider.parent.visible");
    4645
    47         // Ensure the volume slider remains visible.
    48         shouldBeTrue("mediaController.controls.volumeSlider.parent.visible");
    49 
    50         scheduler.frameDidFire = null;
    51     }
    52 };
    53 
    54 // First, ensure controls are shown.
    55 mediaController.controls.showsStartButton = false;
    56 
    57 media.addEventListener("volumechange", () => {
    58     // Because the slider has a 9pt-wide knob and tests are ran at 1x, we can't get 0.5 here, and instead get 0.51 since
    59     // the slider will only increment on whole pixels.
    60     shouldBe("media.volume", "0.51");
    61     container.remove();
    62     media.remove();
    63     finishJSTest();
     46            shouldBecomeEqual("media.volume", "0.51", () => {
     47                container.remove();
     48                media.remove();
     49                finishJSTest();
     50            });
     51        });
     52    });
    6453});
    6554

  • trunk/Source/WebCore/ChangeLog

    r231949 r231950  
     12018-05-18  Antoine Quint  <graouts@apple.com>
     2
     3        [modern-media-controls] AirPlaySupport should be disabled by default
     4        https://bugs.webkit.org/show_bug.cgi?id=185658
     5        <rdar://problem/40272213>
     6
     7        Reviewed by Simon Fraser.
     8
     9        We now only enable AirplaySupport if the controls are visible to the user _and_ media has played.
     10
     11        Test: media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html
     12
     13        * Modules/modern-media-controls/media/airplay-support.js:
     14        (AirplaySupport.prototype.enable):
     15        (AirplaySupport.prototype.controlsUserVisibilityDidChange):
     16        (AirplaySupport.prototype._shouldBeEnabled):
     17        (AirplaySupport):
     18
    1192018-05-18  Tim Horton  <timothy_horton@apple.com>
    220

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

    r231923 r231950  
    3939    }
    4040
     41    enable()
     42    {
     43        if (this._shouldBeEnabled())
     44            super.enable();
     45    }
     46
    4147    buttonWasPressed(control)
    4248    {
    … …  
    4652    controlsUserVisibilityDidChange()
    4753    {
    48         const controls = this.mediaController.controls;
    49         if (controls.visible && !controls.faded)
     54        if (this._shouldBeEnabled())
    5055            this.enable();
    5156        else
    … …  
    6873    }
    6974
     75    // Private
     76
     77    _shouldBeEnabled()
     78    {
     79        if (!this.mediaController.hasPlayed)
     80            return false;
     81
     82        const controls = this.mediaController.controls;
     83        return controls.visible && !controls.faded;
     84    }
     85
    7086}
Note: See TracChangeset for help on using the changeset viewer.