Fix a crash on showing sync setup dialog in guest mode.

Fix a crash on showing sync setup dialog in guest mode.

Navigating directly to chrome://settings/syncSetup in guest mode is causing a DCHECK() failure, since the ProfileSyncService is NULL in the guest mode.

This patch adds a NULL check on ProfileSyncService to fix the crash
and also terminates the sync setup if the service is NULL.

BUG=370861
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269566

[Andrew T Wilson (Slow), 2014-05-07 14:25:42]

This seems to constantly revert - is it possible to write a test in this patch
to cover this case?

[Andrew T Wilson (Slow), 2014-05-07 14:28:20]

LGTM if you add a test for this case.

[Evan Stade, 2014-05-08 01:05:53]

Honestly, I think we don't care what happens if a user types an obscure
chrome:// address directly into the omnibox. Maybe someone can correct me.

[Andrew T Wilson (Slow), 2014-05-08 08:37:58]

On 2014/05/08 01:05:53, Evan Stade wrote:
> Honestly, I think we don't care what happens if a user types an obscure
> chrome:// address directly into the omnibox. Maybe someone can correct me.

Agreed, but I'm still OK with fixing this, since historically there have been
other ways to get to the chrome://settings/syncSetup page and it's always nice
not to crash the browser just because someone redirected to that page without
first checking whether they are in guest mode or sync is disabled. I mostly just
want to have a test because I've seen several regressions in this area over the
last few years.

[babu, 2014-05-08 11:57:13]

On 2014/05/07 14:25:42, Andrew T Wilson wrote:
> This seems to constantly revert - is it possible to write a test in this patch
> to cover this case?

I think we can have a test for this case when the GuestModeOptionsBrowserTest is
extended to the desktop platform. @engedy has created crbug.com/370841 to track
this.

[Andrew T Wilson (Slow), 2014-05-08 11:58:41]

On 2014/05/08 11:57:13, babu wrote:
> On 2014/05/07 14:25:42, Andrew T Wilson wrote:
> > This seems to constantly revert - is it possible to write a test in this
patch
> > to cover this case?
> 
> I think we can have a test for this case when the GuestModeOptionsBrowserTest
is
> extended to the desktop platform. @engedy has created crbug.com/370841 to
track
> this.

There's no reason to wait for that - can you just update SyncSetupHandlerTest,
similar to "ShowSyncSetupWhenNotSignedIn"?

[babu, 2014-05-09 00:18:05]

On 2014/05/08 11:58:41, Andrew T Wilson wrote:
> 
> There's no reason to wait for that - can you just update SyncSetupHandlerTest,
> similar to "ShowSyncSetupWhenNotSignedIn"?

@atwilson, Thanks for the hint. The failure was always reproducible when
IsSyncAccessible() returns false. IsSyncAccessible() will return false always if
IsManaged() returns true. So, added a test to cover this scenario.

[Andrew T Wilson (Slow), 2014-05-09 11:00:54]

LGTM - thanks for adding the test!

[babu, 2014-05-09 11:32:51]

On 2014/05/09 11:00:54, Andrew T Wilson wrote:
> LGTM - thanks for adding the test!

thanks for having an another look.

[babu, 2014-05-09 11:32:55]

The CQ bit was checked by sudarsana.nagineni@intel.com

[commit-bot: I haz the power, 2014-05-09 11:39:35]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/sudarsana.nagineni@intel.com/277433003...

[commit-bot: I haz the power, 2014-05-10 11:57:22]

Change committed as 269566