Bug 214722 - Propagating media only user gesture through Fetch ReadableStream
Summary: Propagating media only user gesture through Fetch ReadableStream
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 181943
  Show dependency treegraph
 
Reported: 2020-07-24 00:05 PDT by Jiewen Tan
Modified: 2020-07-31 00:06 PDT (History)
20 users (show)

See Also:


Attachments
WIP (24.30 KB, patch)
2020-07-28 22:17 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
WIP (28.47 KB, patch)
2020-07-29 08:43 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (35.18 KB, patch)
2020-07-30 05:54 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (35.94 KB, patch)
2020-07-30 09:00 PDT, youenn fablet
jiewen_tan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2020-07-24 00:05:27 PDT
Propagating media only user gesture through Fetch ReadableStream. A follow up from Bug 214444.
Comment 1 Jiewen Tan 2020-07-28 22:17:02 PDT
Created attachment 405439 [details]
WIP
Comment 2 Jiewen Tan 2020-07-28 22:22:04 PDT
The WIP patch is not completed. It is uploaded here for Youenn to review such that we could continue our discussion off line.
Comment 3 youenn fablet 2020-07-29 08:43:49 PDT
Created attachment 405460 [details]
WIP
Comment 4 youenn fablet 2020-07-30 05:54:42 PDT
Created attachment 405563 [details]
Patch
Comment 5 youenn fablet 2020-07-30 06:03:07 PDT
Current patch allows using async appropriately with keeping user gesture until a timeout:
// within click event handler, so user gesture active
const response = await fetch('blo');
// user gesture should still be active if fetch token is not expired
const reader = response.body.getReader();
const chunk = await reader.read();
// user gesture should still be active if fetch token is not expired
...

This seems fine to me but we should probably discuss this, see how the spec is defining this and if not defining this, make sure to start discussion there.

With the current patch, a click event handler doing the following would be like:

// within click event handler, so user gesture active
const promise = fetch('blabla');
...
// promise not yet resolved, may be out of the click event handler
promise.then(callback1)
...
// promise gets resolved before fetch token is expired.
...
// promise is now resolved but fetch token is not expired.
promise.then(callback2)

I believe callback1 would get user gesture priviledges, not callback2.
Comment 6 youenn fablet 2020-07-30 09:00:41 PDT
Created attachment 405573 [details]
Patch
Comment 7 Jiewen Tan 2020-07-30 11:23:36 PDT
(In reply to youenn fablet from comment #5)
> Current patch allows using async appropriately with keeping user gesture
> until a timeout:
> // within click event handler, so user gesture active
> const response = await fetch('blo');
> // user gesture should still be active if fetch token is not expired
> const reader = response.body.getReader();
> const chunk = await reader.read();
> // user gesture should still be active if fetch token is not expired
> ...
> 
> This seems fine to me but we should probably discuss this, see how the spec
> is defining this and if not defining this, make sure to start discussion
> there.
> 
> With the current patch, a click event handler doing the following would be
> like:
> 
> // within click event handler, so user gesture active
> const promise = fetch('blabla');
> ...
> // promise not yet resolved, may be out of the click event handler
> promise.then(callback1)
> ...
> // promise gets resolved before fetch token is expired.
> ...
> // promise is now resolved but fetch token is not expired.
> promise.then(callback2)
> 
> I believe callback1 would get user gesture priviledges, not callback2.

As far I can tell, the user activation spec Google is proposing has deprecated a token base user gesture model with a producer-consumer model.
https://developers.google.com/web/updates/2019/01/user-activation
Comment 8 Jiewen Tan 2020-07-30 23:46:27 PDT
Comment on attachment 405573 [details]
Patch

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

LGTM. r=me. Youenn, thanks for making the patch.

> Source/WebCore/Modules/fetch/FetchBodySource.cpp:105
> +    // We asynchronously call the callback so that any promise that got resolved can be thened by the page before the promise got settled with user gesture priviledge.

'thened' doesn't seem to be an English word. It's unclear to me why doing it synchronously doesn't have the benefit.
Comment 9 Radar WebKit Bug Importer 2020-07-31 00:06:21 PDT
<rdar://problem/66367691>