[webkit-reviews] review denied: [Bug 213816] WKUserScripts injecting into all frames seem to violate app-bound domains : [Attachment 403244] Patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has denied katherine_cheney at apple.com's
request for review:
Bug 213816: WKUserScripts injecting into all frames seem to violate app-bound
domains
https://bugs.webkit.org/show_bug.cgi?id=213816

Attachment 403244: Patch

https://bugs.webkit.org/attachment.cgi?id=403244&action=review




--- Comment #3 from Brady Eidson <beidson at apple.com> ---
Comment on attachment 403244
  --> https://bugs.webkit.org/attachment.cgi?id=403244
Patch

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

Other than rebasing first and adding a test, looks good.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3461
> -    if (shouldEnableInAppBrowserPrivacyProtections()) {
> +    if (frame->shouldEnableInAppBrowserPrivacyProtections()) {
>	   send(Messages::WebPageProxy::ScriptValueCallback({ },
ExceptionDetails { "Unable to execute JavaScript"_s }, callbackID));

You will have to rebase before this lands.

Also, this is an interesting new case that now needs to be tested (as of
https://trac.webkit.org/changeset/263727/webkit) - Could you add to the API
test an attempt to evaluate javascript to the non app-bound domain should fail?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:113
> +    else if ([task.request.URL.path
isEqualToString:@"/should-load-for-main-frame-only"])
> +	   countInjectedScriptSource++;

Another way to do this is checking the WKFrameInfo on the
WKURLSchemeHandlerTask and see if it's the main frame, and get its URL, etc.