Show sync promo at startup

Show sync promo at startup

This CL changes the startup behavior so that for new Chrome installs we now show the chrome://syncpromo page instead of the new tab page.

We also do the same thing when a user creates a new profile.

This CL also keeps track of the number of times the promo page has been shown so that we only show it 10 times.

In a later CL I'll add code to suppress the promo if the user is already signed into sync or if they have clicked the "Skip setup for now" button.

BUG=97779
TEST=


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

[sky, 2011-10-01 20:32:42]

This seems like it overlaps a bit with 95076.

http://codereview.chromium.org/8093016/diff/2001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager.cc (right):

http://codereview.chromium.org/8093016/diff/2001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:70: GURL url;
This doesn't seem right. Shouldn't we be calling into browser_init here?
Otherwise we're not doing session restore and other random things we might want
to trigger.

[sail, 2011-10-01 21:37:33]

http://codereview.chromium.org/8093016/diff/2001/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager.cc (right):

http://codereview.chromium.org/8093016/diff/2001/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager.cc:70: GURL url;
On 2011/10/01 20:32:42, sky wrote:
> This doesn't seem right. Shouldn't we be calling into browser_init here?
> Otherwise we're not doing session restore and other random things we might
want
> to trigger.

This code is only called if the user adds a new profile by clicking the "Add
profile" button. At this point we'd never want to do session restore or open the
welcome page here.

Are you sure we should still be calling browser_init from here?

[sky, 2011-10-01 22:23:03]

Wouldn't it mean you don't have to duplicate code?

  -Scott

[sail, 2011-10-01 22:59:43]

On 2011/10/01 22:23:03, sky wrote:
> Wouldn't it mean you don't have to duplicate code?
I think I would have to add code to browser_init to prevent it from doing things
we don't want to do (such as opening the welcome page).

[sky, 2011-10-03 14:14:16]

On 2011/10/01 22:59:43, sail wrote:
> On 2011/10/01 22:23:03, sky wrote:
> > Wouldn't it mean you don't have to duplicate code?
> I think I would have to add code to browser_init to prevent it from doing
things
> we don't want to do (such as opening the welcome page).

You know this is just the start and over time we'll want to share more
functionality. Making both code paths end up in the same place seems a lot
better from a maintainability stand point. I believe the case you mention here
could be controlled by tweaking prefs::kShouldShowWelcomePage before you call
into browser_init.

[sail, 2011-10-04 08:45:44]

changed to use BrowserInit, please take another look. Thanks.

[sky, 2011-10-04 15:33:53]

http://codereview.chromium.org/8093016/diff/9001/chrome/browser/ui/browser_in...
File chrome/browser/ui/browser_init.h (right):

http://codereview.chromium.org/8093016/diff/9001/chrome/browser/ui/browser_in...
chrome/browser/ui/browser_init.h:70: bool is_first_run, int* return_code);
Each param on its own line, here and all other methods.
Also, multiple booleans like this make the code nearly unreadable. Use an enum
(style guide encourages this too).

[sail, 2011-10-04 19:25:22]

http://codereview.chromium.org/8093016/diff/9001/chrome/browser/ui/browser_in...
File chrome/browser/ui/browser_init.h (right):

http://codereview.chromium.org/8093016/diff/9001/chrome/browser/ui/browser_in...
chrome/browser/ui/browser_init.h:70: bool is_first_run, int* return_code);
On 2011/10/04 15:33:53, sky wrote:
> Each param on its own line, here and all other methods.
> Also, multiple booleans like this make the code nearly unreadable. Use an enum
> (style guide encourages this too).

Done.

[sky, 2011-10-04 20:13:22]

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.cc (right):

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.cc:1292: startup_urls->push_back(GURL());  // New
tab page.
Might 1286 have already added this?

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.h (right):

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.h:233: IsFirstRun is_first_run_;
Feel free to use a boolean here. The enum is meant to clarify the call site, but
when you're in code like this is_first_run_ is better served by a boolean.

[sail, 2011-10-04 20:56:23]

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.cc (right):

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.cc:1292: startup_urls->push_back(GURL());  // New
tab page.
On 2011/10/04 20:13:23, sky wrote:
> Might 1286 have already added this?

Ouch, good catch. Don't know how that line got there.
Fixed.

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.h (right):

http://codereview.chromium.org/8093016/diff/12002/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.h:233: IsFirstRun is_first_run_;
On 2011/10/04 20:13:23, sky wrote:
> Feel free to use a boolean here. The enum is meant to clarify the call site,
but
> when you're in code like this is_first_run_ is better served by a boolean.

Done.

[sky, 2011-10-04 22:12:59]

LGTM

[commit-bot: I haz the power, 2011-10-04 22:14:07]

CQ is trying the patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8093016/16001

[commit-bot: I haz the power, 2011-10-04 23:12:19]

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

[commit-bot: I haz the power, 2011-10-04 23:26:16]

CQ is trying the patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8093016/11014

[commit-bot: I haz the power, 2011-10-05 00:25:52]

CQ is trying the patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8093016/11014

[commit-bot: I haz the power, 2011-10-05 02:51:52]

Change committed as 104054