[webkit-reviews] review granted: [Bug 71631] Need a way to allow a scheme access to Local Storage and Databases while Private Browsing is enabled : [Attachment 113802] Patch (with Chromium build fix)

[bugzilla-daemon at webkit.org]

Jon Honeycutt <jhoneycutt at apple.com> has granted Jessie Berlin
<jberlin at webkit.org>'s request for review:
Bug 71631: Need a way to allow a scheme access to Local Storage and Databases
while Private Browsing is enabled
https://bugs.webkit.org/show_bug.cgi?id=71631

Attachment 113802: Patch (with Chromium build fix)
https://bugs.webkit.org/attachment.cgi?id=113802&action=review

------- Additional Comments from Jon Honeycutt <jhoneycutt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=113802&action=review


r=me

> Source/WebCore/ChangeLog:24
> +	  
(WebCore::SchemeRegistry::registerURLSchmeAsIgnoringPrivateBrowsingForLocalStor
age):

Typo: Schme

> Source/WebCore/ChangeLog:26
> +	  
(WebCore::SchemeRegistry::registerURLSchmeAsIgnoringPrivateBrowsingForDatabases
):

And here (and elsewhere).

> Source/WebCore/platform/SchemeRegistry.h:77
> +    static bool shouldIgnorePrivateBrowsingForLocalStorage(const String&
scheme);
> +    static void registerURLSchmeAsIgnoringPrivateBrowsingForDatabases(const
String& scheme);
> +    static bool shouldIgnorePrivateBrowsingForDatabases(const String&
scheme);

I think names like "allowsLocalStorageInPrivateBrowsing()" would be more
readable, especially when testing the inverse condition.

> Source/WebCore/storage/Storage.cpp:32
> +#include "SchemeRegistry.h"
> +#include "SecurityOrigin.h"

Doesn't look like you need these.