Exempt chrome-extension:// from third-party cookie rules, but not from regular content settings

Exempt chrome-extension:// from third-party cookie rules, but not from regular content settings

BUG=99588
TEST=CookieSettingsTest


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108664

[jochen (gone - plz use gerrit), 2011-11-04 09:44:52]

plz review

[Bernhard Bauer, 2011-11-04 10:00:55]

http://codereview.chromium.org/8462003/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/cookie_settings.cc (right):

http://codereview.chromium.org/8462003/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/cookie_settings.cc:44: //
HostContentSettingsMap::ShouldAllowAllContent.
Couldn't we create a single method for this?

http://codereview.chromium.org/8462003/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/cookie_settings.cc:45: bool
ShouldAllowAllContent(
Nit: I think the first parameter fits on this line?

[jochen (gone - plz use gerrit), 2011-11-04 10:59:08]

http://codereview.chromium.org/8462003/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/cookie_settings.cc (right):

http://codereview.chromium.org/8462003/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/cookie_settings.cc:44: //
HostContentSettingsMap::ShouldAllowAllContent.
On 2011/11/04 10:00:55, Bernhard Bauer wrote:
> Couldn't we create a single method for this?

Done.

[Bernhard Bauer, 2011-11-04 12:12:42]

http://codereview.chromium.org/8462003/diff/2001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8462003/diff/2001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:408: const GURL&
first_party_url,
I think we should swap these parameters, no? IIRC the first party URL is the
secondary URL for cookies, and for other content types we'll probably want to
use the scheme of the primary URL.

[jochen (gone - plz use gerrit), 2011-11-04 12:24:25]

http://codereview.chromium.org/8462003/diff/2001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8462003/diff/2001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:408: const GURL&
first_party_url,
On 2011/11/04 12:12:42, Bernhard Bauer wrote:
> I think we should swap these parameters, no? IIRC the first party URL is the
> secondary URL for cookies, and for other content types we'll probably want to
> use the scheme of the primary URL.

I still don't like this primary/secondary naming scheme, esp. now that cookie
settings are in another file :-/

anyway, i've renamed the parameters to primary/secondary, so it should be more
clear now, right?

[Bernhard Bauer, 2011-11-04 12:45:51]

http://codereview.chromium.org/8462003/diff/6001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8462003/diff/6001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:417: return
secondary_url.SchemeIs(chrome::kChromeDevToolsScheme) ||
Do we actually want to look at the secondary URL here? That would mean for
example that if Javascript is blocked, extensions can execute bundled scripts,
but not scripts included from somewhere else. Same goes for images and plug-ins.

[jochen (gone - plz use gerrit), 2011-11-04 14:16:45]

ptal

[Bernhard Bauer, 2011-11-04 14:22:34]

LGTM!

[commit-bot: I haz the power, 2011-11-04 14:25:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/8462003/1007

[commit-bot: I haz the power, 2011-11-04 16:16:05]

Change committed as 108664