chromeos: shelf activation changes for mash

This changes how ExtensionAppWindowLauncherController watches for activation.
ExtensionAppWindowLauncherController inherits activation watching from
wm::ActivationChangeObserver. The ActivationChangeObserver is added to
Shell's root window. This doesn't work for mash.

This switches ExtensionAppWindowLauncherController to use a Widget observer.
This works in both the mash and non-mash code. I did not promote this logic
to AppWindowLauncherController as AppWindowLauncherController is also used
for arc/crostini windows, which are created in ash and should continue to
use AppWindowLauncherController (at least for the single-process-mash case).
For multi-process-mash the arc/crostini launcher code likely does not work at
all (because the windows are entirely created by ash) and will need more
extensive changes.

BUG=826386
TEST=covered by test

Change-Id: If5f89d88f782ce452f10889da17210f41189dc77
Reviewed-on: https://chromium-review.googlesource.com/c/1306355Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604397}

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

@@ -13,13 +13,12 @@
 AppWindowLauncherController::AppWindowLauncherController(
     ChromeLauncherController* owner)
     : owner_(owner) {
-  if (ash::Shell::HasInstance()) {
-    if (ash::Shell::Get()->GetPrimaryRootWindow()) {
-      activation_client_ =
-          wm::GetActivationClient(ash::Shell::Get()->GetPrimaryRootWindow());
-      if (activation_client_)
-        activation_client_->AddObserver(this);
-    }
+  // TODO: this doesn't work in mash: https://crbug.com/826386 .
+  if (ash::Shell::HasInstance() && ash::Shell::Get()->GetPrimaryRootWindow()) {
+    activation_client_ =
+        wm::GetActivationClient(ash::Shell::Get()->GetPrimaryRootWindow());
+    if (activation_client_)
+      activation_client_->AddObserver(this);
   }
   owner->shelf_model()->AddObserver(this);
 }

chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h

@@ -73,6 +73,8 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
   const WindowList& windows() const { return windows_; }
 
  private:
+  friend class ChromeLauncherControllerTest;
+
   // Returns the action performed. Should be one of SHELF_ACTION_NONE,
   // SHELF_ACTION_WINDOW_ACTIVATED, or SHELF_ACTION_WINDOW_MINIMIZED.
   ash::ShelfAction ShowAndActivateOrMinimize(ui::BaseWindow* window);

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

@@ -552,6 +552,11 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
     extension_service_->AddExtension(extension_chrome_.get());
   }
 
+  ui::BaseWindow* GetLastActiveWindowForItemController(
+      AppWindowLauncherItemController* item_controller) {
+    return item_controller->last_active_window_;
+  }
+
   // Creates a running platform V2 app (not pinned) of type |app_id|.
   virtual void CreateRunningV2App(const std::string& app_id) {
     DCHECK(!test_controller_);
@@ -3446,6 +3451,58 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
   EXPECT_EQ(3, model_->item_count());
 }
 
+TEST_F(ChromeLauncherControllerTest, Active) {
+  InitLauncherController();
+
+  // Creates a new app window.
+  int initial_item_count = model_->item_count();
+  V2App app_1(profile(), extension1_.get());
+  EXPECT_TRUE(app_1.window()->GetNativeWindow()->IsVisible());
+  EXPECT_EQ(initial_item_count + 1, model_->item_count());
+  ash::ShelfItemDelegate* app_item_delegate_1 =
+      model_->GetShelfItemDelegate(model_->items()[initial_item_count].id);
+  ASSERT_TRUE(app_item_delegate_1);
+  AppWindowLauncherItemController* app_item_controller_1 =
+      app_item_delegate_1->AsAppWindowLauncherItemController();
+  ASSERT_TRUE(app_item_controller_1);
+  ui::BaseWindow* last_active =
+      GetLastActiveWindowForItemController(app_item_controller_1);
+  EXPECT_EQ(app_1.window()->GetBaseWindow(), last_active);
+  // Change the status so that we can verify it gets reset when the active
+  // window changes.
+  launcher_controller_->SetItemStatus(app_item_delegate_1->shelf_id(),
+                                      ash::STATUS_ATTENTION);
+
+  // Creates another app window, which should become active and reset |app_1|'s
+  // status (to running).
+  V2App app_2(profile(), extension2_.get());
+  EXPECT_TRUE(app_2.window()->GetNativeWindow()->IsVisible());
+  EXPECT_EQ(initial_item_count + 2, model_->item_count());
+  ash::ShelfItemDelegate* app_item_delegate_2 =
+      model_->GetShelfItemDelegate(model_->items()[initial_item_count + 1].id);
+  ASSERT_TRUE(app_item_delegate_2);
+  AppWindowLauncherItemController* app_item_controller_2 =
+      app_item_delegate_2->AsAppWindowLauncherItemController();
+  ASSERT_TRUE(app_item_controller_2);
+  last_active = GetLastActiveWindowForItemController(app_item_controller_2);
+  EXPECT_EQ(app_2.window()->GetBaseWindow(), last_active);
+  const ash::ShelfItem* shelf_item_1 =
+      launcher_controller_->GetItem(app_item_delegate_1->shelf_id());
+  ASSERT_TRUE(shelf_item_1);
+  EXPECT_EQ(ash::STATUS_RUNNING, shelf_item_1->status);
+
+  launcher_controller_->SetItemStatus(app_item_delegate_2->shelf_id(),
+                                      ash::STATUS_ATTENTION);
+
+  // Activate the first window, which should reset the status of the
+  // second apps window.
+  app_1.window()->GetBaseWindow()->Activate();
+  const ash::ShelfItem* shelf_item_2 =
+      launcher_controller_->GetItem(app_item_delegate_2->shelf_id());
+  ASSERT_TRUE(shelf_item_2);
+  EXPECT_EQ(ash::STATUS_RUNNING, shelf_item_2->status);
+}
+
 // Check that V2 applications will be made visible on the target desktop if
 // another window of the same type got previously teleported there.
 TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

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

@@ -22,6 +22,7 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/base/base_window.h"
+#include "ui/views/widget/widget.h"
 
 using extensions::AppWindow;
 using extensions::AppWindowRegistry;
@@ -62,8 +63,12 @@ ExtensionAppWindowLauncherController::ExtensionAppWindowLauncherController(
 ExtensionAppWindowLauncherController::~ExtensionAppWindowLauncherController() {
   registry_->RemoveObserver(this);
 
-  for (const auto& iter : window_to_shelf_id_map_)
+  for (const auto& iter : window_to_shelf_id_map_) {
     iter.first->RemoveObserver(this);
+    views::Widget* widget = views::Widget::GetWidgetForNativeView(iter.first);
+    DCHECK(widget);  // Extension windows are always backed by Widgets.
+    widget->RemoveObserver(this);
+  }
 }
 
 AppWindowLauncherItemController*
@@ -99,6 +104,15 @@ void ExtensionAppWindowLauncherController::OnItemDelegateDiscarded(
   }
 }
 
+void ExtensionAppWindowLauncherController::OnWindowActivated(
+    wm::ActivationChangeObserver::ActivationReason reason,
+    aura::Window* gained_active,
+    aura::Window* lost_active) {
+  // All work is done in OnWidgetActivationChanged(). This does nothing as the
+  // supplied windows are created by ash, which is *not* the same as the windows
+  // created by the browser when running in mash.
+}
+
 void ExtensionAppWindowLauncherController::OnAppWindowAdded(
     extensions::AppWindow* app_window) {
   RegisterApp(app_window);
@@ -127,6 +141,29 @@ void ExtensionAppWindowLauncherController::OnWindowDestroying(
   UnregisterApp(window);
 }
 
+void ExtensionAppWindowLauncherController::OnWidgetActivationChanged(
+    views::Widget* widget,
+    bool active) {
+  AppWindowLauncherItemController* controller = nullptr;
+  if (active) {
+    aura::Window* active_window = widget->GetNativeWindow();
+    DCHECK(active_window);
+    controller = ControllerForWindow(active_window);
+    DCHECK(controller);  // Observer is only added for known controllers.
+    controller->SetActiveWindow(active_window);
+  }
+  if (!active_shelf_id_.IsNull() &&
+      (!controller || controller->shelf_id() != active_shelf_id_)) {
+    owner()->SetItemStatus(active_shelf_id_, ash::STATUS_RUNNING);
+  }
+  active_shelf_id_ = controller ? controller->shelf_id() : ash::ShelfID();
+}
+
+void ExtensionAppWindowLauncherController::OnWidgetDestroying(
+    views::Widget* widget) {
+  widget->RemoveObserver(this);
+}
+
 void ExtensionAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
   aura::Window* window = app_window->GetNativeWindow();
   const ash::ShelfID shelf_id = GetShelfId(app_window);
@@ -147,6 +184,10 @@ void ExtensionAppWindowLauncherController::RegisterApp(AppWindow* app_window) {
   window_to_shelf_id_map_[window] = shelf_id;
   window->AddObserver(this);
 
+  views::Widget* widget = views::Widget::GetWidgetForNativeView(window);
+  DCHECK(widget);  // Extension windows are always backed by Widgets.
+  widget->AddObserver(this);
+
   // Find or create an item controller and launcher item.
   AppControllerMap::iterator app_controller_iter =
       app_controller_map_.find(shelf_id);
@@ -184,6 +225,10 @@ void ExtensionAppWindowLauncherController::UnregisterApp(aura::Window* window) {
   window_to_shelf_id_map_.erase(window_iter);
   window->RemoveObserver(this);
 
+  views::Widget* widget = views::Widget::GetWidgetForNativeView(window);
+  DCHECK(widget);  // Extension windows are always backed by Widgets.
+  widget->RemoveObserver(this);
+
   AppControllerMap::iterator app_controller_iter =
       app_controller_map_.find(shelf_id);
   if (app_controller_iter == app_controller_map_.end())

chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.h

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_controller.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "ui/aura/window_observer.h"
+#include "ui/views/widget/widget_observer.h"
 
 namespace aura {
 class Window;
@@ -29,10 +30,16 @@ class ExtensionAppWindowLauncherItemController;
 // AppWindowLauncherController observes the app window registry and the
 // aura window manager. It handles adding and removing launcher items from
 // ChromeLauncherController.
+//
+// This uses WidgetObserver to observe activation changes rather than
+// wm::ActivationChangeObserver (inherited from the superclass). WidgetObserver
+// works with mash, where as wm::ActivationChangeObserver only works in the
+// ash process. See https://crbug.com/826386 for details.
 class ExtensionAppWindowLauncherController
     : public AppWindowLauncherController,
       public extensions::AppWindowRegistry::Observer,
-      public aura::WindowObserver {
+      public aura::WindowObserver,
+      public views::WidgetObserver {
  public:
   explicit ExtensionAppWindowLauncherController(
       ChromeLauncherController* owner);
@@ -42,6 +49,9 @@ class ExtensionAppWindowLauncherController
   AppWindowLauncherItemController* ControllerForWindow(
       aura::Window* window) override;
   void OnItemDelegateDiscarded(ash::ShelfItemDelegate* delegate) override;
+  void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
+                         aura::Window* gained_active,
+                         aura::Window* lost_active) override;
 
   // Overridden from AppWindowRegistry::Observer:
   void OnAppWindowAdded(extensions::AppWindow* app_window) override;
@@ -52,6 +62,10 @@ class ExtensionAppWindowLauncherController
   // Overriden from aura::WindowObserver:
   void OnWindowDestroying(aura::Window* window) override;
 
+  // Overriden from views::WidgetObserver:
+  void OnWidgetActivationChanged(views::Widget* widget, bool active) override;
+  void OnWidgetDestroying(views::Widget* widget) override;
+
  protected:
   // Registers an app window with the shelf and this object.
   void RegisterApp(extensions::AppWindow* app_window);
@@ -75,6 +89,10 @@ class ExtensionAppWindowLauncherController
   // Map of aura::Windows to shelf ids.
   std::map<aura::Window*, ash::ShelfID> window_to_shelf_id_map_;
 
+  // If the active Widget is an extension, this is its ShelfID, otherwise this
+  // is empty.
+  ash::ShelfID active_shelf_id_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionAppWindowLauncherController);
 };
 

ui/views/widget/native_widget_aura.cc

@@ -594,9 +594,8 @@ void NativeWidgetAura::Activate() {
 
   // We don't necessarily have a root window yet. This can happen with
   // constrained windows.
-  if (window_->GetRootWindow()) {
+  if (window_->GetRootWindow())
     wm::GetActivationClient(window_->GetRootWindow())->ActivateWindow(window_);
-  }
   if (window_->GetProperty(aura::client::kDrawAttentionKey))
     window_->SetProperty(aura::client::kDrawAttentionKey, false);
 }