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

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 #include "chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h"
 
 #include <string>
 
 #include "ash/aura/wm_window_aura.h"
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm/maximize_mode/maximize_mode_controller.h"
@@ skipping 69 matching lines @@
       return blink::WebScreenOrientationLockLandscapePrimary;
     case arc::mojom::OrientationLock::PORTRAIT_SECONDARY:
       return blink::WebScreenOrientationLockPortraitSecondary;
     case arc::mojom::OrientationLock::LANDSCAPE_SECONDARY:
       return blink::WebScreenOrientationLockLandscapeSecondary;
     default:
       return blink::WebScreenOrientationLockAny;
   }
 }
 
+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 {
  public:
   AppWindow(int task_id,
             const std::string app_id,
             views::Widget* widget,
             ArcAppWindowLauncherController* owner)
       : task_id_(task_id), app_id_(app_id), widget_(widget), owner_(owner) {}
   ~AppWindow() {}
@@ skipping 185 matching lines @@
   // Arc windows has type WINDOW_TYPE_NORMAL.
   if (window->type() != ui::wm::WINDOW_TYPE_NORMAL)
     return;
   observed_windows_.push_back(window);
   window->AddObserver(this);
 }
 
 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) {
   auto it =
       std::find(observed_windows_.begin(), observed_windows_.end(), window);
   DCHECK(it != observed_windows_.end());
   observed_windows_.erase(it);
   window->RemoveObserver(this);
 
   auto it_app_window =
@@ skipping 16 matching lines @@
   if (it == task_id_to_app_window_.end())
     return nullptr;
   return it->second.get();
 }
 
 void ArcAppWindowLauncherController::AttachControllerToWindowsIfNeeded() {
   for (auto* window : observed_windows_)
     AttachControllerToWindowIfNeeded(window);
 }
 
+void ArcAppWindowLauncherController::AttachWindowToMultiUserManager(
+    aura::Window* window) {
+  DCHECK(window);
+  chrome::MultiUserWindowManager::GetInstance()->SetWindowOwner(
+      window,
+      user_manager::UserManager::Get()->GetPrimaryUser()->GetAccountId());
+}
+
 void ArcAppWindowLauncherController::AttachControllerToWindowIfNeeded(
     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;
-
-  if (!task_id)
+  const int task_id = GetWindowTaskId(window);
+  if (task_id <= 0)
     return;
 
   // We need to add the observer after exo started observing shell
   // because we want to update the orientation after exo send
   // the layout switch information.
   if (!observing_shell_) {
     observing_shell_ = true;
     ash::WmShell::Get()->AddShellObserver(this);
   }
 
   // Check if we have controller for this task.
   if (GetAppWindowForTask(task_id))
     return;
 
   // Create controller if we have task info.
   TaskIdToShelfAppIdMap::iterator it = task_id_to_shelf_app_id_.find(task_id);
   if (it == task_id_to_shelf_app_id_.end())
     return;
 
   const std::string& app_id = it->second;
 
   views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
   DCHECK(widget);
   std::unique_ptr<AppWindow> app_window(
       new AppWindow(task_id, app_id, widget, this));
   RegisterApp(app_window.get());
   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()) {
     SetOrientationLockForAppWindow(app_window.get());
   }
   task_id_to_app_window_[task_id] = std::move(app_window);
 }
 
 void ArcAppWindowLauncherController::OnAppReadyChanged(
     const std::string& app_id,
@@ skipping 245 matching lines @@
   if (orientation_lock == arc::mojom::OrientationLock::CURRENT) {
     // Resolve the orientation when it first resolved.
     orientation_lock = GetCurrentOrientation();
     app_window->set_requested_orientation_lock(orientation_lock);
   }
 
   ash::Shell* shell = ash::Shell::GetInstance();
   shell->screen_orientation_controller()->LockOrientationForWindow(
       window, BlinkOrientationLockFromMojom(orientation_lock));
 }