Use GURLS instead of patterns in SetCookieSetting()

Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747
Committed: https://crrev.com/b1a182cafba0902104512f93e641617e9b15ba18
Cr-Commit-Position: refs/heads/master@{#382728}

[lshang, 2016-02-17 04:39:47]

Description was changed from

==========
Use GURLS instead of patterns in SetCookieSetting() and ResetCookieSetting()


change setcookiesetting and resetcookiesetting to use GURLS

BUG=
==========

to

==========
Use GURLS instead of patterns in SetCookieSetting() and ResetCookieSetting()

We changed HostContentSettingsMap::SetContentSetting to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the
ContentSettingsType.

In this CL, SetCookieSetting() and ResetCookieSetting() are changed to use GURLs

instead of patterns as parameters. For SetCookieSetting(), it will call
SetContentSettingDefaultScope() internally. For ResetCookieSetting(), it will
call SetNarrowestContentSetting() to reset setting for origin urls. 

BUG=551747
==========

[lshang, 2016-02-17 04:41:02]

Description was changed from

==========
Use GURLS instead of patterns in SetCookieSetting() and ResetCookieSetting()

We changed HostContentSettingsMap::SetContentSetting to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the
ContentSettingsType.

In this CL, SetCookieSetting() and ResetCookieSetting() are changed to use GURLs

instead of patterns as parameters. For SetCookieSetting(), it will call
SetContentSettingDefaultScope() internally. For ResetCookieSetting(), it will
call SetNarrowestContentSetting() to reset setting for origin urls. 

BUG=551747
==========

to

==========
Use GURLS instead of patterns in SetCookieSetting() and ResetCookieSetting()

We changed HostContentSettingsMap::SetContentSetting to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() and ResetCookieSetting() are changed to use GURLs

instead of patterns as parameters. For SetCookieSetting(), it will call
SetContentSettingDefaultScope() internally. For ResetCookieSetting(),
it will call SetNarrowestContentSetting() to reset setting for origin urls. 

BUG=551747
==========

[lshang, 2016-03-15 04:23:44]

Description was changed from

==========
Use GURLS instead of patterns in SetCookieSetting() and ResetCookieSetting()

We changed HostContentSettingsMap::SetContentSetting to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() and ResetCookieSetting() are changed to use GURLs

instead of patterns as parameters. For SetCookieSetting(), it will call
SetContentSettingDefaultScope() internally. For ResetCookieSetting(),
it will call SetNarrowestContentSetting() to reset setting for origin urls. 

BUG=551747
==========

to

==========
Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For 
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747
==========

[lshang, 2016-03-16 01:06:03]

Patchset #6 (id:100001) has been deleted

[lshang, 2016-03-16 01:06:16]

Patchset #7 (id:130001) has been deleted

[lshang, 2016-03-16 01:06:33]

Patchset #3 (id:40001) has been deleted

[lshang, 2016-03-16 01:06:44]

Patchset #2 (id:20001) has been deleted

[lshang, 2016-03-16 01:07:04]

lshang@chromium.org changed reviewers:
+ raymes@chromium.org

[lshang, 2016-03-16 01:07:05]

PTAL thanks!

[raymes, 2016-03-16 03:24:11]

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:477:
std::string(), setting);
I wonder if we should add a function to GetCookieSettings: AddException or
something which takes patterns?

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:480:
CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
nit: this looks wrong!

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/extensi...
File chrome/browser/extensions/extension_special_storage_policy_unittest.cc
(right):

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/extensi...
chrome/browser/extensions/extension_special_storage_policy_unittest.cc:335:
ContentSettingsPattern::FromString("[*.]pattern.com");
nit: perhaps add a TODO to remove the pattern after ResetCookieSetting is
changed?

[lshang, 2016-03-17 01:53:59]

Raymes, could you PTAL again? Thanks!

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:477:
std::string(), setting);
On 2016/03/16 03:24:11, raymes wrote:
> I wonder if we should add a function to GetCookieSettings: AddException or
> something which takes patterns?

Done.

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:480:
CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
On 2016/03/16 03:24:11, raymes wrote:
> nit: this looks wrong!

Done.⊙▂⊙

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/extensi...
File chrome/browser/extensions/extension_special_storage_policy_unittest.cc
(right):

https://codereview.chromium.org/1694063002/diff/210001/chrome/browser/extensi...
chrome/browser/extensions/extension_special_storage_policy_unittest.cc:335:
ContentSettingsPattern::FromString("[*.]pattern.com");
On 2016/03/16 03:24:11, raymes wrote:
> nit: perhaps add a TODO to remove the pattern after ResetCookieSetting is
> changed?

Done.

[raymes, 2016-03-17 04:21:29]

lgtm with nits

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
File components/content_settings/core/browser/cookie_settings.cc (right):

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
components/content_settings/core/browser/cookie_settings.cc:109:
DCHECK(secondary_pattern == ContentSettingsPattern::Wildcard());
DCHECK_EQ(..., ...);

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
components/content_settings/core/browser/cookie_settings.cc:110: }
nit: no {}

[lshang, 2016-03-17 08:49:53]

Thanks Raymes!

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
File components/content_settings/core/browser/cookie_settings.cc (right):

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
components/content_settings/core/browser/cookie_settings.cc:109:
DCHECK(secondary_pattern == ContentSettingsPattern::Wildcard());
On 2016/03/17 04:21:29, raymes wrote:
> DCHECK_EQ(..., ...);

Didn't address this. DCHECK_EQ can only compare two values.

https://codereview.chromium.org/1694063002/diff/230001/components/content_set...
components/content_settings/core/browser/cookie_settings.cc:110: }
On 2016/03/17 04:21:29, raymes wrote:
> nit: no {}

Done.

[lshang, 2016-03-17 08:55:18]

lshang@chromium.org changed reviewers:
+ agl@chromium.org, bauerb@chromium.org, benwells@chromium.org

[lshang, 2016-03-17 08:55:19]

+OWNER

bauerb@: please review changes in chrome/browser/android/ and
chrome/browser/browsing_data

benwells@: please review changes in chrome/browser/extensions

agl@: please review changes in chrome/browser/net

Thanks!

[Bernhard Bauer, 2016-03-17 09:52:26]

bauerb@chromium.org changed reviewers:
+ finnur@chromium.org

[Bernhard Bauer, 2016-03-17 09:52:27]

The code changes mostly look good, but the existing code seems strange to me.
Finnur, do you know how that came to pass?

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:467:
ContentSettingsPattern::FromURLNoWildcard(url));
This probably should have been ContentSettingsPattern::FromURL() like in the
other cases; then you could just use SetCookieSetting() as well.

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:470: if (value
== -1) {
Okay, this is a bit confusing. This value should never appear -- the Java side
call this with the int value of a (Java) ContentSetting, which takes its values
from ContentSettingValues, which is generated from the C++ ContentSetting enum,
and -1 is not one of the enum values.

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:473: setting =
value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
This is also weird -- we set ALLOW if the value is non-zero, and BLOCK if it is
zero (which is actually DEFAULT!).

[agl, 2016-03-17 09:58:46]

LGTM for chrome/browser/net.

[Finnur, 2016-03-17 10:40:54]

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:470: if (value
== -1) {
All I remember is getting confused by this (or something similar) at one point
and looking into why this occurs an finding a somewhat rational explanation for
it. I don't remember the explanation and don't know if it still applies. It
might not.

I'm also struggling to remember how we exercise this code? Android only allows
toggling the master default between Allow and Block and it doesn't allow adding
exceptions per origin (like JavaScript does).

[Bernhard Bauer, 2016-03-17 16:04:47]

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:470: if (value
== -1) {
On 2016/03/17 10:40:54, Finnur wrote:
> All I remember is getting confused by this (or something similar) at one point
> and looking into why this occurs an finding a somewhat rational explanation
for
> it. I don't remember the explanation and don't know if it still applies. It
> might not.
> 
> I'm also struggling to remember how we exercise this code? Android only allows
> toggling the master default between Allow and Block and it doesn't allow
adding
> exceptions per origin (like JavaScript does).

So, I dabbled in some source code archeology, and the -1 was originally used as
the integer representation of a null Java ContentSetting. I'm fairly certain
we've since replaced that with one with the DEFAULT integer value.

SingleWebsitePreferences has calls to update cookie settings, but it could very
well be that this can't be called in practice, because there is no way to set
cookie exceptions in the UI (Javascript seems to be the same though, or am I
missing something?). So, should we remove this and the call sites in
SingleWebsitePreferences?

[Bernhard Bauer, 2016-03-17 16:11:58]

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:470: if (value
== -1) {
On 2016/03/17 16:04:46, Bernhard Bauer wrote:
> On 2016/03/17 10:40:54, Finnur wrote:
> > All I remember is getting confused by this (or something similar) at one
point
> > and looking into why this occurs an finding a somewhat rational explanation
> for
> > it. I don't remember the explanation and don't know if it still applies. It
> > might not.
> > 
> > I'm also struggling to remember how we exercise this code? Android only
allows
> > toggling the master default between Allow and Block and it doesn't allow
> adding
> > exceptions per origin (like JavaScript does).
> 
> So, I dabbled in some source code archeology, and the -1 was originally used
as
> the integer representation of a null Java ContentSetting. I'm fairly certain
> we've since replaced that with one with the DEFAULT integer value.
> 
> SingleWebsitePreferences has calls to update cookie settings, but it could
very
> well be that this can't be called in practice, because there is no way to set
> cookie exceptions in the UI (Javascript seems to be the same though, or am I
> missing something?). So, should we remove this and the call sites in
> SingleWebsitePreferences?

Hm, OTOH we will sync content settings (and other preferences) on Android soon,
so there might be a way to end up with exceptions even on Android.

[raymes, 2016-03-20 23:35:20]

I just did some digging in the code. I wonder if we pull on the strings, all the
Java side cookies code goes unused atm? Finnur, could you help us verify if
that's the case?

Just to move forward with this CL though, Bernhard: if we switch the
SetCookieException call to SetCookieSetting would you be ok with landing this?
We can file a bug separately to investigate the strange cookies code.

Thanks!

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:473: setting =
value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
On 2016/03/17 09:52:27, Bernhard Bauer wrote:
> This is also weird -- we set ALLOW if the value is non-zero, and BLOCK if it
is
> zero (which is actually DEFAULT!).

Yes that's weird! I wonder if this feature works properly or if it's just that
no one has ever really used it?

[lshang, 2016-03-21 00:57:02]

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:467:
ContentSettingsPattern::FromURLNoWildcard(url));
On 2016/03/17 09:52:27, Bernhard Bauer wrote:
> This probably should have been ContentSettingsPattern::FromURL() like in the
> other cases; then you could just use SetCookieSetting() as well.

We are planning to make all ContentSettingsType origin scoped. After that,
SetCookieSetting() will generate patterns from FromURLNoWildcard(). So maybe
leave this for now and add a TODO here?

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:470: if (value
== -1) {
On 2016/03/17 16:11:58, Bernhard Bauer wrote:
> On 2016/03/17 16:04:46, Bernhard Bauer wrote:
> > On 2016/03/17 10:40:54, Finnur wrote:
> > > All I remember is getting confused by this (or something similar) at one
> point
> > > and looking into why this occurs an finding a somewhat rational
explanation
> > for
> > > it. I don't remember the explanation and don't know if it still applies.
It
> > > might not.
> > > 
> > > I'm also struggling to remember how we exercise this code? Android only
> allows
> > > toggling the master default between Allow and Block and it doesn't allow
> > adding
> > > exceptions per origin (like JavaScript does).
> > 
> > So, I dabbled in some source code archeology, and the -1 was originally used
> as
> > the integer representation of a null Java ContentSetting. I'm fairly certain
> > we've since replaced that with one with the DEFAULT integer value.
> > 
> > SingleWebsitePreferences has calls to update cookie settings, but it could
> very
> > well be that this can't be called in practice, because there is no way to
set
> > cookie exceptions in the UI (Javascript seems to be the same though, or am I
> > missing something?). So, should we remove this and the call sites in
> > SingleWebsitePreferences?
> 
> Hm, OTOH we will sync content settings (and other preferences) on Android
soon,
> so there might be a way to end up with exceptions even on Android.

What do you think @finnur?

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:473: setting =
value ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
On 2016/03/17 09:52:27, Bernhard Bauer wrote:
> This is also weird -- we set ALLOW if the value is non-zero, and BLOCK if it
is
> zero (which is actually DEFAULT!).

Yeah, this doesn't make sense....

[benwells, 2016-03-21 06:04:04]

c/b/e lgtm

[Bernhard Bauer, 2016-03-21 09:03:17]

On 2016/03/20 23:35:20, raymes wrote:
> I just did some digging in the code. I wonder if we pull on the strings, all
the
> Java side cookies code goes unused atm? Finnur, could you help us verify if
> that's the case?

I think there is no way at the moment to set cookie exceptions through the UI,
but there might be exceptions coming from via Sync once we start syncing
preferences on Android. So, the code to show (and maybe delete?) exceptions
should stay IMO.

> Just to move forward with this CL though, Bernhard: if we switch the
> SetCookieException call to SetCookieSetting would you be ok with landing this?
> We can file a bug separately to investigate the strange cookies code.

Yeah, provided that we also fix the weird conversion from value to setting
(where "fix" probably just means passing the value straight through?).

[Bernhard Bauer, 2016-03-21 09:03:27]

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
File chrome/browser/android/preferences/website_preference_bridge.cc (right):

https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
chrome/browser/android/preferences/website_preference_bridge.cc:467:
ContentSettingsPattern::FromURLNoWildcard(url));
On 2016/03/21 00:57:02, Liu Shang wrote:
> On 2016/03/17 09:52:27, Bernhard Bauer wrote:
> > This probably should have been ContentSettingsPattern::FromURL() like in the
> > other cases; then you could just use SetCookieSetting() as well.
> 
> We are planning to make all ContentSettingsType origin scoped. After that,
> SetCookieSetting() will generate patterns from FromURLNoWildcard(). So maybe
> leave this for now and add a TODO here?

Eh, I'd rather make the behavior consistent now and then implement the spec'd
behavior change at once.

[Finnur, 2016-03-21 10:58:40]

Rietveld is broken for me at the moment -- won't let me answer comments in
this CL (but works for other CLs). Email it is.

My general thinking is:

I don't have an opinion on origins vs. pattern and the now vs. later
questions. I'll defer to you guys on that.

But I think we should get to the bottom of this potentially problematic
code now, before we start sync'ing cookie exceptions to Android. Either fix
or delete the erroneous code, and I think you (lshang) are in the best
position to do so -- with an up-to-date Android tree and an in-progress CL
that makes sense to include such changes.

I suggest you programmatically add a cookie exception and try to modify it
using the UI. See what happens. If it works, we'd be surprised but it would
be good to investigate why it works. If it doesn't work (more likely), I'd
suggest switching this to use ContentSettingException for Cookies, sort of
like what we did for JavaScript and Popups in this CL:

https://codereview.chromium.org/1068223002/diff/160001/chrome/android/java/sr...

And then I suspect you could simplify/delete a bunch of code.

On Mon, Mar 21, 2016 at 9:03 AM, <bauerb@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
> File chrome/browser/android/preferences/website_preference_bridge.cc
> (right):
>
>
>
https://codereview.chromium.org/1694063002/diff/250001/chrome/browser/android...
> chrome/browser/android/preferences/website_preference_bridge.cc:467:
> ContentSettingsPattern::FromURLNoWildcard(url));
> On 2016/03/21 00:57:02, Liu Shang wrote:
> > On 2016/03/17 09:52:27, Bernhard Bauer wrote:
> > > This probably should have been ContentSettingsPattern::FromURL()
> like in the
> > > other cases; then you could just use SetCookieSetting() as well.
> >
> > We are planning to make all ContentSettingsType origin scoped. After
> that,
> > SetCookieSetting() will generate patterns from FromURLNoWildcard(). So
> maybe
> > leave this for now and add a TODO here?
>
> Eh, I'd rather make the behavior consistent now and then implement the
> spec'd behavior change at once.
>
> https://codereview.chromium.org/1694063002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[lshang, 2016-03-22 05:38:49]

On 2016/03/21 09:03:17, Bernhard Bauer wrote:
> On 2016/03/20 23:35:20, raymes wrote:
> > I just did some digging in the code. I wonder if we pull on the strings, all
> the
> > Java side cookies code goes unused atm? Finnur, could you help us verify if
> > that's the case?
> 
> I think there is no way at the moment to set cookie exceptions through the UI,
> but there might be exceptions coming from via Sync once we start syncing
> preferences on Android. So, the code to show (and maybe delete?) exceptions
> should stay IMO.
> 
> > Just to move forward with this CL though, Bernhard: if we switch the
> > SetCookieException call to SetCookieSetting would you be ok with landing
this?
> > We can file a bug separately to investigate the strange cookies code.
> 
> Yeah, provided that we also fix the weird conversion from value to setting
> (where "fix" probably just means passing the value straight through?).

Hi Bernhard, I have changed it to use SetCookieSetting() and pass through the
value. Could you PTAL again? Thanks!

[lshang, 2016-03-22 05:41:26]

On 2016/03/21 10:58:40, Finnur wrote:
> Rietveld is broken for me at the moment -- won't let me answer comments in
> this CL (but works for other CLs). Email it is.
> 
> My general thinking is:
> 
> I don't have an opinion on origins vs. pattern and the now vs. later
> questions. I'll defer to you guys on that.
> 
> But I think we should get to the bottom of this potentially problematic
> code now, before we start sync'ing cookie exceptions to Android. Either fix
> or delete the erroneous code, and I think you (lshang) are in the best
> position to do so -- with an up-to-date Android tree and an in-progress CL
> that makes sense to include such changes.
> 
> I suggest you programmatically add a cookie exception and try to modify it
> using the UI. See what happens. If it works, we'd be surprised but it would
> be good to investigate why it works. If it doesn't work (more likely), I'd
> suggest switching this to use ContentSettingException for Cookies, sort of
> like what we did for JavaScript and Popups in this CL:
> 

Thanks for your patient explanation Finnur! I'm happy to help with that:-)

[Finnur, 2016-03-22 10:36:05]

No problem. Happy to help. And thank you, Liu. :)

On Tue, Mar 22, 2016 at 5:41 AM, <lshang@chromium.org> wrote:

> On 2016/03/21 10:58:40, Finnur wrote:
> > Rietveld is broken for me at the moment -- won't let me answer comments
> in
> > this CL (but works for other CLs). Email it is.
> >
> > My general thinking is:
> >
> > I don't have an opinion on origins vs. pattern and the now vs. later
> > questions. I'll defer to you guys on that.
> >
> > But I think we should get to the bottom of this potentially problematic
> > code now, before we start sync'ing cookie exceptions to Android. Either
> fix
> > or delete the erroneous code, and I think you (lshang) are in the best
> > position to do so -- with an up-to-date Android tree and an in-progress
> CL
> > that makes sense to include such changes.
> >
> > I suggest you programmatically add a cookie exception and try to modify
> it
> > using the UI. See what happens. If it works, we'd be surprised but it
> would
> > be good to investigate why it works. If it doesn't work (more likely),
> I'd
> > suggest switching this to use ContentSettingException for Cookies, sort
> of
> > like what we did for JavaScript and Popups in this CL:
> >
>
> Thanks for your patient explanation Finnur! I'm happy to help with that:-)
>
>
>
>
>
> https://codereview.chromium.org/1694063002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Bernhard Bauer, 2016-03-22 10:37:52]

LGTM!

[lshang, 2016-03-22 23:26:57]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-03-22 23:26:57]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
benwells@chromium.org, agl@chromium.org
Link to the patchset: https://codereview.chromium.org/1694063002/#ps330001
(title: "remove unused patterns")

[commit-bot: I haz the power, 2016-03-22 23:27:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1694063002/330001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1694063002/330001

[commit-bot: I haz the power, 2016-03-22 23:32:39]

Description was changed from

==========
Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For 
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747
==========

to

==========
Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For 
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747
==========

[commit-bot: I haz the power, 2016-03-22 23:32:41]

Committed patchset #14 (id:330001)

[commit-bot: I haz the power, 2016-03-22 23:34:14]

Description was changed from

==========
Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of 
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For 
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747
==========

to

==========
Use GURLS instead of patterns in SetCookieSetting()

A new version SetContentSettingDefaultScope() is made to use GURLs instead of
patterns. The patterns are determined by WebsiteSettingsInfo::ScopingType from
the ContentSettingsType.

In this CL, SetCookieSetting() is changed to use GURLs instead of patterns as
parameters and it will call SetContentSettingDefaultScope() internally. For
special test cases, use SetContentSetting() to pass in specific patterns.

BUG=551747

Committed: https://crrev.com/b1a182cafba0902104512f93e641617e9b15ba18
Cr-Commit-Position: refs/heads/master@{#382728}
==========

[commit-bot: I haz the power, 2016-03-22 23:34:16]

Patchset 14 (id:??) landed as
https://crrev.com/b1a182cafba0902104512f93e641617e9b15ba18
Cr-Commit-Position: refs/heads/master@{#382728}