overview: Ensure auto snapping from overveiw for async apps in tablet splitview

This CL fixes the issue that selecting async apps (e.g. ARC apps) from overview does not trigger auto snapping in tablet splitview.
In tablet splitview, when a window is snapped, we can select which window to fill (be snapped in) the other side.
It works for synchronized Chrome windows, but not for async windows because calling Unminimize(), which is used for canceling the restore animation, bypasses auto snapping.
So we should use Show() here and extract async activation logic from OverviewItem to make sure it works for async apps.

BUG=b:162552497
TEST=SelectMinimizedSnappableWindowInSplitView

Change-Id: I653451533c58a6241130a5ab369bb080290810af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327364
Commit-Queue: Toshiki Kikuchi <toshikikikuchi@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795727}

ash/wm/overview/overview_item.cc

@@ -1096,14 +1096,6 @@ void OverviewItem::OnPostWindowStateTypeChange(WindowState* window_state,
   item_widget_->GetLayer()->SetOpacity(1.f);
 
   overview_grid_->PositionWindows(/*animate=*/false);
-  // If |activate_on_unminimized_| is true, then this code path is from an
-  // unminimizing an ARC app async from |OverviewSession::SelectWindow|.
-  if (activate_on_unminimized_) {
-    activate_on_unminimized_ = false;
-    DCHECK(!minimized);
-    SetOpacity(1.f);
-    wm::ActivateWindow(GetWindow());
-  }
 }
 
 gfx::Rect OverviewItem::GetShadowBoundsForTesting() {

ash/wm/overview/overview_item.h

@@ -262,8 +262,6 @@ class ASH_EXPORT OverviewItem : public views::ButtonListener,
 
   void set_disable_mask(bool disable) { disable_mask_ = disable; }
 
-  void set_activate_on_unminimized(bool val) { activate_on_unminimized_ = val; }
-
   void set_unclipped_size(base::Optional<gfx::Size> unclipped_size) {
     unclipped_size_ = unclipped_size;
   }
@@ -416,13 +414,6 @@ class ASH_EXPORT OverviewItem : public views::ButtonListener,
 
   bool prepared_for_overview_ = false;
 
-  // If true, the next time |window_| is uniminimized, we will activate it (and
-  // end overview). Done this way because some windows (ARC app windows) have
-  // their window states changed async, so we need to wait until the window is
-  // fully unminimized before activation as opposed to having two consecutive
-  // calls.
-  bool activate_on_unminimized_ = false;
-
   // This has a value when there is a snapped window, or a window about to be
   // snapped (triggering a splitview preview area). This will be set when items
   // are positioned in OverviewGrid. The bounds delivered in |SetBounds| are the

ash/wm/overview/overview_session.cc

@@ -77,6 +77,50 @@ void EndOverview() {
 
 }  // namespace
 
+// A self-deleting window state observer that runs the given callback when its
+// associated window state has been changed.
+class AsyncWindowStateChangeObserver : public WindowStateObserver,
+                                       public aura::WindowObserver {
+ public:
+  AsyncWindowStateChangeObserver(
+      aura::Window* window,
+      base::OnceCallback<void(WindowState*)> on_post_window_state_changed)
+      : window_(window),
+        on_post_window_state_changed_(std::move(on_post_window_state_changed)) {
+    DCHECK(!on_post_window_state_changed_.is_null());
+    WindowState::Get(window_)->AddObserver(this);
+    window_->AddObserver(this);
+  }
+
+  ~AsyncWindowStateChangeObserver() override { RemoveAllObservers(); }
+
+  AsyncWindowStateChangeObserver(const AsyncWindowStateChangeObserver&) =
+      delete;
+  AsyncWindowStateChangeObserver& operator=(
+      const AsyncWindowStateChangeObserver&) = delete;
+
+  // aura::WindowObserver:
+  void OnWindowDestroying(aura::Window* window) override { delete this; }
+
+  // WindowStateObserver:
+  void OnPostWindowStateTypeChange(WindowState* window_state,
+                                   WindowStateType) override {
+    RemoveAllObservers();
+    std::move(on_post_window_state_changed_).Run(window_state);
+    delete this;
+  }
+
+ private:
+  void RemoveAllObservers() {
+    WindowState::Get(window_)->RemoveObserver(this);
+    window_->RemoveObserver(this);
+  }
+
+  aura::Window* window_;
+
+  base::OnceCallback<void(WindowState*)> on_post_window_state_changed_;
+};
+
 OverviewSession::OverviewSession(OverviewDelegate* delegate)
     : delegate_(delegate),
       restore_focus_window_(window_util::GetFocusedWindow()),
@@ -327,12 +371,24 @@ void OverviewSession::SelectWindow(OverviewItem* item) {
   }
   // If the selected window is a minimized window, un-minimize it first before
   // activating it so that the window can use the scale-up animation instead of
-  // un-minimizing animation. If minimized, the activation of the window will
-  // happen in OverviewItem which listens for window state changes.
-  if (WindowState::Get(window)->IsMinimized()) {
-    item->set_activate_on_unminimized(true);
+  // un-minimizing animation. The activation of the window will happen in an
+  // asynchronous manner on window state has been changed. That's because some
+  // windows (ARC app windows) have their window states changed async, so we
+  // need to wait until the window is fully unminimized before activation as
+  // opposed to having two consecutive calls.
+  auto* window_state = WindowState::Get(window);
+  if (window_state->IsMinimized()) {
     ScopedAnimationDisabler disabler(window);
-    WindowState::Get(window)->Unminimize();
+    // The following instance self-destructs when the window state changed.
+    new AsyncWindowStateChangeObserver(
+        window, base::BindOnce([](WindowState* window_state) {
+          for (auto* window_iter :
+               GetVisibleTransientTreeIterator(window_state->window())) {
+            window_iter->layer()->SetOpacity(1.0);
+          }
+          wm::ActivateWindow(window_state->window());
+        }));
+    window->Show();
     return;
   }
 

ash/wm/overview/overview_session_unittest.cc

@@ -5954,6 +5954,48 @@ TEST_P(SplitViewOverviewSessionTest, OnScreenLock) {
             split_view_controller()->state());
 }
 
+// Verify that selecting an minimized snappable window while in split view
+// triggers auto snapping.
+TEST_P(SplitViewOverviewSessionTest,
+       SelectMinimizedSnappableWindowInSplitView) {
+  // Create two snappable windows.
+  std::unique_ptr<aura::Window> snapped_window = CreateTestWindow();
+  std::unique_ptr<aura::Window> minimized_window = CreateTestWindow();
+  WindowState::Get(minimized_window.get())->Minimize();
+
+  ToggleOverview();
+  ASSERT_TRUE(overview_controller()->InOverviewSession());
+
+  // Snap a window to enter split view mode.
+  split_view_controller()->SnapWindow(snapped_window.get(),
+                                      SplitViewController::LEFT);
+  EXPECT_EQ(SplitViewController::State::kLeftSnapped,
+            split_view_controller()->state());
+
+  // Select the minimized window.
+  OverviewItem* overview_item =
+      GetOverviewItemForWindow(minimized_window.get());
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  generator->set_current_screen_location(
+      gfx::ToRoundedPoint(overview_item->target_bounds().CenterPoint()));
+  generator->ClickLeftButton();
+
+  // Verify that both windows are in a snapped state and overview mode is ended.
+  EXPECT_TRUE(split_view_controller()->InSplitViewMode());
+  EXPECT_TRUE(
+      split_view_controller()->IsWindowInSplitView(snapped_window.get()));
+  EXPECT_EQ(
+      split_view_controller()->GetPositionOfSnappedWindow(snapped_window.get()),
+      SplitViewController::LEFT);
+  EXPECT_TRUE(
+      split_view_controller()->IsWindowInSplitView(minimized_window.get()));
+  EXPECT_EQ(split_view_controller()->GetPositionOfSnappedWindow(
+                minimized_window.get()),
+            SplitViewController::RIGHT);
+  EXPECT_FALSE(overview_controller()->InOverviewSession());
+  EXPECT_EQ(minimized_window.get(), window_util::GetActiveWindow());
+}
+
 // Test the split view and overview functionalities in clamshell mode. Split
 // view is only active when overview is active in clamshell mode.
 class SplitViewOverviewSessionInClamshellTest