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

chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc

Index: chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
index 5038c15ee2d7e8662ad46f5b51cf017612bf5050..4077abe01c80e56e80678cbe004fb47716076ff9 100644
--- a/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
@@ -87,6 +87,18 @@ blink::WebScreenOrientationLockType BlinkOrientationLockFromMojom(
   }
 }
 
+int GetWindowTaskId(aura::Window* window) {
+  const std::string window_app_id = exo::ShellSurface::GetApplicationId(window);
+  if (window_app_id.empty())
+    return -1;
+
+  int task_id = -1;
+  if (sscanf(window_app_id.c_str(), "org.chromium.arc.%d", &task_id) != 1)
+    return -1;
+
+  return task_id;
+}
+
 }  // namespace
 
 class ArcAppWindowLauncherController::AppWindow : public ui::BaseWindow {
@@ -292,13 +304,21 @@ void ArcAppWindowLauncherController::OnWindowInitialized(aura::Window* window) {
 void ArcAppWindowLauncherController::OnWindowVisibilityChanged(
     aura::Window* window,
     bool visible) {
+  if (!visible)
+    return;
   // The application id property should be set at this time. It is important to
   // have window->IsVisible set to true before attaching to a controller because
   // the window is registered in multi-user manager and this manager may
   // consider this new window as hidden for current profile. Multi-user manager
   // uses OnWindowVisibilityChanging event to update window state.
-  if (visible && observed_profile_ == owner()->GetProfile())
+  if (observed_profile_ == owner()->GetProfile()) {
     AttachControllerToWindowIfNeeded(window);
+  } else {
+    // Attach window to multi-user manager now to let it manage visibility state
+    // of the Arc window correctly.
+    if (GetWindowTaskId(window) > 0)
+      AttachWindowToMultiUserManager(window);

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

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:30]

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]

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:44]

Done as we discussed off-line 

+  }
 }
 
 void ArcAppWindowLauncherController::OnWindowDestroying(aura::Window* window) {
@@ -335,17 +355,18 @@ void ArcAppWindowLauncherController::AttachControllerToWindowsIfNeeded() {
     AttachControllerToWindowIfNeeded(window);
 }
 
-void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
+void ArcAppWindowLauncherController::AttachWindowToMultiUserManager(
     aura::Window* window) {
-  const std::string window_app_id = exo::ShellSurface::GetApplicationId(window);
-  if (window_app_id.empty())
-    return;
-
-  int task_id = -1;
-  if (sscanf(window_app_id.c_str(), "org.chromium.arc.%d", &task_id) != 1)
-    return;
+  DCHECK(window);
+  chrome::MultiUserWindowManager::GetInstance()->SetWindowOwner(
+      window,
+      user_manager::UserManager::Get()->GetPrimaryUser()->GetAccountId());
+}
 
-  if (!task_id)
+void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
+    aura::Window* window) {
+  const int task_id = GetWindowTaskId(window);
+  if (task_id <= 0)
     return;
 
   // We need to add the observer after exo started observing shell
@@ -375,9 +396,7 @@ void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
   DCHECK(app_window->controller());
   ash::WmWindowAura::Get(window)->SetIntProperty(
       ash::WmWindowProperty::SHELF_ID, app_window->shelf_id());
-  chrome::MultiUserWindowManager::GetInstance()->SetWindowOwner(
-      window,
-      user_manager::UserManager::Get()->GetPrimaryUser()->GetAccountId());
+  AttachWindowToMultiUserManager(window);
   if (ash::WmShell::Get()
           ->maximize_mode_controller()
           ->IsMaximizeModeWindowManagerEnabled()) {