Bug 167794 – [Modern Media Controls] PiP button is not visible with a live broadcast video

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Antoine Quint]

Created attachment 300534 [details]
Testcase

We're not showing the PiP button for a live broadcast video.

[Radar WebKit Bug Importer]

<rdar://problem/30348790>

[Antoine Quint]

The PiPSupport class (Source/WebCore/Modules/modern-media-controls/media/pip-support.js) listens to several events to update the enabled state of the PiP control: "loadedmetadata", "error", "webkitpresentationmodechanged" and "webkitcurrentplaybacktargetiswirelesschanged". As those events are dispatched, the following method is called:

    syncControl()
    {
        const media = this.mediaController.media;
        if (media.webkitSupportsPresentationMode)
            this.control.enabled = media instanceof HTMLVideoElement && media.webkitSupportsPresentationMode(PiPMode) && !media.webkitCurrentPlaybackTargetIsWireless;
        else
            this.control.enabled = false;
    }

As the testcase loads, we get three calls:

1. one from the constructor (this happens to set some initial state prior to any event firing):
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

2. the "loadedmetadata" event fires:
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

3. the "webkitcurrentplaybacktargetiswirelesschanged" event fires:
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

So in all calls we eventually set "enabled" to false due to "media.webkitSupportsPresentationMode(PiPMode)" being false. Is there some other event we should be using that would determine that a live broadcast video has PiP enabled?

[Antoine Quint]

By contrast, loading a local video yields the same call sequence but "media.webkitSupportsPresentationMode(PiPMode)" is true in both the "loadedmetadata" and "webkitcurrentplaybacktargetiswirelesschanged" event handlers.

[Antoine Quint]

The issue is that we're not taking into account addition (and removal) of video tracks. We do this for FullscreenSupport, we'll just do the same with PiPSupport, likely with a shared superclass.

[Antoine Quint]

Created attachment 300621 [details]
Patch

[Dean Jackson]

Comment on attachment 300621 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300621&action=review

> Source/WebCore/ChangeLog:10
> +        and "webkitcurrentplaybacktargetiswirelesschanged" events to invaidate the enabled state

Typo: invalidate

> Source/WebCore/ChangeLog:18
> +        Since a couple other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)

Nit: a couple of other

[Antoine Quint]

Created attachment 300674 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 300674 [details]
Patch for landing

Clearing flags on attachment: 300674

Committed r211687: <http://trac.webkit.org/changeset/211687>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.