Bug 71631 – Need a way to allow a scheme access to Local Storage and Databases while Private Browsing is enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Jessie Berlin]

<rdar://problem/8605670>

[Jessie Berlin]

Created attachment 113800 [details]
Patch

[WebKit Review Bot]

Comment on attachment 113800 [details]
Patch

Attachment 113800 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10338488

[Jessie Berlin]

Created attachment 113802 [details]
Patch (with Chromium build fix)

[Jon Honeycutt]

Comment on attachment 113802 [details]
Patch (with Chromium build fix)

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

r=me

> Source/WebCore/ChangeLog:24
> +        (WebCore::SchemeRegistry::registerURLSchmeAsIgnoringPrivateBrowsingForLocalStorage):

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.

[Jessie Berlin]

(In reply to comment #4)
> (From update of attachment 113802 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113802&action=review
> 
> r=me

Thanks!

> 
> > Source/WebCore/ChangeLog:24
> > +        (WebCore::SchemeRegistry::registerURLSchmeAsIgnoringPrivateBrowsingForLocalStorage):
> 
> Typo: Schme

Fixed.

> 
> > Source/WebCore/ChangeLog:26
> > +        (WebCore::SchemeRegistry::registerURLSchmeAsIgnoringPrivateBrowsingForDatabases):
> 
> And here (and elsewhere).

Fixed.

> 
> > 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.

Changed to "allowsLocalStorageAccessInPrivateBrowsing".

> 
> > Source/WebCore/storage/Storage.cpp:32
> > +#include "SchemeRegistry.h"
> > +#include "SecurityOrigin.h"
> 
> Doesn't look like you need these.

Removed.

[Jessie Berlin]

Comment on attachment 113802 [details]
Patch (with Chromium build fix)

Committed with the fixes mentioned above in http://trac.webkit.org/changeset/99439

[Yong Li]

May I ask a question, should session storage be allowed in private browsing mode?