Bug 212969 - [GTK] Add an internal API to run javascript without forced user gestures
Summary: [GTK] Add an internal API to run javascript without forced user gestures
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords:
Depends on:
Blocks: 184845
  Show dependency treegraph
 
Reported: 2020-06-09 05:12 PDT by Charlie Turner
Modified: 2020-06-12 01:49 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.53 KB, patch)
2020-06-11 11:27 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2020-06-09 05:12:51 PDT
webkit_web_view_run_javascript currently calls runJavaScriptInMainFrame with forceUserGesture set to true in the RunJavaScriptParameters. This caused a problem during the development of an autoplay API https://bugs.webkit.org/show_bug.cgi?id=184845.

If you deny autoplay as a web view policy, and then the page JavaScript tries to call videoElement.play() without you gesturing the video to play, it clearly should not play the video.
However, if you use the GTK API to run the same JS, it will autoplay the video.

It's fair to say that because API users are fully in control of what JS they send in via the API, this is a non-issue and a new public-facing API is not needed.

But, to test autoplay continues to reject play() requests in "normal" cases as above, there does need to be an API for WebViewTest to run JS without user gestures forced. Given that the tests use the public API, I figured a new one is sensible.

Finally, the default could be switched to always run JavaScript without forced user gestures by modifying webkit_web_view_run_javascript. This breaks the webkit/WebKitWebView/fullscreen test. It uses document.documentElement.webkitRequestFullScreen(); to go fullscreen, called via webkit_web_view_run_javascript. I have not debugged why that requires user-gestures on, only assumed it's for good reason and changing that could cause a lot of breakage.
Comment 1 Carlos Garcia Campos 2020-06-09 05:18:08 PDT
We simulate user interaction in other unit tests when it's required.
Comment 2 Charlie Turner 2020-06-09 10:58:55 PDT
I guess you're referring to UIClient tests and their simulateUserInteraction method.

That method of focusing a hidden input element in the DOM, or artificially injecting key strokes into the event loop does not seem to help in this test, whether I do it before calling webkitRequestFullScreen or after calling it.

I'll dig deeper into what going on in webkitRequestFullScreen.
Comment 3 Charlie Turner 2020-06-10 08:51:33 PDT
I still think we need this API.

The fullscreen request will be denied and the test will hang without active user gestures, because document.documentElement.webkitRequestFullScreen() ends up in
WebCore::FullscreenManager::requestFullscreenForElement, which early
returns if no gesture tokens are installed.

User gestures are kept in a static global on the main thread. webkit_web_view_run_javascript and synthetic key events both cause this global to get a gesture installed, so why doesn't it work calling it right before I run the JS without explicitly enabling user gestures? They should be sticky, no?

Gestures are only active for the lifetime of the script run by webkit_web_view_run_javascript and the processing of the key event in WebCore::EventHandler::internalKeyEvent respectively.

It's a tad tricksy how UserGestureIndicator keeps that static global valid only for the scope in which it's stack allocated.

So, we want an API that runs JS without user gestures explicitly. Cocoa has _evaluateJavaScriptWithoutUserGesture, I think we should too after learning the above.
Comment 4 Michael Catanzaro 2020-06-10 09:39:15 PDT
Hm, it seems like the only use for this API is to use in our own API tests, right? Is there any case where a real-world app would want to run JS without gesture forced?
Comment 5 Charlie Turner 2020-06-10 09:59:57 PDT
(In reply to Michael Catanzaro from comment #4)
> Hm, it seems like the only use for this API is to use in our own API tests,
> right? Is there any case where a real-world app would want to run JS without
> gesture forced?

I mean, in theory yes. I don't have a compelling use case though, so maybe that alone is enough to expose instead some internal alternative.

If that's what we want, I don't see prior art in WebViewTest of calling into internal WebKit APIs without going through the GTK API. Any advice on how to handle that without adding a new API?
Comment 6 Michael Catanzaro 2020-06-10 10:07:32 PDT
Lack of prior art is a problem indeed.

In theory, any exported function *could* be used in the tests. Carlos might consider it a layering violation, but it's *possible* to do.
Comment 7 Carlos Garcia Campos 2020-06-11 01:46:57 PDT
We use private API from jsc tests, for example. And recently we also added private API that is used from WTR. It's not a problem. In this particular case, I would add a WebKitWebViewInternal.h header with the private api required from unit tests.
Comment 8 Charlie Turner 2020-06-11 11:27:53 PDT
Created attachment 401661 [details]
Patch
Comment 9 EWS Watchlist 2020-06-11 11:28:39 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 10 EWS 2020-06-12 01:49:31 PDT
Committed r262940: <https://trac.webkit.org/changeset/262940>

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