RELAND - tablet: Fix edge case with |defer_bounds_updates| set inappropriately.

This CL is meant as a low-risk fix of Issue 926602 to merge back to M73.
For the long term, it will be preferable to avoid maximizing windows
before snapping them. Then we might even be able to eliminate
|defer_bounds_updates| altogether (even though it existed before
anything about snapped windows carrying over from desktop to tablet).

  TabletModeControllerTest.DeferBoundsUpdatesForWindowsInOverview*
  TabletModeControllerTest.ProgrammaticallyStartSplitViewAndThenOverview

Test: ash_unittests TabletModeControllerTest.StartTablet*
Test: ash_unittests
Test: ash_unittests
Bug: 926602
Change-Id: I0a068ffae314871c8466d1364bcd96f72b69d345
Reviewed-on: https://chromium-review.googlesource.com/c/1457235Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#631462}
Reviewed-on: https://chromium-review.googlesource.com/c/1471013
Cr-Commit-Position: refs/heads/master@{#632017}

ash/wm/tablet_mode/tablet_mode_controller_test_api.cc

@@ -5,6 +5,8 @@
 #include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
 
 #include "ash/shell.h"
+#include "ash/wm/tablet_mode/tablet_mode_window_manager.h"
+#include "ash/wm/tablet_mode/tablet_mode_window_state.h"
 #include "base/run_loop.h"
 #include "base/time/default_tick_clock.h"
 #include "services/ws/public/cpp/input_devices/input_device_client_test_api.h"
@@ -83,4 +85,14 @@ void TabletModeControllerTestApi::SetTabletMode(bool on) {
       tick_clock()->NowTicks());
 }
 
+bool TabletModeControllerTestApi::GetDeferBoundsUpdates(aura::Window* window) {
+  TabletModeWindowManager* window_manager = tablet_mode_window_manager();
+  if (window_manager == nullptr)
+    return false;
+  TabletModeWindowManager::WindowToState window_state_map =
+      window_manager->window_state_map_;
+  auto iter = window_state_map.find(window);
+  return iter != window_state_map.end() && iter->second->defer_bounds_updates_;
+}
+
 }  // namespace ash

ash/wm/tablet_mode/tablet_mode_controller_test_api.h

@@ -45,6 +45,8 @@ class TabletModeControllerTestApi {
   void CloseLid();
   void SetTabletMode(bool on);
 
+  bool GetDeferBoundsUpdates(aura::Window* window);
+
   // Sets the event blocker on the tablet mode controller.
   void set_event_blocker(
       std::unique_ptr<InternalInputDevicesEventBlocker> blocker) {

ash/wm/tablet_mode/tablet_mode_controller_unittest.cc

@@ -142,6 +142,10 @@ class TabletModeControllerTest : public AshTestBase {
 
   base::UserActionTester* user_action_tester() { return &user_action_tester_; }
 
+  bool GetDeferBoundsUpdates(aura::Window* window) {
+    return test_api_->GetDeferBoundsUpdates(window);
+  }
+
   // Creates a test window snapped on the left in desktop mode.
   std::unique_ptr<aura::Window> CreateDesktopWindowSnappedLeft() {
     std::unique_ptr<aura::Window> window = CreateTestWindow();
@@ -1115,6 +1119,7 @@ TEST_F(TabletModeControllerTest, StartTabletActiveNoSnap) {
   tablet_mode_controller()->EnableTabletModeWindowManager(true);
   EXPECT_EQ(SplitViewController::NO_SNAP, split_view_controller->state());
   EXPECT_FALSE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window.get()));
 }
 
 // Test that if the active window is snapped on the left before tablet mode,
@@ -1128,6 +1133,7 @@ TEST_F(TabletModeControllerTest, StartTabletActiveLeftSnap) {
   EXPECT_EQ(SplitViewController::LEFT_SNAPPED, split_view_controller->state());
   EXPECT_EQ(window.get(), split_view_controller->left_window());
   EXPECT_TRUE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window.get()));
 }
 
 // Test that if the active window is snapped on the right before tablet mode,
@@ -1141,6 +1147,7 @@ TEST_F(TabletModeControllerTest, StartTabletActiveRightSnap) {
   EXPECT_EQ(SplitViewController::RIGHT_SNAPPED, split_view_controller->state());
   EXPECT_EQ(window.get(), split_view_controller->right_window());
   EXPECT_TRUE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window.get()));
 }
 
 // Test that if before tablet mode, the active window is snapped on the left and
@@ -1158,6 +1165,8 @@ TEST_F(TabletModeControllerTest, StartTabletActiveLeftSnapPreviousRightSnap) {
   EXPECT_EQ(left_window.get(), split_view_controller->left_window());
   EXPECT_EQ(right_window.get(), split_view_controller->right_window());
   EXPECT_FALSE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(left_window.get()));
+  EXPECT_FALSE(GetDeferBoundsUpdates(right_window.get()));
 }
 
 // Test that if before tablet mode, the active window is snapped on the right
@@ -1175,6 +1184,8 @@ TEST_F(TabletModeControllerTest, StartTabletActiveRightSnapPreviousLeftSnap) {
   EXPECT_EQ(left_window.get(), split_view_controller->left_window());
   EXPECT_EQ(right_window.get(), split_view_controller->right_window());
   EXPECT_FALSE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(left_window.get()));
+  EXPECT_FALSE(GetDeferBoundsUpdates(right_window.get()));
 }
 
 // Test that if before tablet mode, the active window is an ARC window snapped
@@ -1193,6 +1204,8 @@ TEST_F(TabletModeControllerTest,
   tablet_mode_controller()->EnableTabletModeWindowManager(true);
   EXPECT_EQ(SplitViewController::NO_SNAP, split_view_controller->state());
   EXPECT_FALSE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(left_window.get()));
+  EXPECT_FALSE(GetDeferBoundsUpdates(right_window.get()));
 }
 
 // Test that if before tablet mode, the active window is snapped on the left,
@@ -1216,6 +1229,9 @@ TEST_F(TabletModeControllerTest,
   EXPECT_EQ(SplitViewController::LEFT_SNAPPED, split_view_controller->state());
   EXPECT_EQ(left_window.get(), split_view_controller->left_window());
   EXPECT_TRUE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(left_window.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(right_window.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(extra_right_window.get()));
 }
 
 // Test that if overview is triggered on entering tablet mode, then the app list
@@ -1230,4 +1246,82 @@ TEST_F(TabletModeControllerTest, AppListWorksAfterEnteringTabletForOverview) {
   EXPECT_TRUE(app_list_controller->IsVisible());
 }
 
+// Test that bounds updates are deferred for windows in overview.
+TEST_F(TabletModeControllerTest, DeferBoundsUpdatesForWindowsInOverview) {
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  std::unique_ptr<aura::Window> window1 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window2 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window3 = CreateTestWindow();
+  ::wm::ActivateWindow(window1.get());
+  ASSERT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  ASSERT_FALSE(GetDeferBoundsUpdates(window2.get()));
+  ASSERT_FALSE(GetDeferBoundsUpdates(window3.get()));
+  Shell::Get()->overview_controller()->ToggleOverview();
+  EXPECT_TRUE(GetDeferBoundsUpdates(window1.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window2.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window3.get()));
+}
+
+// Test that bounds updates are deferred for windows in overview, in the case
+// that there are two snapped windows when overview is started.
+TEST_F(TabletModeControllerTest,
+       DeferBoundsUpdatesForWindowsInOverviewEnteredFromSplitView) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  std::unique_ptr<aura::Window> window1 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window2 = CreateTestWindow();
+  std::unique_ptr<aura::Window> window3 = CreateTestWindow();
+  ::wm::ActivateWindow(window1.get());
+  split_view_controller->SnapWindow(window1.get(), SplitViewController::LEFT);
+  split_view_controller->SnapWindow(window2.get(), SplitViewController::RIGHT);
+  ASSERT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  ASSERT_FALSE(GetDeferBoundsUpdates(window2.get()));
+  ASSERT_FALSE(GetDeferBoundsUpdates(window3.get()));
+  Shell::Get()->overview_controller()->ToggleOverview();
+  ASSERT_EQ(SplitViewController::LEFT_SNAPPED, split_view_controller->state());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window2.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window3.get()));
+}
+
+// Test that if both the active window and the previous window are snapped on
+// the left before tablet mode, then split view is activated with the active
+// window on the left, and if the user proceeds to snap the other window on the
+// right and then enter overview again, bounds updates are still not deferred on
+// the window that remains snapped.
+TEST_F(TabletModeControllerTest, StartTabletActiveLeftSnapPreviousLeftSnap) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  std::unique_ptr<aura::Window> window1 = CreateDesktopWindowSnappedLeft();
+  std::unique_ptr<aura::Window> window2 = CreateDesktopWindowSnappedLeft();
+  ::wm::ActivateWindow(window1.get());
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  EXPECT_EQ(SplitViewController::LEFT_SNAPPED, split_view_controller->state());
+  EXPECT_EQ(window1.get(), split_view_controller->left_window());
+  EXPECT_TRUE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window2.get()));
+  split_view_controller->SnapWindow(window2.get(), SplitViewController::RIGHT);
+  EXPECT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  EXPECT_FALSE(GetDeferBoundsUpdates(window2.get()));
+  ASSERT_TRUE(Shell::Get()->overview_controller()->ToggleOverview());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window1.get()));
+  EXPECT_TRUE(GetDeferBoundsUpdates(window2.get()));
+}
+
+// Test that it is okay to write code that first starts split view by snapping a
+// window on one side, and then starts overview to be seen on the other side.
+TEST_F(TabletModeControllerTest,
+       ProgrammaticallyStartSplitViewAndThenOverview) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  std::unique_ptr<aura::Window> window = CreateTestWindow();
+  ::wm::ActivateWindow(window.get());
+  split_view_controller->SnapWindow(window.get(), SplitViewController::LEFT);
+  EXPECT_TRUE(Shell::Get()->overview_controller()->ToggleOverview());
+  EXPECT_FALSE(GetDeferBoundsUpdates(window.get()));
+}
+
 }  // namespace ash

ash/wm/tablet_mode/tablet_mode_window_manager.cc

@@ -115,8 +115,14 @@ void TabletModeWindowManager::OnSplitViewModeEnded() {
 }
 
 void TabletModeWindowManager::OnOverviewModeStarting() {
-  for (auto& pair : window_state_map_)
-    SetDeferBoundsUpdates(pair.first, /*defer_bounds_updates=*/true);
+  aura::Window* default_snapped_window =
+      Shell::Get()->split_view_controller()->GetDefaultSnappedWindow();
+  for (auto& pair : window_state_map_) {
+    aura::Window* window = pair.first;
+    if (window == default_snapped_window)
+      continue;
+    SetDeferBoundsUpdates(window, /*defer_bounds_updates=*/true);
+  }
 }
 
 void TabletModeWindowManager::OnOverviewModeEnding(

ash/wm/tablet_mode/tablet_mode_window_manager.h

@@ -97,6 +97,8 @@ class ASH_EXPORT TabletModeWindowManager
   TabletModeWindowManager();
 
  private:
+  friend class TabletModeControllerTestApi;
+
   using WindowToState = std::map<aura::Window*, TabletModeWindowState*>;
 
   // Maximize all windows, except that a snapped active window shall become

ash/wm/tablet_mode/tablet_mode_window_state.h

@@ -54,6 +54,8 @@ class TabletModeWindowState : public wm::WindowState::State {
   }
 
  private:
+  friend class TabletModeControllerTestApi;
+
   // Updates the window to |new_state_type| and resulting bounds:
   // Either full screen, maximized centered or minimized. If the state does not
   // change, only the bounds will be changed. If |animate| is set, the bound