arc: Prevent showing Arc app window for secondary user profile.

arc: Prevent showing Arc app window for secondary user profile.

Arc has a limitation to be visible for primary profile only. However,
there are situations when Arc task can be started while the secondary
profile is shown. Before this led to case when Arc window was shown,
which is unexpected. This CL fixes this issue and prevents showing Arc
window for non-primary profiles.

Test=Manually on the device. Start Arc app in deferred mode and switch
     to the secondary profile fast. Arc app is started but window is not
     shown. Switch back to primary profile and window is automatically
     restored there.
BUG=b/31500778
BUG=647379
Committed: https://crrev.com/a4d9c53e0ca156489f3a9b970925594786bab1c0
Cr-Commit-Position: refs/heads/master@{#419497}

[khmel, 2016-09-15 20:13:43]

khmel@chromium.org changed reviewers:
+ reveman@chromium.org

[khmel, 2016-09-15 20:13:44]

Hi David,

PTAL

https://codereview.chromium.org/2345043002/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/arc_app_deferred_launcher_controller.cc
(right):

https://codereview.chromium.org/2345043002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_deferred_launcher_controller.cc:135:
arc::LaunchApp(observed_profile_, app_id);
Arc is associated with primary profile and we have to launch apps in this
context.

[reveman, 2016-09-16 00:37:12]

how do we handle similar cases for chrome apps? is this something that the
generic profile switching code should be aware of and have proper support for?

[khmel, 2016-09-16 18:08:16]

On 2016/09/16 00:37:12, reveman wrote:
> how do we handle similar cases for chrome apps? is this something that the
> generic profile switching code should be aware of and have proper support for?

Good question. Actually all visibility control is done in multi-user manager. So
this simplifies the fix. All we need is to add arc window to multi-user user on
the moment of activation during the secondary profile active and multiuser
manager automatically hides it.

[khmel, 2016-09-16 18:09:53]

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

[khmel, 2016-09-16 18:09:54]

Adding Stefan to review.

PTAL

[Mr4D (OOO till 08-26), 2016-09-16 18:59:56]

Apps and chrome browser pages are getting automatically tagged for a particular
user when their controller gets instantiated.

Since ARC++ windows do not have such a controller this is not happening.

Note also that windows which belong to everyone can simply not be tagged at all.

The MultiWindowManager is taking care of showing / hiding of user specific
windows and / or shelf items.

[Mr4D (OOO till 08-26), 2016-09-16 19:08:55]

Please have a look at the comment!

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
(right):

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:320:
AttachWindowToMultiUserManager(window);
Hmm.. This should only be needed in one place. Why do we need it here and not
only in AttachControllerToWindowIfNeeded?

IF we however need it here, we would have to also handle the !visible case here
as well since otherwise the window would not become visible anymore when the
user changes.
Note: The MultiUserWindowManager will keep track of current visibility upon
attaching (and changing of it).

[khmel, 2016-09-16 19:19:31]

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
(right):

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:320:
AttachWindowToMultiUserManager(window);
On 2016/09/16 19:08:55, Mr4D wrote:
> Hmm.. This should only be needed in one place. Why do we need it here and not
> only in AttachControllerToWindowIfNeeded?
> 
> IF we however need it here, we would have to also handle the !visible case
here
> as well since otherwise the window would not become visible anymore when the
> user changes.
> Note: The MultiUserWindowManager will keep track of current visibility upon
> attaching (and changing of it).

Let me summarize: 
1. AttachControllerToWindowIfNeeded creates shelf item as well and we don't want
shelf item be created for secondary profile. This is why if (observed_profile_
== owner()->GetProfile()) exists.

2. Exo component (components/exo) creates Arc window. At the moment of creation
we don't say here if this Arc window or not. After creation, exo attaches
required Arc attributes and call ->Show() for window.

3. At this moment new window is not attached to any user and we catch visibility
changed event in this callback. 

4. We cannot call  AttachControllerToWindowIfNeeded because it would create new
shelf icon for secondary profile.

5. That is why we register this window in multi-user manager. MU manager detects
that this window should not be shown for current profile and hides it. Not
window is hidden and under the control of multi-user manager.

6. Once user switches back to main profile, this window is automatically
restored and we can create shelf item for it.

Does this answer your question?

[Mr4D (OOO till 08-26), 2016-09-16 22:07:28]

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
(right):

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:320:
AttachWindowToMultiUserManager(window);
Okay. I understand that the icon would be shown since
AttachControllerToWindowIfNeeded does automatically create that as well.

However - I do not like the idea that the window's visibility control is done by
another side effect. Once there is a window it should be associated with a
profile.

Wouldn't it be possible to change "AttachControllerToWindowIfNeeded" to only add
the item when the profile is correct? Wouldn't that resolve this?

[khmel, 2016-09-16 22:38:45]

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
(right):

https://codereview.chromium.org/2345043002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:320:
AttachWindowToMultiUserManager(window);
On 2016/09/16 22:07:28, Mr4D wrote:
> Okay. I understand that the icon would be shown since
> AttachControllerToWindowIfNeeded does automatically create that as well.
> 
> However - I do not like the idea that the window's visibility control is done
by
> another side effect. Once there is a window it should be associated with a
> profile.
> 
> Wouldn't it be possible to change "AttachControllerToWindowIfNeeded" to only
add
> the item when the profile is correct? Wouldn't that resolve this?

Done as we discussed off-line

[Mr4D (OOO till 08-26), 2016-09-16 23:22:53]

Thanks! LGTM!

[khmel, 2016-09-19 17:16:19]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-09-19 17:16:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 17:49:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-19 17:52:00]

Description was changed from

==========
arc: Prevent showing Arc app window for secondary user profile.

Arc has a limitation to be visible for primary profile only. However,
there are situations when Arc task can be started while the secondary
profile is shown. Before this led to case when Arc window was shown,
which is unexpected. This CL fixes this issue and prevents showing Arc
window for non-primary profiles.

Test=Manually on the device. Start Arc app in deferred mode and switch
     to the secondary profile fast. Arc app is started but window is not
     shown. Switch back to primary profile and window is automatically
     restored there.
BUG=b/31500778
BUG=647379
==========

to

==========
arc: Prevent showing Arc app window for secondary user profile.

Arc has a limitation to be visible for primary profile only. However,
there are situations when Arc task can be started while the secondary
profile is shown. Before this led to case when Arc window was shown,
which is unexpected. This CL fixes this issue and prevents showing Arc
window for non-primary profiles.

Test=Manually on the device. Start Arc app in deferred mode and switch
     to the secondary profile fast. Arc app is started but window is not
     shown. Switch back to primary profile and window is automatically
     restored there.
BUG=b/31500778
BUG=647379

Committed: https://crrev.com/a4d9c53e0ca156489f3a9b970925594786bab1c0
Cr-Commit-Position: refs/heads/master@{#419497}
==========

[commit-bot: I haz the power, 2016-09-19 17:52:01]

Patchset 3 (id:??) landed as
https://crrev.com/a4d9c53e0ca156489f3a9b970925594786bab1c0
Cr-Commit-Position: refs/heads/master@{#419497}