Enable fast user switching in Win8/Ash mode.

Enable fast user switching in Win8/Ash mode.

TEST=
1. Launch chrome, create two users and Relaunch browser in Win 8 mode from wrench menu.
2. Right click on avatar bubble at top right corner and observe.
3. Fast User Switcher should be shown.

BUG=467895
Committed: https://crrev.com/7dc985da4a3f3cf821023942a50b271787c48625
Cr-Commit-Position: refs/heads/master@{#321792}

[anthonyvd, 2015-03-17 21:16:29]

anthonyvd@chromium.org changed reviewers:
+ grt@chromium.org, sky@chromium.org

[anthonyvd, 2015-03-17 21:16:29]

Hi guys, can you please take a quick look at this change that enables fast user
switching in win8/metro mode?

grt@chromium.org: chrome/app/chrome_command_ids.h
sky@chromium.org: chrome/browser/ui/*

Thanks a lot for your time :)

[sky, 2015-03-17 23:09:34]

sky@chromium.org changed reviewers:
+ cpu@chromium.org

[sky, 2015-03-17 23:09:35]

For some reason I didn't think we wanted a user switcher for ash/win8. cpu, am I
wrong there? Does doing a switch update everything correctly, say the launcher
along the bottom of the screen?

[cpu_(ooo_6.6-7.5), 2015-03-17 23:37:09]

cpu@chromium.org changed reviewers:
+ tapted@chromium.org

[cpu_(ooo_6.6-7.5), 2015-03-17 23:37:10]

the issue before and still might be is that the ash shell itself was unaware of
the switch, so basically your calculator app was running as /defaut/ and your
chrome window would be running as somebody else.

I was told this was bad and the reason why chrome os switcher did not mix
different users in the same ash desktop. 

So we followed suit.

That was long time ago, I don't know if that still holds.

tapted@ who from cr-apps should look at this.

[tapted, 2015-03-18 00:32:04]

On 2015/03/17 23:37:10, cpu wrote:
> the issue before and still might be is that the ash shell itself was unaware
of
> the switch, so basically your calculator app was running as /defaut/ and your
> chrome window would be running as somebody else.

That's definitely still the case. While the app launcher has switching code for
desktop, it's disabled in Ash (and the Profile it uses for Metro mode is
effectively hardcoded, to match the ash::Shell). The other shelf-bits don't
support any profile switching in metro mode that I'm aware of. I think you'd
need to tear down and replace the whole ash::Shell which I don't think anyone
has looked into for Metro mode.

However, there are already other ways to get browser windows with a mix of
profiles at the same time in Metro mode, since any browser launch from Desktop
mode (e.g. profile-pinned desktop shortcuts) requested while Chrome is
configured for Metro mode will open a browser with that profile in a Metro
session. But that's a tricky state to get into, and only for people who have
already opted-in to multi-profile, I'd be hesitant to make it easier to get in
that state.

I'm a bit out of touch with metro mode these days.. but I'm little surprised the
avatar button is there at all for Metro mode. Even adding a user is a confusing
experience. I'm pretty sure there is no way to have the app launcher showing
anything other that 'Default' (i.e. no way to show Profile 1, 2, etc.). Even if
you delete the profile under `$USER_DATA_DIR/Default` from desktop mode, the
next time you open Metro mode it would just re-create an empty profile there.

[anthonyvd, 2015-03-18 14:04:24]

Metro mode indeed has the new Avatar Button so it's really easy to get multiple
windows of different profiles (Avatar Button -> Switch person -> Choose a
different profile in the user manager). Since that's the same UI as desktop
mode, it might make sense to allow right-click user switching in a consistent
way (it's enabled as of M42 on desktop). 

CC our PM joberbeck@ who might have some insight as to the rationale behind the
Avatar Button/Profile Switching.

[sky, 2015-03-18 15:58:00]

We're not against showing the button per se, our concern is the rest
of the system isn't going to do the right thing and will end up in a
weird and confusing state.

  -Scott

On Wed, Mar 18, 2015 at 7:04 AM,  <anthonyvd@chromium.org> wrote:
> Metro mode indeed has the new Avatar Button so it's really easy to get
> multiple
> windows of different profiles (Avatar Button -> Switch person -> Choose a
> different profile in the user manager). Since that's the same UI as desktop
> mode, it might make sense to allow right-click user switching in a
> consistent
> way (it's enabled as of M42 on desktop).
>
> CC our PM joberbeck@ who might have some insight as to the rationale behind
> the
> Avatar Button/Profile Switching.
>
> https://codereview.chromium.org/1011233002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cpu_(ooo_6.6-7.5), 2015-03-18 21:58:32]

the avatar button is there because I failed to realize we needed to remove it.
That is, most things we don't want in metro we need to add code (or ifdefs) to
remove them.

But frankly, if the cr apps folks are ok with the issues then I am ok as well.

Now if we go this path then we need to restore the profile management view
(which is disabled for metro) from the settings page.

[tapted, 2015-03-18 22:28:46]

On 2015/03/18 21:58:32, cpu wrote:
> But frankly, if the cr apps folks are ok with the issues then I am ok as well.

If UX/QA are OK with it, I think this is fine (although I'm secretly hoping that
using multiple-profiles within Metro mode doesn't catch on). For example, it's
already confusing for anyone who made a `Profile 1` user and ignored the
`Default` profile that came on first run.

I think there's only been real strife when someone tries to delete the Default
profile within metro mode (e.g. http://crbug.com/448352 ).

And the code here looks reasonable, so you have my non-owner lgtm :)

[sky, 2015-03-18 23:08:03]

On 2015/03/17 23:37:10, cpu wrote:
> the issue before and still might be is that the ash shell itself was unaware
of
> the switch, so basically your calculator app was running as /defaut/ and your
> chrome window would be running as somebody else.
> 
> I was told this was bad and the reason why chrome os switcher did not mix
> different users in the same ash desktop. 
> 
> So we followed suit.
> 
> That was long time ago, I don't know if that still holds.
> 
> tapted@ who from cr-apps should look at this.

Aren't there other things though, such as the launcher and notifications and
what not that need to update too?

[sky, 2015-03-18 23:11:59]

sky@chromium.org changed reviewers:
+ skuhne@chromium.org

[sky, 2015-03-18 23:12:17]

+skuhne as he knows more about whether the launcher and other things will update
correctly here.

[chromium-reviews, 2015-03-18 23:39:31]

After playing around with this, I think that IF we show the User Menu,
there's no reason to not show the fast user switcher.

I think we're okay showing the fast user switcher.  I imagine the primary
use case of multi-profile within Ash will be for power users, i.e. one
human with two profiles.  In that case I'm fine having the ash UI be sticky
to the primary profile.

On Wed, Mar 18, 2015 at 4:12 PM, <sky@chromium.org> wrote:

> +skuhne as he knows more about whether the launcher and other things will
> update
> correctly here.
>
> https://codereview.chromium.org/1011233002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2015-03-18 23:40:03]

>
> I think we're okay showing the fast user switcher

s/fast user switcher/user menu

On Wed, Mar 18, 2015 at 4:39 PM, John Oberbeck <joberbeck@google.com> wrote:

> After playing around with this, I think that IF we show the User Menu,
> there's no reason to not show the fast user switcher.
>
> I think we're okay showing the fast user switcher.  I imagine the primary
> use case of multi-profile within Ash will be for power users, i.e. one
> human with two profiles.  In that case I'm fine having the ash UI be sticky
> to the primary profile.
>
> On Wed, Mar 18, 2015 at 4:12 PM, <sky@chromium.org> wrote:
>
>> +skuhne as he knows more about whether the launcher and other things will
>> update
>> correctly here.
>>
>> https://codereview.chromium.org/1011233002/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mr4D (OOO till 08-26), 2015-03-19 00:37:08]

On 2015/03/18 23:12:17, sky wrote:
> +skuhne as he knows more about whether the launcher and other things will
update
> correctly here.

I would need to play with Windows8 to see if / what is missing.

E.g.: Do we change the wallpaper, shelf (orientation / items / ..)?

Does anyone have a Win8 system with this version on it to play around with for a
few minutes?

[anthonyvd, 2015-03-19 13:46:13]

On 2015/03/19 00:37:08, Mr4D wrote:
> On 2015/03/18 23:12:17, sky wrote:
> > +skuhne as he knows more about whether the launcher and other things will
> update
> > correctly here.
> 
> I would need to play with Windows8 to see if / what is missing.
> 
> E.g.: Do we change the wallpaper, shelf (orientation / items / ..)?
> 
> Does anyone have a Win8 system with this version on it to play around with for
a
> few minutes?

I do but I'm in a remote office. However, there shouldn't be anything different
if you just launch stable or beta in Win8 mode. Profile switching is the same,
this only enables switching with a right-click instead of going through the User
Manager.

[Mr4D (OOO till 08-26), 2015-03-19 13:56:23]

On 2015/03/19 13:46:13, anthonyvd wrote:
> On 2015/03/19 00:37:08, Mr4D wrote:
> > On 2015/03/18 23:12:17, sky wrote:
> > > +skuhne as he knows more about whether the launcher and other things will
> > update
> > > correctly here.
> > 
> > I would need to play with Windows8 to see if / what is missing.
> > 
> > E.g.: Do we change the wallpaper, shelf (orientation / items / ..)?
> > 
> > Does anyone have a Win8 system with this version on it to play around with
for
> a
> > few minutes?
> 
> I do but I'm in a remote office. However, there shouldn't be anything
different
> if you just launch stable or beta in Win8 mode. Profile switching is the same,
> this only enables switching with a right-click instead of going through the
User
> Manager.

I do neither have Windows 8 - nor do I have a recent windows development system.
Getting to a running state will take therefore hours.  Anyone you can recommend
here in M43?

[Mr4D (OOO till 08-26), 2015-03-19 14:33:16]

Since this has only an effect on the window itself and not installed app's or
the launcher, etc. this should be fine.

lgtm

[grt (UTC plus 2), 2015-03-19 18:08:17]

rubberstamp lgtm on chrome/app/chrome_command_ids.h

[sky, 2015-03-19 19:29:49]

LGTM

[cpu_(ooo_6.6-7.5), 2015-03-20 19:55:07]

lgtm assuming that anthonyvd is willing to enable a couple more things have we
hidden.

[anthonyvd, 2015-03-21 00:42:22]

On 2015/03/20 19:55:07, cpu wrote:
> lgtm assuming that anthonyvd is willing to enable a couple more things have we
> hidden.

I'd be happy to, what kind of stuff are we talking about?

[anthonyvd, 2015-03-23 15:49:33]

The CQ bit was checked by anthonyvd@chromium.org

[commit-bot: I haz the power, 2015-03-23 15:49:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1011233002/1

[commit-bot: I haz the power, 2015-03-23 15:53:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-23 15:53:24]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[anthonyvd, 2015-03-23 16:02:40]

The CQ bit was checked by anthonyvd@chromium.org

[anthonyvd, 2015-03-23 16:02:40]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
cpu@chromium.org, grt@chromium.org, skuhne@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1011233002/#ps20001
(title: "Rebase + fix conflict")

[commit-bot: I haz the power, 2015-03-23 16:02:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1011233002/20001

[commit-bot: I haz the power, 2015-03-23 16:53:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-23 16:55:24]

Patchset 2 (id:??) landed as
https://crrev.com/7dc985da4a3f3cf821023942a50b271787c48625
Cr-Commit-Position: refs/heads/master@{#321792}