Pass Setup Chrome Sync to main profile from Incognito.

Pass Setup Chrome Sync to main profile from Incognito.

Incognito mode doesn't have a sync service.

Additionally, if there is no browser window associated with the main
profile, create one.

BUG=90570, 90466
TEST="Set Up Chrome Sync" from Incognito wrench menu doesn't crash (and opens tab in main profile).
TEST=Test also works if all main-profile windows are closed.
TEST=On Mac, close all windows and select "Set Up Sync" from Chrome menu.


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

[akalin, 2011-07-27 17:27:53]

On 2011/07/26 21:04:55, shess wrote:

LGTM

[tim (not reviewing), 2011-07-27 18:10:27]

+qsr, as he's been trying to remove the Browser dep from ProfileSyncService.

[akalin, 2011-08-02 20:17:32]

On 2011/07/27 18:10:27, timsteele wrote:
> +qsr, as he's been trying to remove the Browser dep from ProfileSyncService.

Ping?

[qsr (NOT THE RIGHT qsr), 2011-08-03 07:13:02]

> Ping?

 Sorry, though it was for information only.

 I have nothing particular to say, except that this:

+  Browser* browser = BrowserList::GetLastActiveWithProfile(profile());
+  if (!browser) {
+    browser = Browser::Create(profile());
+    browser->ShowOptionsTab(chrome::kSyncSetupSubPage);
+    browser->window()->Show();
+  } else {
+    browser->ShowOptionsTab(chrome::kSyncSetupSubPage);
+  }

 should not be in this class because it is pure UI stuff, and sync should not
depends directly on Browser. But this is not something related to this bug, as
the line it replaces has exactly the same problem.

[Scott Hess - ex-Googler, 2011-08-03 17:19:16]

On 2011/08/03 07:13:02, qsr wrote:
> > Ping?
> 
>  Sorry, though it was for information only.
> 
>  I have nothing particular to say, except that this:
> 
> +  Browser* browser = BrowserList::GetLastActiveWithProfile(profile());
> +  if (!browser) {
> +    browser = Browser::Create(profile());
> +    browser->ShowOptionsTab(chrome::kSyncSetupSubPage);
> +    browser->window()->Show();
> +  } else {
> +    browser->ShowOptionsTab(chrome::kSyncSetupSubPage);
> +  }
> 
>  should not be in this class because it is pure UI stuff, and sync should not
> depends directly on Browser. But this is not something related to this bug, as
> the line it replaces has exactly the same problem.

Can I take that as "Unhappily acceptable for now?"  I'd improve things, but
would probably require hand-holding because I'm pretty sure I don't understand
the goals of your control-flow (for purposes of these bugs, the control-flow
seemed pretty brittle, but I'm betting it implements a particular UX goal).

[qsr (NOT THE RIGHT qsr), 2011-08-03 17:59:05]

> Can I take that as "Unhappily acceptable for now?"  I'd improve things, but
> would probably require hand-holding because I'm pretty sure I don't understand
> the goals of your control-flow (for purposes of these bugs, the control-flow
> seemed pretty brittle, but I'm betting it implements a particular UX goal).

 Yes. I only commented because of akalin's ping, I agree with you that
correcting this is not the aim of this CL.

[akalin, 2011-08-04 01:29:13]

Committing this.

On 2011/08/03 17:59:05, qsr wrote:
> > Can I take that as "Unhappily acceptable for now?"  I'd improve things, but
> > would probably require hand-holding because I'm pretty sure I don't
understand
> > the goals of your control-flow (for purposes of these bugs, the control-flow
> > seemed pretty brittle, but I'm betting it implements a particular UX goal).
> 
>  Yes. I only commented because of akalin's ping, I agree with you that
> correcting this is not the aim of this CL.

[commit-bot: I haz the power, 2011-08-04 03:22:59]

Change committed as 95381