[webkit-reviews] review granted: [Bug 185615] Storage Access API: Allow documents that have been granted storage access to also do a popup : [Attachment 340536] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 185615: Storage Access API: Allow documents that have been granted storage
access to also do a popup
https://bugs.webkit.org/show_bug.cgi?id=185615

Attachment 340536: Patch

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




--- Comment #16 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 340536
  --> https://bugs.webkit.org/attachment.cgi?id=340536
Patch

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

r=me

> Source/WebKit/ChangeLog:3
> +	   Allow the WebContent process to read global ViewBridge preferences

Bad change log.

>
LayoutTests/http/tests/storageAccess/request-and-grant-storage-access-cross-ori
gin-non-sandboxed-iframe-pop-window.html:81
> +    <iframe onload="runTest()" id="theIframe"
src="http://localhost:8000/storageAccess/resources/request-storage-access-ifram
e-and-pop-window.html#userShouldGrantAccess,userShouldBeConsulted,policyShouldG
rantAccess,isNotSameOriginIframe"></iframe>

I would have used regular GET (search) parameters rather than hash but OK. Note
that search parameters are easy parsable using 'new
URLSearchParams(location.search).get("myParam")'.

>
LayoutTests/http/tests/storageAccess/resources/request-storage-access-iframe-an
d-pop-window.html:11
> +	   if (window.internals) {

nit: unnecessary curly brackets

>
LayoutTests/http/tests/storageAccess/resources/request-storage-access-iframe-an
d-pop-window.html:50
> +	       var win =
window.open("http://localhost:8000/storageAccess/resources/request-storage-acce
ss-second-window.html", "test window");

You could have omitted "http://localhost:8000/storageAccess/resources/" I
believe since it is same origin as this frame.