[chromium] Go through WebPermissionClient for local storage access. Also cleanup left over code from previous WebPermissionClient change.
Created attachment 92137 [details] Patch
ccing Darin since it's WebKit API, although I don't think we need to wait for his ok for this since it's just adding a similar function to an API he just reviewed.
chrome side change: http://codereview.chromium.org/6915017/
Comment on attachment 92137 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92137&action=review > Source/WebKit/chromium/public/WebPermissionClient.h:49 > + virtual bool allowLocalStorage(WebFrame*, const WebURL&) { return true; } the URL can be generated from the WebFrame, so it's redundant here > Source/WebKit/chromium/src/StorageAreaProxy.cpp:89 > + if (webView->permissionClient() > + && !webView->permissionClient()->allowLocalStorage(webFrame, url)) { nit: don't line wrap this
Comment on attachment 92137 [details] Patch According to Dr Barth the allowLocalStorage() implementation should be using securityOrigin() and not URL. There are case where the two can differ and we definitely don't want to let a page get around the user's local storage settings by faking out the url.
(In reply to comment #5) > (From update of attachment 92137 [details]) > According to Dr Barth the allowLocalStorage() implementation should be using securityOrigin() and not URL. There are case where the two can differ and we definitely don't want to let a page get around the user's local storage settings by faking out the url. So the old code was calling frame->document()->url(), hence why I moved it to do the same. Does securityOrigin() return the whole url, or only scheme+host+port? If it's the latter, then that wouldn't work if the user has a content setting for a url that has path for example.
Created attachment 92179 [details] Patch
> So the old code was calling frame->document()->url(), hence why I moved it to do the same. Does securityOrigin() return the whole url, or only scheme+host+port? If it's the latter, then that wouldn't work if the user has a content setting for a url that has path for example. Security restrictions at a finer granularity than a scheme+host+port don't work.
ok so I tried to set content settings policies for a url with a path, and that didn't work. so it looks like it's just for hostnames then, in which case using securityOrigin should be fine. however, that should be done in a separate cl, in case that has any side-effects. this change has others that depend on it, and once they're committed, reverting this won't be possible without reverting the chorme side.
Comment on attachment 92179 [details] Patch R=me. I agree that changing the url/securityOrigin behavior should be a separate patch.
Created attachment 92182 [details] Patch
I filed bug 60108 for the url issue. i updated the patch to include the chromium revision with the chrome side change.
Created attachment 92184 [details] Patch
Committed r85703: <http://trac.webkit.org/changeset/85703>