Bug 238462

Summary: [macOS] Muted video is sometimes paused when entering fullscreen
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cdumez, changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, kondapallykalyan, pdr, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=238472
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch for landing none

Description Eric Carlson 2022-03-28 11:26:23 PDT 
Muted video is sometimes paused when entering fullscreen
Comment 1 Eric Carlson 2022-03-28 11:27:09 PDT 
rdar://89104216
Comment 2 Eric Carlson 2022-03-28 11:37:26 PDT 
Created attachment 455933 [details]
Patch
Comment 3 Eric Carlson 2022-03-28 12:05:01 PDT 
Created attachment 455937 [details]
Patch
Comment 4 Jer Noble 2022-03-28 14:07:56 PDT 
Comment on attachment 455937 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:6058
> +#if ENABLE(FULLSCREEN_API)
> +    auto& fullscreenManager = document().fullscreenManager();
> +    if (isVideo() && fullscreenManager.isFullscreen() && fullscreenManager.currentFullscreenElement())
> +        return false;
> +#endif
> +
> +    if (m_videoFullscreenMode != VideoFullscreenModeNone)
> +        return false;
> +
> +    return document().hidden();

I think this is incorrect long-term, insofar as element fullscreen on a background space (on macOS) shouldn't always be "visible", but we should file a bug to follow up later.

> Source/WebCore/html/HTMLMediaElement.cpp:8344
> +        queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, [this] {
> +            if (isContextStopped())
> +                return;
> +            mediaSession().isVisibleInViewportChanged();
> +            updateShouldAutoplay();
> +            schedulePlaybackControlsManagerUpdate();
> +        });

Is this necessary?
Comment 5 Eric Carlson 2022-03-28 15:48:34 PDT 
Comment on attachment 455937 [details]
Patch

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

>> Source/WebCore/html/HTMLMediaElement.cpp:6058
>> +    return document().hidden();
> 
> I think this is incorrect long-term, insofar as element fullscreen on a background space (on macOS) shouldn't always be "visible", but we should file a bug to follow up later.

I filed bug 238472

>> Source/WebCore/html/HTMLMediaElement.cpp:8344
>> +        });
> 
> Is this necessary?

Oops!
Comment 6 Eric Carlson 2022-03-28 15:54:37 PDT 
Created attachment 455964 [details]
Patch for landing
Comment 7 Eric Carlson 2022-03-29 10:17:52 PDT 
Comment on attachment 455964 [details]
Patch for landing

The failing tests don't seem to be related.
Comment 8 EWS 2022-03-29 11:32:04 PDT 
Committed r292049 (248986@main): <https://commits.webkit.org/248986@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455964 [details].