Bug 239781 - Video playback fails when using postMessage() during a User Gesture
Summary: Video playback fails when using postMessage() during a User Gesture
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-26 11:41 PDT by Jer Noble
Modified: 2022-05-06 09:30 PDT (History)
3 users (show)

See Also:


Attachments
Patch (11.36 KB, patch)
2022-04-26 11:51 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (11.50 KB, patch)
2022-04-28 11:25 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (12.12 KB, patch)
2022-05-05 08:39 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (11.83 KB, patch)
2022-05-05 11:35 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2022-04-26 11:41:35 PDT
Video playback fails when using postMessage() during a User Gesture
Comment 1 Jer Noble 2022-04-26 11:42:09 PDT
<rdar://91281385>
Comment 2 Jer Noble 2022-04-26 11:51:42 PDT
Created attachment 458386 [details]
Patch
Comment 3 Eric Carlson 2022-04-26 11:58:02 PDT
Comment on attachment 458386 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Certain frameworks handle user gestures by sending messages through postMessage() to
> +        to a non-shared worker, where the message is processed and a response is sent back

s/to to/to/

> LayoutTests/workers/worker-user-gesture.html:11
> +    const mc = new MessageChannel();
> +    const w = new Worker("worker-user-gesture.js");

Vowels are cheap, you should use some to name these variables.
Comment 4 youenn fablet 2022-04-26 23:34:19 PDT
Comment on attachment 458386 [details]
Patch

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

> Source/WebCore/workers/WorkerMessagingProxy.cpp:78
> +    WorkerUserGestureForwarder(RefPtr<UserGestureToken>&& token)

explicit.

> Source/WebCore/workers/WorkerMessagingProxy.cpp:136
> +    m_scriptExecutionContext->postTask([this, message = WTFMove(message), userGestureForwarder = m_userGestureForwarder] (ScriptExecutionContext& context) mutable {

s/ScriptExecutionContext/auto/

> Source/WebCore/workers/WorkerMessagingProxy.cpp:143
> +            UserGestureIndicator userGestureIndicator(userGestureForwarder ? userGestureForwarder->userGestureToForward() : nullptr);

queueTaskKeepingObjectAlive maybe delay the event dispatching in case of page cache.
This potentially means that when page exits page cache, it might have user gesture privilege a long time after the actual user gesture. Do we want that?

Also, it seems a page could do ping pong postMessage between window and worker to keep user gesture privilege for a potentially long time.
Is that what we want? Should we instead move to a timer-based approach like how HTML defines transient activation?
Comment 5 Eric Carlson 2022-04-27 07:14:10 PDT
Comment on attachment 458386 [details]
Patch

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

>> Source/WebCore/workers/WorkerMessagingProxy.cpp:143
>> +            UserGestureIndicator userGestureIndicator(userGestureForwarder ? userGestureForwarder->userGestureToForward() : nullptr);
> 
> queueTaskKeepingObjectAlive maybe delay the event dispatching in case of page cache.
> This potentially means that when page exits page cache, it might have user gesture privilege a long time after the actual user gesture. Do we want that?
> 
> Also, it seems a page could do ping pong postMessage between window and worker to keep user gesture privilege for a potentially long time.
> Is that what we want? Should we instead move to a timer-based approach like how HTML defines transient activation?

I came back to suggest the same thing - shouldn't we have a user gesture timeout for workers like we do for XHR and Fetch?
Comment 6 Jer Noble 2022-04-27 12:51:51 PDT
(In reply to Eric Carlson from comment #5)
> Comment on attachment 458386 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=458386&action=review
> 
> >> Source/WebCore/workers/WorkerMessagingProxy.cpp:143
> >> +            UserGestureIndicator userGestureIndicator(userGestureForwarder ? userGestureForwarder->userGestureToForward() : nullptr);
> > 
> > queueTaskKeepingObjectAlive maybe delay the event dispatching in case of page cache.
> > This potentially means that when page exits page cache, it might have user gesture privilege a long time after the actual user gesture. Do we want that?
> > 
> > Also, it seems a page could do ping pong postMessage between window and worker to keep user gesture privilege for a potentially long time.
> > Is that what we want? Should we instead move to a timer-based approach like how HTML defines transient activation?
> 
> I came back to suggest the same thing - shouldn't we have a user gesture
> timeout for workers like we do for XHR and Fetch?

Okay, seems totally reasonable. I'll adopt the existing maximumIntervalForUserGestureForwarding duration for this.
Comment 7 Jer Noble 2022-04-28 11:25:12 PDT
Created attachment 458538 [details]
Patch for landing
Comment 8 Jer Noble 2022-05-05 08:39:10 PDT
Created attachment 458876 [details]
Patch for landing
Comment 9 Jer Noble 2022-05-05 11:35:51 PDT
Created attachment 458899 [details]
Patch for landing
Comment 10 EWS 2022-05-06 09:30:38 PDT
Committed r293896 (?): <https://commits.webkit.org/r293896>

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