Disable sync promo when RestoreOnStartupURLs policy is used

Honor the session.restore_on_startup_pref on first start

This CL fixes a regression introduced by CL 9481003 that made the browser ignore the session.restore_on_startup pref on first start.

BUG=125467
TEST=session.restore_on_startup, session.urls_to_restore_on_startup
TEST=and RestoreOnStartup policy are honored on first start as well

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

[Mattias Nissler (ping if slow), 2012-05-22 14:58:41]

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:1094:
EXPECT_TRUE(base::Value::CreateBooleanValue(false)->
You are leaking the created value here. An alternative would be

EXPECT_TRUE(base::FundamentalValue<bool>(false).Equals(out_sync_promo_enabled));

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/ui/webu...
File chrome/browser/ui/webui/sync_promo/sync_promo_ui.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/ui/webu...
chrome/browser/ui/webui/sync_promo/sync_promo_ui.cc:176: // automatically be the
case when the RestoreOnStartupURLs policy is set.
Please retain the master_preferences reference in the comment.

[bartfab (slow), 2012-05-22 15:30:54]

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:1094:
EXPECT_TRUE(base::Value::CreateBooleanValue(false)->
I leaked it on purpose because I figured in_urls was being leaked as well so it
probably made no difference. I see now that PolicyMap takes ownership.
Done.

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/ui/webu...
File chrome/browser/ui/webui/sync_promo/sync_promo_ui.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/ui/webu...
chrome/browser/ui/webui/sync_promo/sync_promo_ui.cc:176: // automatically be the
case when the RestoreOnStartupURLs policy is set.
On 2012/05/22 14:58:41, Mattias Nissler wrote:
> Please retain the master_preferences reference in the comment.

Done.

[Mattias Nissler (ping if slow), 2012-05-22 15:35:34]

LGTM, assuming you have actually tested this successfully. Would be pretty
simple to write a PyAuto for this one btw.

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/1/chrome/browser/policy/...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:1094:
EXPECT_TRUE(base::Value::CreateBooleanValue(false)->
On 2012/05/22 15:30:54, bartfab wrote:
> I leaked it on purpose because I figured in_urls was being leaked as well so
it
> probably made no difference. I see now that PolicyMap takes ownership.
> Done.

The rule is "Never leak a thing!", even in unit tests.

[bartfab (slow), 2012-05-23 12:43:04]

Hi Joao, can you please review?

[Joao da Silva, 2012-05-23 12:49:46]

lgtm

https://chromiumcodereview.appspot.com/10417032/diff/7001/chrome/browser/ui/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/7001/chrome/browser/ui/s...
chrome/browser/ui/startup/startup_browser_creator.cc:251: profile->GetPrefs()))
{
Nit: use |prefs| here too. This can probably be collapsed into line 250.

[bartfab (slow), 2012-05-23 12:55:10]

Hi Peter, could you do an OWNER review for browser/ui please?

https://chromiumcodereview.appspot.com/10417032/diff/7001/chrome/browser/ui/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://chromiumcodereview.appspot.com/10417032/diff/7001/chrome/browser/ui/s...
chrome/browser/ui/startup/startup_browser_creator.cc:251: profile->GetPrefs()))
{
On 2012/05/23 12:49:46, Joao da Silva wrote:
> Nit: use |prefs| here too. This can probably be collapsed into line 250.

Done.

[Peter Kasting, 2012-05-23 18:37:49]

LGTM

http://codereview.chromium.org/10417032/diff/9002/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

http://codereview.chromium.org/10417032/diff/9002/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_browser_creator.cc:235: PrefService *prefs =
profile->GetPrefs();
Nit: * goes on type

[bartfab (slow), 2012-05-23 18:44:00]

http://codereview.chromium.org/10417032/diff/9002/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

http://codereview.chromium.org/10417032/diff/9002/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_browser_creator.cc:235: PrefService *prefs =
profile->GetPrefs();
On 2012/05/23 18:37:49, Peter Kasting wrote:
> Nit: * goes on type

Done.

[commit-bot: I haz the power, 2012-05-23 18:45:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/10417032/7006

[commit-bot: I haz the power, 2012-05-23 22:20:24]

Change committed as 138617