chromeos: further MultiUserWindowManager refactoring

This moves the majority of the MultiUserWindowManagerDelegate functions
into the mojom. For classic mode, a mojo::Binding is created so that notification
happens async (just as it will in multi-process mdoe).

MultiUserWindowManagerWindowDelegate has been removed.

MultiUserWindowManagerDelegate has been removed.

Adds MultiUserWindowManagerDelegateClassic. Here's the comment for it:
// Delegate used in classic and single-process mash mode. This delegate is
// notified when the state of a window changes. In classic mode, this delegate
// is called for all windows, in single-process mash mode, this delegate is
// called for Windows created in ash's window hierarchy (more specifically,
// Windows whose aura::Env has a mode of aura::Mode::LOCAL). For example, Arc
// creates Windows that are parented directly to Ash's window hierarchy, changes
// to such windows are called through this delegate.

Ideally MultiUserWindowManagerDelegateClassic wouldn't be necessary, but that
would require assigning unique ids for local windows, which would be rather
error prone.

The biggest change here is that MultiUserWindowManagerChromeOS is now notified
async of the functions that were in MultiUserWindowManagerDelegate, even in
classic.


BUG=875111
TEST=covered by tests

Change-Id: I51e8a89ae3c4e6096d0cf04dc91133781b8cb4cf
Reviewed-on: https://chromium-review.googlesource.com/c/1354558
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612428}

ash/BUILD.gn

@@ -52,7 +52,7 @@ component("ash") {
     # TODO: move MultiUserWindowManager (and delegate) to sources:
     # https://crbug.com/756085
     "multi_user/multi_user_window_manager.h",
-    "multi_user/multi_user_window_manager_delegate.h",
+    "multi_user/multi_user_window_manager_delegate_classic.h",
     "new_window_controller.h",
     "root_window_controller.h",
     "screenshot_delegate.h",
@@ -526,7 +526,6 @@ component("ash") {
     "multi_profile_uma.cc",
     "multi_profile_uma.h",
     "multi_user/multi_user_window_manager.cc",
-    "multi_user/multi_user_window_manager_window_delegate.h",
     "multi_user/user_switch_animator.cc",
     "multi_user/user_switch_animator.h",
     "network_connect_delegate_mus.cc",

ash/multi_user/multi_user_window_manager.cc

@@ -9,8 +9,7 @@
 
 #include "ash/media_controller.h"
 #include "ash/multi_user/multi_user_window_manager.h"
-#include "ash/multi_user/multi_user_window_manager_delegate.h"
-#include "ash/multi_user/multi_user_window_manager_window_delegate.h"
+#include "ash/multi_user/multi_user_window_manager_delegate_classic.h"
 #include "ash/multi_user/user_switch_animator.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/session/session_controller.h"
@@ -117,10 +116,23 @@ class AnimationSetter {
   DISALLOW_COPY_AND_ASSIGN(AnimationSetter);
 };
 
+MultiUserWindowManager::WindowEntry::WindowEntry(
+    const AccountId& account_id,
+    base::Optional<ws::Id> window_id)
+    : owner_(account_id),
+      show_for_user_(account_id),
+      window_id_(std::move(window_id)),
+      from_window_service_(window_id.has_value()) {}
+
+MultiUserWindowManager::WindowEntry::~WindowEntry() = default;
+
 MultiUserWindowManager::MultiUserWindowManager(
-    MultiUserWindowManagerDelegate* delegate,
+    mojom::MultiUserWindowManagerClient* client,
+    MultiUserWindowManagerDelegateClassic* classic_delegate,
     const AccountId& account_id)
-    : delegate_(delegate), current_account_id_(account_id) {
+    : client_(client),
+      classic_delegate_(classic_delegate),
+      current_account_id_(account_id) {
   g_instance = this;
   Shell::Get()->tablet_mode_controller()->AddObserver(this);
   Shell::Get()->session_controller()->AddObserver(this);
@@ -152,11 +164,20 @@ MultiUserWindowManager* MultiUserWindowManager::Get() {
   return g_instance;
 }
 
-void MultiUserWindowManager::SetWindowOwner(
-    aura::Window* window,
-    const AccountId& account_id,
-    bool show_for_current_user,
-    MultiUserWindowManagerWindowDelegate* window_delegate) {
+void MultiUserWindowManager::SetClient(
+    mojom::MultiUserWindowManagerClient* client) {
+  client_ = client;
+
+  // Window ids are unique to a particular client. If the client changes, drop
+  // any existing ids.
+  for (auto& pair : window_to_entry_)
+    pair.second->reset_window_id();
+}
+
+void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
+                                            const AccountId& account_id,
+                                            bool show_for_current_user,
+                                            base::Optional<ws::Id> window_id) {
   // Make sure the window is valid and there was no owner yet.
   DCHECK(window);
   DCHECK(account_id.is_valid());
@@ -165,7 +186,7 @@ void MultiUserWindowManager::SetWindowOwner(
     return;
   DCHECK(GetWindowOwner(window).empty());
   std::unique_ptr<WindowEntry> window_entry_ptr =
-      std::make_unique<WindowEntry>(account_id, window_delegate);
+      std::make_unique<WindowEntry>(account_id, std::move(window_id));
   WindowEntry* window_entry = window_entry_ptr.get();
   window_to_entry_[window] = std::move(window_entry_ptr);
 
@@ -189,14 +210,6 @@ void MultiUserWindowManager::SetWindowOwner(
     SetWindowVisibility(window, false);
 }
 
-void MultiUserWindowManager::RemoveWindowDelegate(
-    MultiUserWindowManagerWindowDelegate* delegate) {
-  for (auto& pair : window_to_entry_) {
-    if (pair.second->delegate() == delegate)
-      pair.second->clear_delegate();
-  }
-}
-
 const AccountId& MultiUserWindowManager::GetWindowOwner(
     aura::Window* window) const {
   WindowToEntryMap::const_iterator it = window_to_entry_.find(window);
@@ -258,8 +271,8 @@ void MultiUserWindowManager::OnActiveUserSessionChanged(
   // This needs to be set before the animation starts.
   current_account_id_ = account_id;
 
-  if (delegate_)
-    delegate_->OnWillSwitchActiveAccount(current_account_id_);
+  if (client_)
+    client_->OnWillSwitchActiveAccount(current_account_id_);
 
   // Here to avoid a very nasty race condition, we must destruct any previously
   // created animation before creating a new one. Otherwise, the newly
@@ -400,11 +413,12 @@ bool MultiUserWindowManager::ShowWindowForUserIntern(
   }
 
   // Notify entry change.
-  if (window_entry->delegate()) {
-    window_entry->delegate()->OnWindowOwnerEntryChanged(window, account_id,
-                                                        minimized, teleported);
-  } else if (delegate_) {
-    delegate_->OnOwnerEntryChanged(window, account_id, minimized, teleported);
+  if (!window_entry->from_window_service()) {
+    classic_delegate_->OnOwnerEntryChanged(window, account_id, minimized,
+                                           teleported);
+  } else if (client_ && window_entry->window_id().has_value()) {
+    client_->OnWindowOwnerEntryChanged(*window_entry->window_id(), account_id,
+                                       minimized, teleported);
   }
   return true;
 }

ash/multi_user/multi_user_window_manager.h

@@ -9,19 +9,21 @@
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/public/interfaces/multi_user_window_manager.mojom.h"
 #include "ash/session/session_observer.h"
 #include "ash/wm/tablet_mode/tablet_mode_observer.h"
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "components/account_id/account_id.h"
+#include "services/ws/common/types.h"
 #include "ui/aura/window_observer.h"
 #include "ui/wm/core/transient_window_observer.h"
 
 namespace ash {
 
-class MultiUserWindowManagerDelegate;
-class MultiUserWindowManagerWindowDelegate;
+class MultiUserWindowManagerDelegateClassic;
 class UserSwitchAnimator;
 
 // MultiUserWindowManager associates windows with users and ensures the
@@ -35,6 +37,12 @@ class UserSwitchAnimator;
 // account are the same, but the user may choose to show a window from an other
 // account, in which case the 'shown' account changes.
 //
+// MultiUserWindowManager makes use of the following client/delegate interfaces
+// mojom::MultiUserWindowManagerClient: used for windows created by the window
+// service, as well as major state changes (such as animation changing).
+// MultiUserWindowManagerDelegateClassic: used for all other windows. See
+// MultiUserWindowManagerDelegateClassic for details on what this means.
+//
 // Note:
 // - aura::Window::Hide() is currently hiding the window and all owned transient
 //   children. However aura::Window::Show() is only showing the window itself.
@@ -55,24 +63,32 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
     ANIMATION_SPEED_DISABLED  // Unit tests which do not require animations.
   };
 
-  MultiUserWindowManager(MultiUserWindowManagerDelegate* delegate,
-                         const AccountId& account_id);
+  MultiUserWindowManager(
+      mojom::MultiUserWindowManagerClient* client,
+      MultiUserWindowManagerDelegateClassic* classic_delegate,
+      const AccountId& account_id);
   ~MultiUserWindowManager() override;
 
   static MultiUserWindowManager* Get();
 
+  // Resets the client. This is called when running in mash. In single-process
+  // mash, the browser creates this class (with no client) and
+  // MultiUserWindowManagerBridge sets the client (as the client is provided
+  // over mojom). In multi-process mash, MultiUserWindowManagerBridge creates
+  // this and sets the client. This function is only necessary until
+  // multi-process mash is the default.
+  void SetClient(mojom::MultiUserWindowManagerClient* client);
+
   // Associates a window with a particular account. This may result in hiding
   // |window|. This should *not* be called more than once with a different
   // account. If |show_for_current_user| is true, this sets the 'shown'
-  // account to the current account. If |delegate| is non-null, it is notified
-  // of changes rather than MultiUserWindowManagerDelegate.
+  // account to the current account. If |window_id| is valid, changes to
+  // |window| are notified through MultiUserWindowManagerClient. If |window_id|
+  // is empty, MultiUserWindowManagerDelegateClassic is used.
   void SetWindowOwner(aura::Window* window,
                       const AccountId& account_id,
                       bool show_for_current_user,
-                      MultiUserWindowManagerWindowDelegate* delegate = nullptr);
-
-  // Removes all references to |delegate|.
-  void RemoveWindowDelegate(MultiUserWindowManagerWindowDelegate* delegate);
+                      base::Optional<ws::Id> window_id = base::nullopt);
 
   // Sets the 'shown' account for a window. See class description for details on
   // what the 'shown' account is. This function may trigger changing the active
@@ -106,13 +122,14 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
   // Returns the current user for unit tests.
   const AccountId& GetCurrentUserForTest() const;
 
- protected:
+ private:
+  friend class MultiUserWindowManagerChromeOSTest;
+  friend class UserSwitchAnimator;
+
   class WindowEntry {
    public:
-    WindowEntry(const AccountId& account_id,
-                MultiUserWindowManagerWindowDelegate* delegate)
-        : owner_(account_id), show_for_user_(account_id), delegate_(delegate) {}
-    ~WindowEntry() {}
+    WindowEntry(const AccountId& account_id, base::Optional<ws::Id> window_id);
+    ~WindowEntry();
 
     // Returns the owner of this window. This cannot be changed.
     const AccountId& owner() const { return owner_; }
@@ -132,8 +149,15 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
     // Sets if the window gets shown for the active user or not.
     void set_show(bool show) { show_ = show; }
 
-    void clear_delegate() { delegate_ = nullptr; }
-    MultiUserWindowManagerWindowDelegate* delegate() { return delegate_; }
+    // True if this window was created by the window service.
+    bool from_window_service() const { return from_window_service_; }
+
+    // Unsets the |window_id|. This does not effect whether the window is
+    // from the window-service, only the stored id. Resetting the id happens
+    // when the client changes. This is necessary as the id is generally unique
+    // to a client.
+    void reset_window_id() { window_id_.reset(); }
+    const base::Optional<ws::Id> window_id() const { return window_id_; }
 
    private:
     // The user id of the owner of this window.
@@ -142,14 +166,19 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
     // The user id of the user on which desktop the window gets shown.
     AccountId show_for_user_;
 
-    MultiUserWindowManagerWindowDelegate* delegate_;
-
     // True if the window should be visible for the user which shows the window.
     bool show_ = true;
 
+    // The id assigned to the window by the WindowService.
+    base::Optional<ws::Id> window_id_;
+
+    const bool from_window_service_;
+
     DISALLOW_COPY_AND_ASSIGN(WindowEntry);
   };
 
+  using TransientWindowToVisibility = base::flat_map<aura::Window*, bool>;
+
   using WindowToEntryMap =
       std::map<aura::Window*, std::unique_ptr<WindowEntry>>;
 
@@ -185,12 +214,6 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
 
   const WindowToEntryMap& window_to_entry() { return window_to_entry_; }
 
- private:
-  friend class MultiUserWindowManagerChromeOSTest;
-  friend class UserSwitchAnimator;
-
-  using TransientWindowToVisibility = base::flat_map<aura::Window*, bool>;
-
   // Show the window and its transient children. However - if a transient child
   // was turned invisible by some other operation, it will stay invisible.
   // |animation_time| is the amount of time to animate.
@@ -219,7 +242,9 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
   // Returns the time for an animation.
   base::TimeDelta GetAdjustedAnimationTime(base::TimeDelta default_time) const;
 
-  MultiUserWindowManagerDelegate* delegate_;
+  mojom::MultiUserWindowManagerClient* client_;
+
+  MultiUserWindowManagerDelegateClassic* classic_delegate_;
 
   // A lookup to see to which user the given window belongs to, where and if it
   // should get shown.

ash/multi_user/multi_user_window_manager_delegate.h → ash/multi_user/multi_user_window_manager_delegate_classic.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_H_
-#define ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_H_
+#ifndef ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_CLASSIC_H_
+#define ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_CLASSIC_H_
 
 #include "ash/ash_export.h"
 
@@ -15,34 +15,27 @@ class Window;
 
 namespace ash {
 
-class ASH_EXPORT MultiUserWindowManagerDelegate {
+// Delegate used in classic and single-process mash mode. This delegate is
+// notified when the state of a window changes. In classic mode, this delegate
+// is called for all windows, in single-process mash mode, this delegate is
+// called for Windows created in ash's window hierarchy (more specifically,
+// Windows whose aura::Env has a mode of aura::Mode::LOCAL). For example, Arc
+// creates Windows that are parented directly to Ash's window hierarchy, changes
+// to such windows are called through this delegate.
+class ASH_EXPORT MultiUserWindowManagerDelegateClassic {
  public:
   // Called when the owner of the window tracked by the manager is changed.
   // |was_minimized| is true if the window was minimized. |teleported| is true
   // if the window was not on the desktop of the current user.
-  // NOTE: when running this is not called for windows that are created through
-  // the WindowService. Windows created by the WindowService supply a
-  // MultiUserWindowManagerWindowDelegate.
   virtual void OnOwnerEntryChanged(aura::Window* window,
                                    const AccountId& account_id,
                                    bool was_minimized,
                                    bool teleported) {}
 
-  // Called when the active account changes. This is followed by
-  // OnTransitionUserShelfToNewAccount() and OnDidSwitchActiveAccount().
-  virtual void OnWillSwitchActiveAccount(const AccountId& account_id) {}
-
-  // Called at the time when the user's shelf should transition to the account
-  // supplied to OnWillSwitchActiveAccount().
-  virtual void OnTransitionUserShelfToNewAccount() {}
-
-  // Called when the active account change is complete.
-  virtual void OnDidSwitchActiveAccount() {}
-
  protected:
-  virtual ~MultiUserWindowManagerDelegate() {}
+  virtual ~MultiUserWindowManagerDelegateClassic() {}
 };
 
 }  // namespace ash
 
-#endif  // ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_H_
+#endif  // ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_DELEGATE_CLASSIC_H_

ash/multi_user/multi_user_window_manager_window_delegate.h

-// Copyright 2018 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.
-
-#ifndef ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_WINDOW_DELEGATE_H_
-#define ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_WINDOW_DELEGATE_H_
-
-#include "ash/ash_export.h"
-
-class AccountId;
-
-namespace aura {
-class Window;
-}  // namespace aura
-
-namespace ash {
-
-// Delegate associated with a single Window. This is used by windows created
-// by the WindowService.
-class ASH_EXPORT MultiUserWindowManagerWindowDelegate {
- public:
-  // Called when the owner of the window tracked by the manager is changed.
-  // |was_minimized| is true if the window was minimized. |teleported| is true
-  // if the window was not on the desktop of the current user.
-  virtual void OnWindowOwnerEntryChanged(aura::Window* window,
-                                         const AccountId& account_id,
-                                         bool was_minimized,
-                                         bool teleported) {}
-
- protected:
-  virtual ~MultiUserWindowManagerWindowDelegate() {}
-};
-
-}  // namespace ash
-
-#endif  // ASH_MULTI_USER_MULTI_USER_WINDOW_MANAGER_WINDOW_DELEGATE_H_

ash/multi_user/user_switch_animator.cc

@@ -5,7 +5,6 @@
 #include "ash/multi_user/user_switch_animator.h"
 
 #include "ash/multi_user/multi_user_window_manager.h"
-#include "ash/multi_user/multi_user_window_manager_delegate.h"
 #include "ash/shell.h"
 #include "ash/wallpaper/wallpaper_controller.h"
 #include "ash/wm/mru_window_tracker.h"
@@ -140,8 +139,8 @@ void UserSwitchAnimator::AdvanceUserTransitionAnimation() {
     case ANIMATION_STEP_FINALIZE:
       user_changed_animation_timer_.reset();
       animation_step_ = ANIMATION_STEP_ENDED;
-      if (owner_->delegate_)
-        owner_->delegate_->OnDidSwitchActiveAccount();
+      if (owner_->client_)
+        owner_->client_->OnDidSwitchActiveAccount();
       break;
     case ANIMATION_STEP_ENDED:
       NOTREACHED();
@@ -197,8 +196,8 @@ void UserSwitchAnimator::TransitionUserShelf(AnimationStep animation_step) {
   if (animation_step != ANIMATION_STEP_SHOW_NEW_USER)
     return;
 
-  if (owner_->delegate_)
-    owner_->delegate_->OnTransitionUserShelfToNewAccount();
+  if (owner_->client_)
+    owner_->client_->OnTransitionUserShelfToNewAccount();
 }
 
 void UserSwitchAnimator::TransitionWindows(AnimationStep animation_step) {

ash/public/interfaces/multi_user_window_manager.mojom

@@ -31,4 +31,15 @@ interface MultiUserWindowManagerClient {
                             signin.mojom.AccountId account_id,
                             bool was_minimized,
                             bool teleported);
+
+  // Called when the active account changes. This is followed by
+  // OnTransitionUserShelfToNewAccount() and OnDidSwitchActiveAccount().
+  OnWillSwitchActiveAccount(signin.mojom.AccountId account_id);
+
+  // Called at the time when the user's shelf should transition to the account
+  // supplied to OnWillSwitchActiveAccount().
+  OnTransitionUserShelfToNewAccount();
+
+  // Called when the active account change is complete.
+  OnDidSwitchActiveAccount();
 };

ash/ws/multi_user_window_manager_bridge.cc

@@ -21,12 +21,13 @@ MultiUserWindowManagerBridge::MultiUserWindowManagerBridge(
 MultiUserWindowManagerBridge::~MultiUserWindowManagerBridge() {
   // We may get here after MultiUserWindowManager has been destroyed.
   if (ash::MultiUserWindowManager::Get())
-    ash::MultiUserWindowManager::Get()->RemoveWindowDelegate(this);
+    ash::MultiUserWindowManager::Get()->SetClient(nullptr);
 }
 
 void MultiUserWindowManagerBridge::SetClient(
     mojom::MultiUserWindowManagerClientAssociatedPtrInfo client_info) {
   client_.Bind(std::move(client_info));
+  ash::MultiUserWindowManager::Get()->SetClient(client_.get());
 }
 
 void MultiUserWindowManagerBridge::SetWindowOwner(ws::Id window_id,
@@ -39,7 +40,7 @@ void MultiUserWindowManagerBridge::SetWindowOwner(ws::Id window_id,
   aura::Window* window = window_tree_->GetWindowByTransportId(window_id);
   if (window && window_tree_->IsTopLevel(window)) {
     ash::MultiUserWindowManager::Get()->SetWindowOwner(
-        window, account_id, show_for_current_user, this);
+        window, account_id, show_for_current_user, {window_id});
   } else {
     DVLOG(1) << "SetWindowOwner passed invalid window, id=" << window_id;
   }
@@ -59,16 +60,4 @@ void MultiUserWindowManagerBridge::ShowWindowForUser(
     DVLOG(1) << "ShowWindowForUser passed invalid window, id=" << window_id;
 }
 
-void MultiUserWindowManagerBridge::OnWindowOwnerEntryChanged(
-    aura::Window* window,
-    const AccountId& account_id,
-    bool was_minimized,
-    bool teleported) {
-  if (!client_)
-    return;
-
-  client_->OnWindowOwnerEntryChanged(window_tree_->TransportIdForWindow(window),
-                                     account_id, was_minimized, teleported);
-}
-
 }  // namespace ash

ash/ws/multi_user_window_manager_bridge.h

@@ -5,7 +5,6 @@
 #ifndef ASH_WS_MULTI_USER_WINDOW_MANAGER_BRIDGE_H_
 #define ASH_WS_MULTI_USER_WINDOW_MANAGER_BRIDGE_H_
 
-#include "ash/multi_user/multi_user_window_manager_window_delegate.h"
 #include "ash/public/interfaces/multi_user_window_manager.mojom.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "services/ws/common/types.h"
@@ -22,10 +21,8 @@ class WindowTree;
 namespace ash {
 
 // Trivially forwards calls to MultiUserWindowManager.
-class MultiUserWindowManagerBridge
-    : public mojom::MultiUserWindowManager,
-      public ws::WindowManagerInterface,
-      public MultiUserWindowManagerWindowDelegate {
+class MultiUserWindowManagerBridge : public mojom::MultiUserWindowManager,
+                                     public ws::WindowManagerInterface {
  public:
   MultiUserWindowManagerBridge(ws::WindowTree* window_tree,
                                mojo::ScopedInterfaceEndpointHandle handle);
@@ -41,12 +38,6 @@ class MultiUserWindowManagerBridge
                          const AccountId& account_id) override;
 
  private:
-  // MultiUserWindowManagerWindowDelegate:
-  void OnWindowOwnerEntryChanged(aura::Window* window,
-                                 const AccountId& account_id,
-                                 bool was_minimized,
-                                 bool teleported) override;
-
   ws::WindowTree* window_tree_;
   mojo::AssociatedBinding<mojom::MultiUserWindowManager> binding_;
   mojom::MultiUserWindowManagerClientAssociatedPtr client_;

chrome/browser/ui/ash/multi_user/DEPS

@@ -2,7 +2,7 @@ specific_include_rules = {
   # https://crbug.com/756085
   "multi_user_window_manager_chromeos\.*": [
     "+ash/multi_user/multi_user_window_manager.h",
-    "+ash/multi_user/multi_user_window_manager_delegate.h",
+    "+ash/multi_user/multi_user_window_manager_delegate_classic.h",
   ],
   # https://crbug.com/875111
   "multi_user_window_manager_chromeos_unittest\.cc": [

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

@@ -25,6 +25,7 @@
 #include "content/public/browser/notification_service.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
 #include "ui/aura/mus/window_mus.h"
@@ -131,13 +132,24 @@ class AppObserver : public extensions::AppWindowRegistry::Observer {
   DISALLOW_COPY_AND_ASSIGN(AppObserver);
 };
 
+// Used only in classic mode. In classic mode a mojo Binding is used that
+// results in the delegate being notified async. Doing this gives the same
+// async delay seen when the WindowService is used.
+struct MultiUserWindowManagerChromeOS::ClassicSupport {
+  explicit ClassicSupport(MultiUserWindowManagerChromeOS* host)
+      : binding(host) {}
+
+  ash::mojom::MultiUserWindowManagerClientPtr client_ptr;
+  mojo::Binding<ash::mojom::MultiUserWindowManagerClient> binding;
+};
+
 MultiUserWindowManagerChromeOS::MultiUserWindowManagerChromeOS(
     const AccountId& current_account_id)
-    : current_account_id_(current_account_id),
-      ash_multi_user_window_manager_(
-          std::make_unique<ash::MultiUserWindowManager>(this,
-                                                        current_account_id)) {
+    : current_account_id_(current_account_id) {
+  ash::mojom::MultiUserWindowManagerClient* client = nullptr;
   if (features::IsUsingWindowService()) {
+    // This path doesn't set |client| as it'll be wired up in ash when it
+    // sees the MultiUserWindowManagerClient registration.
     multi_user_window_manager_mojom_ =
         views::MusClient::Get()
             ->window_tree_client()
@@ -145,6 +157,16 @@ MultiUserWindowManagerChromeOS::MultiUserWindowManagerChromeOS(
     ash::mojom::MultiUserWindowManagerClientAssociatedPtrInfo ptr_info;
     client_binding_.Bind(mojo::MakeRequest(&ptr_info));
     multi_user_window_manager_mojom_->SetClient(std::move(ptr_info));
+  } else {
+    classic_support_ = std::make_unique<ClassicSupport>(this);
+    classic_support_->binding.Bind(
+        mojo::MakeRequest(&classic_support_->client_ptr));
+    client = classic_support_->client_ptr.get();
+  }
+  if (!features::IsMultiProcessMash()) {
+    ash_multi_user_window_manager_ =
+        std::make_unique<ash::MultiUserWindowManager>(client, this,
+                                                      current_account_id);
   }
 }
 
@@ -225,6 +247,9 @@ void MultiUserWindowManagerChromeOS::SetWindowOwner(
           window_mus->server_id(), account_id, show_for_current_user);
     }  // else case can happen during shutdown, or for child windows.
   } else {
+    // If there is a non-MUS window, then we must have created
+    // |ash_multi_user_window_manager_|.
+    DCHECK(ash_multi_user_window_manager_);
     ash_multi_user_window_manager_->SetWindowOwner(window, account_id,
                                                    show_for_current_user);
   }
@@ -433,3 +458,8 @@ void MultiUserWindowManagerChromeOS::OnWindowOwnerEntryChanged(
   OnOwnerEntryChanged(widget->GetNativeWindow(), account_id, was_minimized,
                       teleported);
 }
+
+void MultiUserWindowManagerChromeOS::FlushForTesting() {
+  DCHECK(!features::IsUsingWindowService());
+  classic_support_->binding.FlushForTesting();
+}

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

@@ -8,7 +8,7 @@
 #include <map>
 #include <memory>
 
-#include "ash/multi_user/multi_user_window_manager_delegate.h"
+#include "ash/multi_user/multi_user_window_manager_delegate_classic.h"
 #include "ash/public/interfaces/multi_user_window_manager.mojom.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
@@ -46,7 +46,7 @@ class Window;
 // a mojom. https://crbug.com/875111.
 class MultiUserWindowManagerChromeOS
     : public MultiUserWindowManager,
-      public ash::MultiUserWindowManagerDelegate,
+      public ash::MultiUserWindowManagerDelegateClassic,
       public ash::mojom::MultiUserWindowManagerClient,
       public aura::WindowObserver,
       public content::NotificationObserver {
@@ -82,14 +82,11 @@ class MultiUserWindowManagerChromeOS
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // ash::MultiUserWindowManagerDelegate overrides:
+  // ash::MultiUserWindowManagerDelegateClassic overrides:
   void OnOwnerEntryChanged(aura::Window* window,
                            const AccountId& account_id,
                            bool was_minimized,
                            bool teleported) override;
-  void OnWillSwitchActiveAccount(const AccountId& account_id) override;
-  void OnTransitionUserShelfToNewAccount() override;
-  void OnDidSwitchActiveAccount() override;
 
   // Returns the current user for unit tests.
   const AccountId& GetCurrentUserForTest() const;
@@ -97,6 +94,8 @@ class MultiUserWindowManagerChromeOS
  private:
   friend class ash::MultiUserWindowManagerChromeOSTest;
 
+  struct ClassicSupport;
+
   class WindowEntry {
    public:
     explicit WindowEntry(const AccountId& account_id)
@@ -130,6 +129,11 @@ class MultiUserWindowManagerChromeOS
                                  const AccountId& account_id,
                                  bool was_minimized,
                                  bool teleported) override;
+  void OnWillSwitchActiveAccount(const AccountId& account_id) override;
+  void OnTransitionUserShelfToNewAccount() override;
+  void OnDidSwitchActiveAccount() override;
+
+  void FlushForTesting();
 
   using AccountIdToAppWindowObserver = std::map<AccountId, AppObserver*>;
 
@@ -139,6 +143,8 @@ class MultiUserWindowManagerChromeOS
   // Add a browser window to the system so that the owner can be remembered.
   void AddBrowserWindow(Browser* browser);
 
+  std::unique_ptr<ClassicSupport> classic_support_;
+
   // A lookup to see to which user the given window belongs to, where and if it
   // should get shown.
   WindowToEntryMap window_to_entry_;

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

@@ -185,6 +185,8 @@ class MultiUserWindowManagerChromeOSTest : public AshTestBase {
     fake_user_manager_->SwitchActiveUser(id);
     ash::MultiUserWindowManager::Get()->OnActiveUserSessionChanged(id);
     FlushWindowTreeClientMessages();
+    if (!features::IsUsingWindowService())
+      multi_user_window_manager_->FlushForTesting();
   }
 
   // Set up the test environment for this many windows.