Chrome OS: New window on teleported browser window should show on current desktop

chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc

Index: chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
diff --git a/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc b/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
index 6a3d0007542b56cd11f89550597fdfa29ae8cc1a..3d6acf2150b07d1a9d55f8d8016744442b417aa8 100644
--- a/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
+++ b/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
@@ -58,47 +58,6 @@ const int kUserFadeTimeMS = 110;
 // The animation time in ms for a window which get teleported to another screen.
 const int kTeleportAnimationTimeMS = 300;
 
-// Checks if a given event is a user event.
-bool IsUserEvent(const ui::Event* e) {
-  if (e) {
-    ui::EventType type = e->type();
-    if (type != ui::ET_CANCEL_MODE &&
-        type != ui::ET_UMA_DATA &&
-        type != ui::ET_UNKNOWN)
-      return true;
-  }
-  return false;
-}
-
-// Test if we are currently processing a user event which might lead to a
-// browser / app creation.
-bool IsProcessingUserEvent() {
-  // When there is a nested message loop (e.g. active menu or drag and drop
-  // operation) - we are in a nested loop and can ignore this.
-  // Note: Unit tests might not have a message loop.
-  base::MessageLoop* message_loop = base::MessageLoop::current();
-  if (message_loop && message_loop->is_running() && message_loop->IsNested())
-    return false;
-
-  // TODO(skuhne): "Open link in new window" will come here after the menu got
-  // closed, executing the command from the nested menu loop. However at that
-  // time there is no active event processed. A solution for that need to be
-  // found past M-32. A global event handler filter (pre and post) might fix
-  // that problem in conjunction with a depth counter - but - for the menu
-  // execution we come here after the loop was finished (so it's not nested
-  // anymore) and the root window should therefore still have the event which
-  // lead to the menu invocation, but it is not. By fixing that problem this
-  // would "magically work".
-  aura::Window::Windows root_window_list = ash::Shell::GetAllRootWindows();
-  for (aura::Window::Windows::iterator it = root_window_list.begin();
-       it != root_window_list.end();
-       ++it) {
-    if (IsUserEvent((*it)->GetHost()->dispatcher()->current_event()))
-      return true;
-  }
-  return false;
-}
-
 // Records the type of window which was transferred to another desktop.
 void RecordUMAForTransferredWindowType(aura::Window* window) {
   // We need to figure out what kind of window this is to record the transfer.
@@ -171,24 +130,18 @@ class AnimationSetter {
  public:
   AnimationSetter(aura::Window* window, int animation_time_in_ms)
       : window_(window),
-        previous_animation_type_(
-            wm::GetWindowVisibilityAnimationType(window_)),
+        previous_animation_type_(wm::GetWindowVisibilityAnimationType(window_)),
         previous_animation_time_(
             wm::GetWindowVisibilityAnimationDuration(*window_)) {
     wm::SetWindowVisibilityAnimationType(
-        window_,
-        wm::WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
+        window_, wm::WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
     wm::SetWindowVisibilityAnimationDuration(
-        window_,
-        base::TimeDelta::FromMilliseconds(animation_time_in_ms));
+        window_, base::TimeDelta::FromMilliseconds(animation_time_in_ms));
   }
 
   ~AnimationSetter() {
-    wm::SetWindowVisibilityAnimationType(window_,
-                                                    previous_animation_type_);

[Qiang(Joe) Xu, 2017/02/08 00:57:36]

This trunk is just formatting.

-    wm::SetWindowVisibilityAnimationDuration(
-        window_,
-        previous_animation_time_);
+    wm::SetWindowVisibilityAnimationType(window_, previous_animation_type_);
+    wm::SetWindowVisibilityAnimationDuration(window_, previous_animation_time_);
   }
 
  private:
@@ -317,11 +270,6 @@ void MultiUserWindowManagerChromeOS::SetWindowOwner(
   window->AddObserver(this);
   wm::TransientWindowManager::Get(window)->AddObserver(this);
 
-  // Check if this window was created due to a user interaction. If it was,
-  // transfer it to the current user.
-  if (IsProcessingUserEvent())
-    window_to_entry_[window]->set_show_for_user(current_account_id_);

[Qiang(Joe) Xu, 2017/02/08 00:57:36]

we don't need this.. checked https://codereview.chromium.org/55303003, the bug
doesn't happen any more.

-
   // Add all transient children to our set of windows. Note that the function
   // will add the children but not the owner to the transient children map.
   AddTransientOwnerRecursive(window, window);
@@ -644,8 +592,10 @@ void MultiUserWindowManagerChromeOS::AddBrowserWindow(Browser* browser) {
   // come here with no valid window.
   if (!browser->window() || !browser->window()->GetNativeWindow())
     return;
-  SetWindowOwner(browser->window()->GetNativeWindow(),
+  aura::Window* native_window = browser->window()->GetNativeWindow();
+  SetWindowOwner(native_window,
                  multi_user_util::GetAccountIdFromProfile(browser->profile()));
+  window_to_entry_[native_window]->set_show_for_user(current_account_id_);

[oshima, 2017/02/08 01:29:31]

Won't this always open the window in the current active? That may not be the
right desktop if the window is opened after the desktop has switched for
example. 

I think you need to tell MUWM on which account's desktop it should open in
ChromeNewWindowClient.

 }
 
 void MultiUserWindowManagerChromeOS::ShowWithTransientChildrenRecursive(