Changes to support new cookie policy....

Changes to support new cookie policy.

Changes:
1- net::CookiePolicy becomes an interface.
2- Old implementaiton of CookiePolicy copied to StaticCookiePolicy.
3- ChromeULRRequestContext implements CookiePolicy.
4- HostContentSettingsMap gets a global "BlockThirdPartyCookies" pref.

R=pkasting


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

[darin (slow to review), 2010-01-30 06:45:52]

It'd be great if you could verify that I added the third-party cookie blocking
setting where you wanted it.  If so, then I'll go and hook it up to the pref
system.  I can migrate the existing cookie behavior pref while I'm at it.

I also haven't implemented the ASK code yet.

[Peter Kasting, 2010-01-30 06:58:15]

LGTM, only 2 substantive comments

http://codereview.chromium.org/556095/diff/4001/4012
File chrome/browser/automation/url_request_automation_job.cc (right):

http://codereview.chromium.org/556095/diff/4001/4012#newcode1
chrome/browser/automation/url_request_automation_job.cc:1: // Copyright (c) 2009
The Chromium Authors. All rights reserved.
Nit: 2010

http://codereview.chromium.org/556095/diff/4001/4013
File chrome/browser/net/chrome_url_request_context.cc (right):

http://codereview.chromium.org/556095/diff/4001/4013#newcode653
chrome/browser/net/chrome_url_request_context.cc:653: if (setting ==
CONTENT_SETTING_DEFAULT)
If you want, another option is to make the HostContentSettingsMap do this
fallback automatically behind the scenes.

I think this might make sense because it simplifies n callers at the cost of
making 1 callee more complex.

http://codereview.chromium.org/556095/diff/4001/4016
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/556095/diff/4001/4016#newcode1
chrome/browser/renderer_host/resource_message_filter.cc:1: // Copyright (c) 2009
The Chromium Authors. All rights reserved.
Nit: 2010

http://codereview.chromium.org/556095/diff/4001/4010
File chrome/browser/views/options/advanced_contents_view.cc (left):

http://codereview.chromium.org/556095/diff/4001/4010#oldcode455
chrome/browser/views/options/advanced_contents_view.cc:455:
IDS_OPTIONS_COOKIES_ACCEPT_ALL_COOKIES,
You can rip these strings out of generated_resources.grd.

http://codereview.chromium.org/556095/diff/4001/4002
File net/net.gyp (right):

http://codereview.chromium.org/556095/diff/4001/4002#newcode1
net/net.gyp:1: # Copyright (c) 2009 The Chromium Authors. All rights reserved.
Nit: 2010

http://codereview.chromium.org/556095/diff/4001/4008
File net/url_request/url_request_context.h (right):

http://codereview.chromium.org/556095/diff/4001/4008#newcode1
net/url_request/url_request_context.h:1: // Copyright (c) 2006-2008 The Chromium
Authors. All rights reserved.
Nit: 2010

http://codereview.chromium.org/556095/diff/4001/4007
File net/url_request/url_request_http_job.cc (right):

http://codereview.chromium.org/556095/diff/4001/4007#newcode1
net/url_request/url_request_http_job.cc:1: // Copyright (c) 2006-2009 The
Chromium Authors. All rights reserved.
Nit: 2010

[darin (slow to review), 2010-01-30 07:20:38]

Thanks for the review.

http://codereview.chromium.org/556095/diff/4001/4013
File chrome/browser/net/chrome_url_request_context.cc (right):

http://codereview.chromium.org/556095/diff/4001/4013#newcode653
chrome/browser/net/chrome_url_request_context.cc:653: if (setting ==
CONTENT_SETTING_DEFAULT)
Yeah, that's probably a good idea.

http://codereview.chromium.org/556095/diff/4001/4010
File chrome/browser/views/options/advanced_contents_view.cc (left):

http://codereview.chromium.org/556095/diff/4001/4010#oldcode455
chrome/browser/views/options/advanced_contents_view.cc:455:
IDS_OPTIONS_COOKIES_ACCEPT_ALL_COOKIES,
On 2010/01/30 06:58:15, Peter Kasting wrote:
> You can rip these strings out of generated_resources.grd.

I'm going to postpone since these are still referenced by the Mac and Linux
ports.  I don't want to muck with the UI for those ports in this CL.

[Peter Kasting, 2010-01-30 07:24:20]

http://codereview.chromium.org/556095/diff/4001/4010
File chrome/browser/views/options/advanced_contents_view.cc (left):

http://codereview.chromium.org/556095/diff/4001/4010#oldcode455
chrome/browser/views/options/advanced_contents_view.cc:455:
IDS_OPTIONS_COOKIES_ACCEPT_ALL_COOKIES,
On 2010/01/30 07:20:38, darin wrote:
> On 2010/01/30 06:58:15, Peter Kasting wrote:
> > You can rip these strings out of generated_resources.grd.
> 
> I'm going to postpone since these are still referenced by the Mac and Linux
> ports.  I don't want to muck with the UI for those ports in this CL.

Good call.  But we should probably make a list somewhere of things to do to
clean up later, and this should be on it...

[Peter Kasting, 2010-01-30 08:03:11]

The linter complained about missing newlines, I dunno if all its complaints are
real.

Basically LGTM, I think you can simplify the HostContentSettingsMap code.

http://codereview.chromium.org/556095/diff/5002/5010
File chrome/browser/host_content_settings_map.cc (right):

http://codereview.chromium.org/556095/diff/5002/5010#newcode85
chrome/browser/host_content_settings_map.cc:85: default_content_settings_ :
AssignDefaults(i->second);
I'd probably just roll that function in here:

if (i == host_content_settings_.end())
  return default_content_settings_;
for (int i = CONTENT_SETTINGS_FIRST_TYPE; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
  ...

Doing this also lets you save making an "output" object and copying the input to
the output when it doesn't need changing.

http://codereview.chromium.org/556095/diff/5002/5010#newcode202
chrome/browser/host_content_settings_map.cc:202: for (int i = 0; i <
CONTENT_SETTINGS_NUM_TYPES; ++i) {
Nit: To be consistent with other code, I'd use CONTENT_SETTINGS_FIRST_TYPE
instead of 0

http://codereview.chromium.org/556095/diff/5002/5010#newcode210
chrome/browser/host_content_settings_map.cc:210: }
Nit: Make sure you have a LF at EOF

http://codereview.chromium.org/556095/diff/5002/5021
File chrome/browser/net/cookie_policy_browsertest.cc (right):

http://codereview.chromium.org/556095/diff/5002/5021#newcode92
chrome/browser/net/cookie_policy_browsertest.cc:92: #endif
Nit: Make sure there is an LF at EOF

http://codereview.chromium.org/556095/diff/5002/5007
File net/base/static_cookie_policy.h (right):

http://codereview.chromium.org/556095/diff/5002/5007#newcode57
net/base/static_cookie_policy.h:57: #endif // NET_BASE_STATIC_COOKIE_POLICY_H_
Nit: Don't we do 2 spaces here?

Also, make sure there is an LF at EOF

[darin (slow to review), 2010-01-30 08:19:18]

http://codereview.chromium.org/556095/diff/5002/5010
File chrome/browser/host_content_settings_map.cc (right):

http://codereview.chromium.org/556095/diff/5002/5010#newcode85
chrome/browser/host_content_settings_map.cc:85: default_content_settings_ :
AssignDefaults(i->second);
good idea

[Avi (use Gerrit), 2010-02-01 20:52:38]

Followup cleanup at http://codereview.chromium.org/552265 .