[webkit-reviews] review denied: [Bug 134000] Expose the location of website data : [Attachment 233264] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 17 16:07:42 PDT 2014
mitz at webkit.org <mitz at webkit.org> has denied Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 134000: Expose the location of website data
https://bugs.webkit.org/show_bug.cgi?id=134000
Attachment 233264: Patch
https://bugs.webkit.org/attachment.cgi?id=233264&action=review
------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=233264&action=review
> Source/WebKit2/Shared/mac/SandboxUtilities.cpp:50
> +static bool processIsAppSandboxed(pid_t pid)
Given what this does, I think the name is wrong. There is a difference between
being in a container and being app-sandboxed (which implies the former), but
this checks just for the container. So I think the name should be something
like processHasContainer().
> Source/WebKit2/Shared/mac/SandboxUtilities.cpp:57
> + if (path[0] == '\0')
!path[0]
> Source/WebKit2/Shared/mac/SandboxUtilities.cpp:63
> +bool processIsAppSandboxed()
Ditto.
> Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:108
> ++ (NSURL *)_websiteDataURLForContainerWithURL:(NSURL *)containerURL
The fact that the containerURL parameter isn’t used is telling. This won’t work
if called from a process that doesn’t have a container and wants to find the
website data location for a process with the given container.
More information about the webkit-reviews
mailing list