Bug 239781 – Video playback fails when using postMessage() during a User Gesture

• Component: Media
• Status: RESOLVED FIXED

[Jer Noble]

Video playback fails when using postMessage() during a User Gesture

[Jer Noble]

<rdar://91281385>

[Jer Noble]

Created attachment 458386 [details]
Patch

[Eric Carlson]

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.

[youenn fablet]

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?

[Eric Carlson]

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?

[Jer Noble]

(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.

[Jer Noble]

Created attachment 458538 [details]
Patch for landing

[Jer Noble]

Created attachment 458876 [details]
Patch for landing

[Jer Noble]

Created attachment 458899 [details]
Patch for landing

[EWS]

Committed r293896 (?): <https://commits.webkit.org/r293896>

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