Bug 95915 - Extend third-party storage blocking API to optionally allow blocking all storage
Summary: Extend third-party storage blocking API to optionally allow blocking all storage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vicki Pfau
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-09-05 17:12 PDT by Vicki Pfau
Modified: 2012-09-12 09:16 PDT (History)
4 users (show)

See Also:


Attachments
Patch (29.00 KB, patch)
2012-09-05 17:27 PDT, Vicki Pfau
no flags Details | Formatted Diff | Diff
Patch (32.55 KB, patch)
2012-09-06 15:20 PDT, Vicki Pfau
no flags Details | Formatted Diff | Diff
Patch (34.93 KB, patch)
2012-09-06 18:12 PDT, Vicki Pfau
no flags Details | Formatted Diff | Diff
Patch (34.95 KB, patch)
2012-09-07 12:19 PDT, Vicki Pfau
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vicki Pfau 2012-09-05 17:12:00 PDT
The third-party storage blocking API should be extended to allow a user to block all storage instead of either no data or third-party data. This mirrors what we already have in place for cookies.
Comment 1 Vicki Pfau 2012-09-05 17:27:08 PDT
Created attachment 162377 [details]
Patch
Comment 2 WebKit Review Bot 2012-09-06 14:03:21 PDT
Comment on attachment 162377 [details]
Patch

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

New failing tests:
http/tests/security/same-origin-storage-blocked.html
Comment 3 Brady Eidson 2012-09-06 14:08:34 PDT
(In reply to comment #2)
> (From update of attachment 162377 [details])
> Attachment 162377 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/13774441
> 
> New failing tests:
> http/tests/security/same-origin-storage-blocked.html

This seems bad.
Comment 4 Brady Eidson 2012-09-06 14:09:42 PDT
Comment on attachment 162377 [details]
Patch

Since there's a test failing, I did more of a "glance-over" instead of a full review.  Seems fine as a general approach.  I'll look more closely with the new patch that resolves the failing test.
Comment 5 Vicki Pfau 2012-09-06 15:20:35 PDT
Created attachment 162604 [details]
Patch
Comment 6 Brady Eidson 2012-09-06 17:36:23 PDT
Comment on attachment 162604 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKPreferences.h:210
> -WK_EXPORT void WKPreferencesSetThirdPartyStorageBlockingEnabled(WKPreferencesRef preferencesRef, bool enabled);
> -WK_EXPORT bool WKPreferencesGetThirdPartyStorageBlockingEnabled(WKPreferencesRef preferencesRef);
> +WK_EXPORT void WKPreferencesSetStorageBlockingPolicy(WKPreferencesRef preferencesRef, uint32_t policy);
> +WK_EXPORT uint32_t WKPreferencesGetStorageBlockingPolicy(WKPreferencesRef preferencesRef);

This should be exposed as an enum, not a uint32_t

There's precedent for this - See WKPreferencesPrivate.h for a directly comparable example.
Comment 7 Vicki Pfau 2012-09-06 18:12:47 PDT
Created attachment 162636 [details]
Patch
Comment 8 Build Bot 2012-09-06 19:37:51 PDT
Comment on attachment 162636 [details]
Patch

Attachment 162636 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13764931
Comment 9 Vicki Pfau 2012-09-07 12:19:29 PDT
Created attachment 162843 [details]
Patch
Comment 10 Brady Eidson 2012-09-07 16:33:57 PDT
Comment on attachment 162843 [details]
Patch

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

I'd like you to consider making the changes I suggested, but won't hold up the patch any longer.

> Source/WebCore/page/SecurityOrigin.h:52
> +    enum StorageBlockingPolicy {
> +        AllowAllStorage = 0,
> +        BlockAllStorage,
> +        BlockThirdPartyStorage
> +    };

It reads better to me if these enums move from most permissive to least permissive.

-Allow All
-Block 3rd Parties
-Block All.

> Source/WebKit2/UIProcess/API/C/WKPreferences.h:43
> +enum WKStorageBlockingPolicy {
> +    kWKAllowAllStorage = 0,
> +    kWKBlockAllStorage,
> +    kWKBlockThirdPartyStorage
> +};

Same comment.
Comment 11 Vicki Pfau 2012-09-07 19:05:49 PDT
Committed r127956: <http://trac.webkit.org/changeset/127956>
Comment 12 Vicki Pfau 2012-09-07 19:06:41 PDT
<rdar://problem/12259830>