Deprecate and re-wire the ClearSiteDataOnExit policy.

Deprecate and re-wire the ClearSiteDataOnExit policy.

This becomes necessary after Chrome has abandoned prefs::kClearSiteDataOnExit.

BUG=chromium:130986
TEST=unit tests, configure ClearSiteDataOnExit policy and make sure that it limits the cookie content setting to session-only.


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

[Mattias Nissler (ping if slow), 2012-06-18 15:29:45]

Please review.

[pastarmovj, 2012-06-18 15:42:16]

LGTM with a few nits.

http://codereview.chromium.org/10562036/diff/1/chrome/app/policy/policy_templ...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10562036/diff/1/chrome/app/policy/policy_templ...
chrome/app/policy/policy_templates.json:750: 'desc': '''This policy is an
override for the "Clear cookies and other site data when I close my browser"
content settings option that <ph name="PRODUCT_NAME">$1<ex>Google
Chrome</ex></ph> supported previous to version 21.
isn't it "prior to version..." instead to "previous to..."

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
File chrome/browser/policy/configuration_policy_handler.cc (right):

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
chrome/browser/policy/configuration_policy_handler.cc:1115:
errors->AddError(key::kDefaultCookiesSetting, IDS_POLICY_OVERRIDDEN,
Split the two params on separate lines.

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
File chrome/browser/policy/configuration_policy_handler.h (right):

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
chrome/browser/policy/configuration_policy_handler.h:359: // Handles the
(deprecated) ClearSiteDataOnExit policy.
Maybe add a TODO here to remove it in the near future with a link to a bug
tracking that?

[Mattias Nissler (ping if slow), 2012-06-18 16:28:51]

Thanks for the review, I just fired another round of try jobs and plan to tick
the box if they come back green.

http://codereview.chromium.org/10562036/diff/1/chrome/app/policy/policy_templ...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10562036/diff/1/chrome/app/policy/policy_templ...
chrome/app/policy/policy_templates.json:750: 'desc': '''This policy is an
override for the "Clear cookies and other site data when I close my browser"
content settings option that <ph name="PRODUCT_NAME">$1<ex>Google
Chrome</ex></ph> supported previous to version 21.
On 2012/06/18 15:42:16, pastarmovj wrote:
> isn't it "prior to version..." instead to "previous to..."

Done.

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
File chrome/browser/policy/configuration_policy_handler.cc (right):

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
chrome/browser/policy/configuration_policy_handler.cc:1115:
errors->AddError(key::kDefaultCookiesSetting, IDS_POLICY_OVERRIDDEN,
On 2012/06/18 15:42:16, pastarmovj wrote:
> Split the two params on separate lines.

Not required by the style guide, but the rest of the file seems to do it. Done.

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
File chrome/browser/policy/configuration_policy_handler.h (right):

http://codereview.chromium.org/10562036/diff/1/chrome/browser/policy/configur...
chrome/browser/policy/configuration_policy_handler.h:359: // Handles the
(deprecated) ClearSiteDataOnExit policy.
On 2012/06/18 15:42:16, pastarmovj wrote:
> Maybe add a TODO here to remove it in the near future with a link to a bug
> tracking that?

Done.

[commit-bot: I haz the power, 2012-06-18 17:58:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/2002

[commit-bot: I haz the power, 2012-06-18 19:50:14]

Try job failure for 10562036-2002 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-18 20:19:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/2002

[commit-bot: I haz the power, 2012-06-18 20:31:59]

Try job failure for 10562036-2002 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-18 23:25:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/2002

[commit-bot: I haz the power, 2012-06-19 03:06:52]

Try job failure for 10562036-2002 (retry) (retry) on win_rel for step
"chrome_frame_net_tests".
It's a second try, previously, step "chrome_frame_net_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-06-19 08:16:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/2002

[Joao da Silva, 2012-06-19 10:23:55]

Drive by nits.

http://codereview.chromium.org/10562036/diff/2002/chrome/app/policy/policy_te...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10562036/diff/2002/chrome/app/policy/policy_te...
chrome/app/policy/policy_templates.json:754: If this policy is enabled, will be
translated to the appropriate policy setting for the DefaultCookiesSetting
policy, setting it to session-only if the DefaultCookiesSetting policy is not
configured or set to Allow.
Nit: "If this policy is enable, _it_ will be ..."

http://codereview.chromium.org/10562036/diff/2002/chrome/browser/policy/confi...
File chrome/browser/policy/configuration_policy_handler_list.cc (right):

http://codereview.chromium.org/10562036/diff/2002/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list.cc:324:
handlers_.push_back(new ClearSiteDataOnExitPolicyHandler());
Nit: these were in alphabetical order.

[Mattias Nissler (ping if slow), 2012-06-20 08:15:40]

http://codereview.chromium.org/10562036/diff/2002/chrome/app/policy/policy_te...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10562036/diff/2002/chrome/app/policy/policy_te...
chrome/app/policy/policy_templates.json:754: If this policy is enabled, will be
translated to the appropriate policy setting for the DefaultCookiesSetting
policy, setting it to session-only if the DefaultCookiesSetting policy is not
configured or set to Allow.
On 2012/06/19 10:23:55, Joao da Silva wrote:
> Nit: "If this policy is enable, _it_ will be ..."

Done.

http://codereview.chromium.org/10562036/diff/2002/chrome/browser/policy/confi...
File chrome/browser/policy/configuration_policy_handler_list.cc (right):

http://codereview.chromium.org/10562036/diff/2002/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list.cc:324:
handlers_.push_back(new ClearSiteDataOnExitPolicyHandler());
On 2012/06/19 10:23:55, Joao da Silva wrote:
> Nit: these were in alphabetical order.

Done.

[commit-bot: I haz the power, 2012-06-20 08:15:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/2002

[commit-bot: I haz the power, 2012-06-20 08:16:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnissler@chromium.org/10562036/18003

[commit-bot: I haz the power, 2012-06-20 10:42:52]

Change committed as 143163