chromeos: adds client interface for MultiUserWindow

The client is notified of changes to the window owner.

BUG=756085,875111
TEST=covered by test

Change-Id: I8cec4fe2b547bb1f9f31899e2bc2a9437c186d27
Reviewed-on: https://chromium-review.googlesource.com/c/1343507
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611017}

ash/BUILD.gn

@@ -526,6 +526,7 @@ 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

@@ -10,15 +10,14 @@
 #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/user_switch_animator.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
-#include "ash/ws/window_service_owner.h"
 #include "base/auto_reset.h"
 #include "base/macros.h"
-#include "services/ws/window_service.h"
 #include "ui/aura/mus/window_mus.h"
 #include "ui/aura/mus/window_tree_client.h"
 #include "ui/aura/window.h"
@@ -83,35 +82,6 @@ mojom::WallpaperUserInfoPtr WallpaperUserInfoForAccount(
   return wallpaper_user_info;
 }
 
-// If |window| has a remote client, this converts it to the remote window used
-// by the delegate. This effectively undoes the mapping that
-// MultiUserWindowManagerBridge does.
-// TODO: remove this and instead notify about changes to these windows over a
-// mojom. https://crbug.com/875111.
-aura::Window* MapWindowIfNecessary(aura::Window* window) {
-  if (!ws::WindowService::HasRemoteClient(window) ||
-      !views::MusClient::Exists()) {
-    return window;
-  }
-
-  const ws::Id window_id = Shell::Get()
-                               ->window_service_owner()
-                               ->window_service()
-                               ->GetTopLevelWindowId(window);
-  if (window_id == ws::kInvalidTransportId)
-    return window;
-
-  // children[0] is used to deal with DesktopNativeWidgetAura. In particular,
-  // client code generally expects to see the first child, which corresponds to
-  // Widget::GetNativeWindow(), when using DesktopNativeWidgetAura.
-  aura::WindowMus* window_mus =
-      views::MusClient::Get()->window_tree_client()->GetWindowByServerId(
-          window_id);
-  return window_mus && !window_mus->GetWindow()->children().empty()
-             ? window_mus->GetWindow()->children()[0]
-             : window;
-}
-
 }  // namespace
 
 // A class to temporarily change the animation properties for a window.
@@ -182,9 +152,11 @@ MultiUserWindowManager* MultiUserWindowManager::Get() {
   return g_instance;
 }
 
-void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
-                                            const AccountId& account_id,
-                                            bool show_for_current_user) {
+void MultiUserWindowManager::SetWindowOwner(
+    aura::Window* window,
+    const AccountId& account_id,
+    bool show_for_current_user,
+    MultiUserWindowManagerWindowDelegate* window_delegate) {
   // Make sure the window is valid and there was no owner yet.
   DCHECK(window);
   DCHECK(account_id.is_valid());
@@ -193,7 +165,7 @@ void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
     return;
   DCHECK(GetWindowOwner(window).empty());
   std::unique_ptr<WindowEntry> window_entry_ptr =
-      std::make_unique<WindowEntry>(account_id);
+      std::make_unique<WindowEntry>(account_id, window_delegate);
   WindowEntry* window_entry = window_entry_ptr.get();
   window_to_entry_[window] = std::move(window_entry_ptr);
 
@@ -217,6 +189,14 @@ void MultiUserWindowManager::SetWindowOwner(aura::Window* window,
     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);
@@ -417,9 +397,11 @@ bool MultiUserWindowManager::ShowWindowForUserIntern(
   }
 
   // Notify entry change.
-  if (delegate_) {
-    delegate_->OnOwnerEntryChanged(MapWindowIfNecessary(window), account_id,
-                                   minimized, teleported);
+  if (window_entry->delegate()) {
+    window_entry->delegate()->OnWindowOwnerEntryChanged(window, account_id,
+                                                        minimized, teleported);
+  } else if (delegate_) {
+    delegate_->OnOwnerEntryChanged(window, account_id, minimized, teleported);
   }
   return true;
 }

ash/multi_user/multi_user_window_manager.h

@@ -21,6 +21,7 @@
 namespace ash {
 
 class MultiUserWindowManagerDelegate;
+class MultiUserWindowManagerWindowDelegate;
 class UserSwitchAnimator;
 
 // MultiUserWindowManager associates windows with users and ensures the
@@ -63,10 +64,15 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
   // 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.
+  // account to the current account. If |delegate| is non-null, it is notified
+  // of changes rather than MultiUserWindowManagerDelegate.
   void SetWindowOwner(aura::Window* window,
                       const AccountId& account_id,
-                      bool show_for_current_user);
+                      bool show_for_current_user,
+                      MultiUserWindowManagerWindowDelegate* delegate = nullptr);
+
+  // Removes all references to |delegate|.
+  void RemoveWindowDelegate(MultiUserWindowManagerWindowDelegate* delegate);
 
   // 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
@@ -103,8 +109,9 @@ class ASH_EXPORT MultiUserWindowManager : public SessionObserver,
  protected:
   class WindowEntry {
    public:
-    explicit WindowEntry(const AccountId& account_id)
-        : owner_(account_id), show_for_user_(account_id) {}
+    WindowEntry(const AccountId& account_id,
+                MultiUserWindowManagerWindowDelegate* delegate)
+        : owner_(account_id), show_for_user_(account_id), delegate_(delegate) {}
     ~WindowEntry() {}
 
     // Returns the owner of this window. This cannot be changed.
@@ -125,6 +132,9 @@ 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_; }
+
    private:
     // The user id of the owner of this window.
     const AccountId owner_;
@@ -132,6 +142,8 @@ 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;
 

ash/multi_user/multi_user_window_manager_delegate.h

@@ -20,6 +20,9 @@ class ASH_EXPORT MultiUserWindowManagerDelegate {
   // 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,

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/public/interfaces/multi_user_window_manager.mojom

@@ -9,6 +9,8 @@ import "components/account_id/interfaces/account_id.mojom";
 // Used to assign windows to user accounts so that ash shows the appropriate set
 // of windows based on the active user.
 interface MultiUserWindowManager {
+  SetClient(associated MultiUserWindowManagerClient client);
+
   // Associates a window with an account. If |show_for_current_user| is true,
   // the window is associated with |account_id|, but is shown for the currently
   // active user.
@@ -20,3 +22,13 @@ interface MultiUserWindowManager {
   ShowWindowForUser(uint64 window_id,
                     signin.mojom.AccountId account_id);
 };
+
+interface MultiUserWindowManagerClient {
+  // Called when the owner of a window supplied to SetWindowOwner() changes.
+  // |was_minimized| is true if the window was minimized. |teleported| is true
+  // if the window was not on the desktop of the current user.
+  OnWindowOwnerEntryChanged(uint64 window_id,
+                            signin.mojom.AccountId account_id,
+                            bool was_minimized,
+                            bool teleported);
+};

ash/ws/multi_user_window_manager_bridge.cc

@@ -18,7 +18,16 @@ MultiUserWindowManagerBridge::MultiUserWindowManagerBridge(
                mojo::AssociatedInterfaceRequest<mojom::MultiUserWindowManager>(
                    std::move(handle))) {}
 
-MultiUserWindowManagerBridge::~MultiUserWindowManagerBridge() = default;
+MultiUserWindowManagerBridge::~MultiUserWindowManagerBridge() {
+  // We may get here after MultiUserWindowManager has been destroyed.
+  if (ash::MultiUserWindowManager::Get())
+    ash::MultiUserWindowManager::Get()->RemoveWindowDelegate(this);
+}
+
+void MultiUserWindowManagerBridge::SetClient(
+    mojom::MultiUserWindowManagerClientAssociatedPtrInfo client_info) {
+  client_.Bind(std::move(client_info));
+}
 
 void MultiUserWindowManagerBridge::SetWindowOwner(ws::Id window_id,
                                                   const AccountId& account_id,
@@ -29,8 +38,8 @@ void MultiUserWindowManagerBridge::SetWindowOwner(ws::Id window_id,
   DCHECK(ash::MultiUserWindowManager::Get());
   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);
+    ash::MultiUserWindowManager::Get()->SetWindowOwner(
+        window, account_id, show_for_current_user, this);
   } else {
     DVLOG(1) << "SetWindowOwner passed invalid window, id=" << window_id;
   }
@@ -50,4 +59,16 @@ 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,6 +5,7 @@
 #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"
@@ -21,14 +22,18 @@ class WindowTree;
 namespace ash {
 
 // Trivially forwards calls to MultiUserWindowManager.
-class MultiUserWindowManagerBridge : public mojom::MultiUserWindowManager,
-                                     public ws::WindowManagerInterface {
+class MultiUserWindowManagerBridge
+    : public mojom::MultiUserWindowManager,
+      public ws::WindowManagerInterface,
+      public MultiUserWindowManagerWindowDelegate {
  public:
   MultiUserWindowManagerBridge(ws::WindowTree* window_tree,
                                mojo::ScopedInterfaceEndpointHandle handle);
   ~MultiUserWindowManagerBridge() override;
 
   // mojom::MultiUserWindowManager overrides:
+  void SetClient(mojom::MultiUserWindowManagerClientAssociatedPtrInfo
+                     client_info) override;
   void SetWindowOwner(ws::Id window_id,
                       const AccountId& account_id,
                       bool show_for_current_user) override;
@@ -36,8 +41,15 @@ class MultiUserWindowManagerBridge : public mojom::MultiUserWindowManager,
                          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_;
 
   DISALLOW_COPY_AND_ASSIGN(MultiUserWindowManagerBridge);
 };

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

@@ -142,6 +142,9 @@ MultiUserWindowManagerChromeOS::MultiUserWindowManagerChromeOS(
         views::MusClient::Get()
             ->window_tree_client()
             ->BindWindowManagerInterface<ash::mojom::MultiUserWindowManager>();
+    ash::mojom::MultiUserWindowManagerClientAssociatedPtrInfo ptr_info;
+    client_binding_.Bind(mojo::MakeRequest(&ptr_info));
+    multi_user_window_manager_mojom_->SetClient(std::move(ptr_info));
   }
 }
 
@@ -409,3 +412,24 @@ void MultiUserWindowManagerChromeOS::AddBrowserWindow(Browser* browser) {
   SetWindowOwner(browser->window()->GetNativeWindow(),
                  multi_user_util::GetAccountIdFromProfile(browser->profile()));
 }
+
+void MultiUserWindowManagerChromeOS::OnWindowOwnerEntryChanged(
+    ws::Id window_id,
+    const AccountId& account_id,
+    bool was_minimized,
+    bool teleported) {
+  // This undoes the logic in GetWindowMus(). See it for details.
+  aura::WindowMus* window_mus =
+      views::MusClient::Get()->window_tree_client()->GetWindowByServerId(
+          window_id);
+  if (!window_mus)
+    return;
+
+  views::Widget* widget =
+      views::Widget::GetWidgetForNativeWindow(window_mus->GetWindow());
+  if (!widget)
+    return;
+
+  OnOwnerEntryChanged(widget->GetNativeWindow(), account_id, was_minimized,
+                      teleported);
+}

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

@@ -16,6 +16,8 @@
 #include "components/account_id/account_id.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "mojo/public/cpp/bindings/associated_binding.h"
+#include "services/ws/common/types.h"
 #include "ui/aura/window_observer.h"
 
 class AppObserver;
@@ -45,6 +47,7 @@ class Window;
 class MultiUserWindowManagerChromeOS
     : public MultiUserWindowManager,
       public ash::MultiUserWindowManagerDelegate,
+      public ash::mojom::MultiUserWindowManagerClient,
       public aura::WindowObserver,
       public content::NotificationObserver {
  public:
@@ -122,6 +125,12 @@ class MultiUserWindowManagerChromeOS
     DISALLOW_COPY_AND_ASSIGN(WindowEntry);
   };
 
+  // ash::mojom::MultiUserWindowManagerClient:
+  void OnWindowOwnerEntryChanged(ws::Id window_id,
+                                 const AccountId& account_id,
+                                 bool was_minimized,
+                                 bool teleported) override;
+
   using AccountIdToAppWindowObserver = std::map<AccountId, AppObserver*>;
 
   using WindowToEntryMap =
@@ -159,6 +168,9 @@ class MultiUserWindowManagerChromeOS
   ash::mojom::MultiUserWindowManagerAssociatedPtr
       multi_user_window_manager_mojom_;
 
+  mojo::AssociatedBinding<ash::mojom::MultiUserWindowManagerClient>
+      client_binding_{this};
+
   DISALLOW_COPY_AND_ASSIGN(MultiUserWindowManagerChromeOS);
 };
 

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

@@ -302,6 +302,10 @@ class MultiUserWindowManagerChromeOSTest : public AshTestBase {
     return ash::UserSwitchAnimator::CoversScreen(window);
   }
 
+  void FlushWindowClientBinding() {
+    multi_user_window_manager_->client_binding_.FlushForTesting();
+  }
+
  private:
   chromeos::ScopedStubInstallAttributes test_install_attributes_;
 
@@ -1737,7 +1741,10 @@ TEST_F(MultiUserWindowManagerChromeOSMashTest, SetWindowOwner) {
   multi_user_window_manager()->ShowWindowForUser(widget->GetNativeWindow(),
                                                  account2);
   aura::test::WaitForAllChangesToComplete();
+  FlushWindowClientBinding();
   EXPECT_TRUE(widget->IsVisible());
+  EXPECT_EQ(account2, multi_user_window_manager()->GetUserPresentingWindow(
+                          widget->GetNativeWindow()));
 }
 
 }  // namespace ash

services/ws/window_service.cc

@@ -127,17 +127,6 @@ bool WindowService::IsTopLevelWindow(const aura::Window* window) {
   return server_window && server_window->IsTopLevel();
 }
 
-ws::Id WindowService::GetTopLevelWindowId(aura::Window* window) {
-  ServerWindow* server_window = ServerWindow::GetMayBeNull(window);
-  if (!server_window || !server_window->IsTopLevel())
-    return kInvalidTransportId;
-
-  const ws::WindowTree* owning_tree = server_window->owning_window_tree();
-  return BuildTransportId(
-      owning_tree->client_id(),
-      owning_tree->ClientWindowIdForWindow(window).sink_id());
-}
-
 aura::Window* WindowService::GetWindowByClientId(Id transport_id) {
   const ClientSpecificId client_id = ClientIdFromTransportId(transport_id);
   WindowTree* window_tree = GetTreeById(client_id);

services/ws/window_service.h

@@ -110,10 +110,6 @@ class COMPONENT_EXPORT(WINDOW_SERVICE) WindowService
   // Returns true if |window| hosts a remote client and is a toplevel window.
   static bool IsTopLevelWindow(const aura::Window* window);
 
-  // Returns the transport id for |window|. If |window| is not a top-level
-  // window, returns kInvalidTransportId.
-  ws::Id GetTopLevelWindowId(aura::Window* window);
-
   // Returns the window representing the specified id.
   aura::Window* GetWindowByClientId(Id transport_id);
 

services/ws/window_tree.cc

@@ -317,6 +317,11 @@ bool WindowTree::IsWindowKnown(aura::Window* window) const {
   return window && known_windows_map_.count(window) > 0u;
 }
 
+Id WindowTree::TransportIdForWindow(aura::Window* window) const {
+  DCHECK(IsWindowKnown(window));
+  return ClientWindowIdToTransportId(ClientWindowIdForWindow(window));
+}
+
 ClientWindowId WindowTree::ClientWindowIdForWindow(aura::Window* window) const {
   auto iter = known_windows_map_.find(window);
   return iter == known_windows_map_.end() ? ClientWindowId()
@@ -638,11 +643,6 @@ Id WindowTree::ClientWindowIdToTransportId(
   return (client_id << 32) | client_window_id.sink_id();
 }
 
-Id WindowTree::TransportIdForWindow(aura::Window* window) const {
-  DCHECK(IsWindowKnown(window));
-  return ClientWindowIdToTransportId(ClientWindowIdForWindow(window));
-}
-
 ClientWindowId WindowTree::MakeClientWindowId(Id transport_window_id) const {
   if (!ClientIdFromTransportId(transport_window_id))
     return ClientWindowId(client_id_, transport_window_id);

services/ws/window_tree.h

@@ -149,6 +149,8 @@ class COMPONENT_EXPORT(WINDOW_SERVICE) WindowTree
   // windows exposed to the client are referred to as the known windows.
   bool IsWindowKnown(aura::Window* window) const;
 
+  Id TransportIdForWindow(aura::Window* window) const;
+
   ClientWindowId ClientWindowIdForWindow(aura::Window* window) const;
 
   // If |window| is a client root, the ClientRoot is returned. This does not
@@ -283,8 +285,6 @@ class COMPONENT_EXPORT(WINDOW_SERVICE) WindowTree
   // the client doesn't know its own id).
   Id ClientWindowIdToTransportId(const ClientWindowId& client_window_id) const;
 
-  Id TransportIdForWindow(aura::Window* window) const;
-
   // Returns the ClientWindowId from a transport id. Uses |client_id_| as the
   // ClientWindowId::client_id part if invalid. This function does a straight
   // mapping, there may not be a window with the returned id.