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

 // Copyright 2013 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/multi_user/multi_user_window_manager_chromeos.h"
 
 #include "ash/common/media_controller.h"
 #include "ash/common/multi_profile_uma.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/wm/maximize_mode/maximize_mode_controller.h"
@@ skipping 40 matching lines @@
 // in / out.
 const int kAnimationTimeMS = 100;
 
 // The animation time in milliseconds for the fade in and / or out when
 // switching users.
 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.
   Browser* browser = chrome::FindBrowserWithWindow(window);
   ash::MultiProfileUMA::TeleportWindowType window_type =
       ash::MultiProfileUMA::TELEPORT_WINDOW_UNKNOWN;
   if (browser) {
     if (browser->profile()->IsOffTheRecord()) {
       window_type = ash::MultiProfileUMA::TELEPORT_WINDOW_INCOGNITO_BROWSER;
     } else if (browser->is_app()) {
@@ skipping 52 matching lines @@
 
 }  // namespace
 
 namespace chrome {
 
 // A class to temporarily change the animation properties for a window.
 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:
   // The window which gets used.
   aura::Window* window_;
 
   // Previous animation type.
   const int previous_animation_type_;
 
   // Previous animation time.
@@ skipping 108 matching lines @@
   DCHECK(GetWindowOwner(window).empty());
   window_to_entry_[window] = new WindowEntry(account_id);
 
   // Remember the initial visibility of the window.
   window_to_entry_[window]->set_show(window->IsVisible());
 
   // Add observers to track state changes.
   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);
 
   // Notify entry adding.
   for (Observer& observer : observers_)
     observer.OnOwnerEntryAdded(window);
 
   if (!IsWindowOnDesktopOfUser(window, current_account_id_))
     SetWindowVisibility(window, false, 0);
@@ skipping 302 matching lines @@
 void MultiUserWindowManagerChromeOS::NotifyAfterUserSwitchAnimationFinished() {
   for (Observer& observer : observers_)
     observer.OnUserSwitchAnimationFinished();
 }
 
 void MultiUserWindowManagerChromeOS::AddBrowserWindow(Browser* browser) {
   // A unit test (e.g. CrashRestoreComplexTest.RestoreSessionForThreeUsers) can
   // 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(
     aura::Window* window, int animation_time_in_ms) {
   aura::Window::Windows::const_iterator it =
       wm::GetTransientChildren(window).begin();
   for (; it != wm::GetTransientChildren(window).end(); ++it)
     ShowWithTransientChildrenRecursive(*it, animation_time_in_ms);
 
   // We show all children which were not explicitly hidden.
@@ skipping 97 matching lines @@
     window->Hide();
 }
 
 int MultiUserWindowManagerChromeOS::GetAdjustedAnimationTimeInMS(
     int default_time_in_ms) const {
   return animation_speed_ == ANIMATION_SPEED_NORMAL ? default_time_in_ms :
       (animation_speed_ == ANIMATION_SPEED_FAST ? 10 : 0);
 }
 
 }  // namespace chrome