Changes to "launch with multiple profiles" behavior.

Changes to "launch with multiple profiles" behavior.

- When started with --profile-directory, launch only one profile.
- Don't launch additional profiles which wouldn't open any pages. (When restarting
after an update, all profiles will reopen last pages.) The last active profile
is always launched.


BUG=99088, 111118
TEST=See bugs.


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

[marja, 2012-01-24 12:30:45]

Hi sky, here's yet another launch-with-multiple-profiles fix, ptal.

[sail, 2012-01-24 15:09:24]

http://codereview.chromium.org/9117024/diff/2002/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9117024/diff/2002/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:396: // Reset kProfilesLastActive to an
empty list to prevent launching the last
I don't understand why you need to clear this locate state pref when creating a
new profile.

What I would expect to happen is that non-active (front most) profiles only get
added to kProfilesLastActive list if setting is to re-open their last tabs.

Can you test that the following works:
  - Create two profiles, both with "reopen last tabs" set
  - Open a window for each profile with multiple tabs open
  - Exit chrome
Result: launching chrome should open both profiles. Does this work?

[marja, 2012-01-24 15:29:34]

Thanks for comments, logging some IM discussion here:

http://codereview.chromium.org/9117024/diff/2002/chrome/browser/chrome_browse...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/9117024/diff/2002/chrome/browser/chrome_browse...
chrome/browser/chrome_browser_main.cc:396: // Reset kProfilesLastActive to an
empty list to prevent launching the last
On 2012/01/24 15:09:24, sail wrote:
> I don't understand why you need to clear this locate state pref when creating
a
> new profile.

This is not the code for creating a new profile :)

> What I would expect to happen is that non-active (front most) profiles only
get
> added to kProfilesLastActive list if setting is to re-open their last tabs.

The kProfilesLastActive needs to be kept up to date (so that it contains all
active profiles, not only those which have the "reopen pages" setting), so that
we can recover from crashes and launch the correct profiles after restarting
because of an update.

> Can you test that the following works:
>   - Create two profiles, both with "reopen last tabs" set
>   - Open a window for each profile with multiple tabs open
>   - Exit chrome
> Result: launching chrome should open both profiles. Does this work?

Works. It doesn't hit this code path since switches::kProfileDirectory is not
passed.

Reformulated the comment to "Clear kProfilesLastActive since the user only wants
to launch a specific profile."

[sail, 2012-01-24 15:34:06]

lgtm

[sky, 2012-01-24 17:03:19]

This LGTM, but I have a question that can be dealt with separately.
What happens if you have multiple profiles open, crash and then restart?
Presumably you get the crashed infobar only for sessions that have restore
enabled (because of this patch) and clicking restore only restores for a
particular profile. I wonder if you should only open the first profile, but
restore should restore them all. It's worth taking with the UX folks for their
input.

[commit-bot: I haz the power, 2012-01-24 20:20:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/9117024/1004

[commit-bot: I haz the power, 2012-01-25 08:54:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/9117024/1004

[marja, 2012-01-25 09:18:33]

On 2012/01/24 17:03:19, sky wrote:
> What happens if you have multiple profiles open, crash and then restart?
> Presumably you get the crashed infobar only for sessions that have restore
> enabled (because of this patch) and clicking restore only restores for a
> particular profile. I wonder if you should only open the first profile, but
> restore should restore them all. It's worth taking with the UX folks for their
> input.

Thanks for pointing that out. Currently, only the "last active profile" gets the
crash infobar. Other active profiles with "reopen pages" just reopen the pages
as normal, and other active profiles without this setting are not launched at
all. This is not optimal.

We could either launch all active profiles and display the crash infobar for
each (then dismissing one infobar could also make the decision for all
profiles... or not). Or we could just launch the last active profile with the
infobar, and make it so that clicking "restore" would bring up the other active
profiles. (Otoh, if the user dismisses the infobar, it's a bit weird that then
the other profiles, which would've reopened last pages after a clean exit, are
just gone.)

[commit-bot: I haz the power, 2012-01-25 11:26:34]

Change committed as 119048