Bug 223775 - Propagate user gestures through `requestAnimationFrame` just like `setTimeout`
Summary: Propagate user gestures through `requestAnimationFrame` just like `setTimeout`
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-25 16:49 PDT by Devin Rousso
Modified: 2021-03-29 15:55 PDT (History)
11 users (show)

See Also:


Attachments
Patch (9.69 KB, patch)
2021-03-25 16:51 PDT, Devin Rousso
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (13.33 KB, patch)
2021-03-25 19:04 PDT, Devin Rousso
ggaren: review+
Details | Formatted Diff | Diff
Patch (13.32 KB, patch)
2021-03-29 15:07 PDT, Devin Rousso
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (13.32 KB, patch)
2021-03-29 15:09 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2021-03-25 16:49:09 PDT
`setTimeout` and `requestAnimationFrame` are used somewhat interchangeably on the web. There should be similar features/"affordances" for both so that if a developer decides to use a display-linked animation "loop" instead of a strictly time-based one they're able to do similar things in the callback/handler.
Comment 1 Radar WebKit Bug Importer 2021-03-25 16:49:25 PDT
<rdar://problem/75860868>
Comment 2 Devin Rousso 2021-03-25 16:51:17 PDT
Created attachment 424298 [details]
Patch
Comment 3 Devin Rousso 2021-03-25 19:04:05 PDT
Created attachment 424311 [details]
Patch
Comment 4 Geoffrey Garen 2021-03-29 14:01:07 PDT
Comment on attachment 424311 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:10
> +        There should be similar features/"affordances" for both so that if a developer decides

No need to scare-quote affordances -- this is an affordance.

> Source/WebCore/dom/ScriptedAnimationController.cpp:170
> +        if (userGestureTokenToForward && userGestureTokenToForward->hasExpired(UserGestureToken::maximumIntervalForUserGestureForwarding))
> +            userGestureTokenToForward = nullptr;
> +        UserGestureIndicator gestureIndicator(userGestureTokenToForward);

Not new in this patch, but it would be nice (perhaps in a follow-up patch) if the UserGestureIndicator constructor checked the token for expiration, and automatically set the null user gesture state in that case. That could eliminate some boilerplate code and, more importantly, make it impossible to construct an invalid user gesture state.
Comment 5 Devin Rousso 2021-03-29 15:06:30 PDT
Comment on attachment 424311 [details]
Patch

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

>> Source/WebCore/ChangeLog:10
>> +        There should be similar features/"affordances" for both so that if a developer decides
> 
> No need to scare-quote affordances -- this is an affordance.

heh very true :P

will remove :)

>> Source/WebCore/dom/ScriptedAnimationController.cpp:170
>> +        UserGestureIndicator gestureIndicator(userGestureTokenToForward);
> 
> Not new in this patch, but it would be nice (perhaps in a follow-up patch) if the UserGestureIndicator constructor checked the token for expiration, and automatically set the null user gesture state in that case. That could eliminate some boilerplate code and, more importantly, make it impossible to construct an invalid user gesture state.

ooo good idea!  I'll create a followup :)
Comment 6 Devin Rousso 2021-03-29 15:07:09 PDT
Created attachment 424588 [details]
Patch
Comment 7 EWS 2021-03-29 15:08:03 PDT
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
Comment 8 Devin Rousso 2021-03-29 15:09:09 PDT
Created attachment 424591 [details]
Patch

oops forgot about the LayoutTests changelog 😅
Comment 9 EWS 2021-03-29 15:55:55 PDT
Committed r275187: <https://commits.webkit.org/r275187>

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