Reland "Tweak tablet mode transition timing for shelf"

This is a reland of commit 08d04bd7a2

Original change's description:
> Tweak tablet mode transition timing for shelf
>
> Updates tablet mode transition logic to explicitly update the shelf
> configuration to match target tablet mode state. This can ensure that
> the shelf transition happens:
> 1.  After the screenshots for the transition have been taken -
>     screenshot logic creates a phantom layer for shelf by recreating
>     shelf container layer (updating shelf before hand would result in
>     incorrect shelf state, and cause issues like b/328360925)
> 2.  Before windows get updated, to avoid changing work area bounds while
>     app window animation is in progress.
>
> Change-Id: I85a66578c040577f6644a63457041d8555ab88fa
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5460975
> Reviewed-by: Sammie Quon <sammiequon@chromium.org>
> Commit-Queue: Toni Barzic <tbarzic@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1288972}

BUG=b/328360925

Change-Id: I0151bed2ec2c1fa19c85797064390e6d63d41d8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466803
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290234}

ash/public/cpp/shelf_config.h

@@ -75,7 +75,6 @@ class ASH_EXPORT ShelfConfig : public SessionObserver,
   void OnSessionStateChanged(session_manager::SessionState state) override;
 
   // DisplayObserver:
-  void OnDisplayTabletStateChanged(display::TabletState state) override;
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
 
@@ -85,6 +84,10 @@ class ASH_EXPORT ShelfConfig : public SessionObserver,
   // AppListControllerObserver:
   void OnAppListVisibilityWillChange(bool shown, int64_t display_id) override;
 
+  // Updates the shelf configuration to match the provided tablet mode state.
+  // Called during transitions to enter or exit tablet mode.
+  void UpdateForTabletMode(bool in_tablet_mode);
+
   // Whether the shelf control buttons must be shown for accessibility
   // reasons.
   bool ShelfControlsForcedShownForAccessibility() const;

ash/shelf/shelf_config.cc

@@ -212,36 +212,11 @@ void ShelfConfig::OnSessionStateChanged(session_manager::SessionState state) {
   UpdateConfig(is_app_list_visible_, /*tablet_mode_changed=*/false);
 }
 
-void ShelfConfig::OnDisplayTabletStateChanged(display::TabletState state) {
-  switch (state) {
-    case display::TabletState::kInClamshellMode:
-      break;
-    case display::TabletState::kEnteringTabletMode:
-      // Update the shelf config at the "starting" stage of the tablet mode
-      // transition, so that the shelf bounds are set and remains stable during
-      // the transition animation. Otherwise, updating the shelf bounds during
-      // the animation will lead to work-area bounds changes which lead to many
-      // re-layouts, hurting the animation's smoothness.
-      // https://crbug.com/1044316.
-      DCHECK(!in_tablet_mode_);
-      in_tablet_mode_ = true;
-
-      UpdateConfig(is_app_list_visible_, /*tablet_mode_changed=*/true);
-      break;
-    case display::TabletState::kInTabletMode:
-      break;
-    case display::TabletState::kExitingTabletMode:
-      // Many events can lead to UpdateConfig being called as a result of
-      // kInClamshellMode event, therefore we need to listen to the "ending"
-      // stage rather than the "ended", so `in_tablet_mode_` gets updated
-      // correctly, and the shelf bounds are stabilized early so as not to have
-      // multiple unnecessary work-area bounds changes.
-      in_tablet_mode_ = false;
-
-      UpdateConfig(is_app_list_visible_, /*tablet_mode_changed=*/true);
-
-      has_shown_elevated_app_bar_ = std::nullopt;
-      break;
+void ShelfConfig::UpdateForTabletMode(bool in_tablet_mode) {
+  in_tablet_mode_ = in_tablet_mode;
+  UpdateConfig(is_app_list_visible_, /*tablet_mode_changed=*/true);
+  if (!in_tablet_mode_) {
+    has_shown_elevated_app_bar_ = std::nullopt;
   }
 }
 

ash/system/overview/overview_button_tray_unittest.cc

@@ -447,18 +447,22 @@ TEST_F(OverviewButtonTrayTest, ForDevTabletModeForcesTheButtonShown) {
   EXPECT_FALSE(display::Screen::GetScreen()->InTabletMode());
   EXPECT_FALSE(GetTray()->GetVisible());
 
-  // When there is a window, a screenshot will be taken and entering tablet mode
-  // becomes asynchronous, but the display tablet state is synchronously
-  // updated.
+  // When there is a window, a screenshot will be taken before shelf enters
+  // tablet mode state.
   std::unique_ptr<aura::Window> window(
       CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
 
   EXPECT_FALSE(GetTray()->GetVisible());
+  TabletMode::Waiter waiter(/*enable=*/true);
   Shell::Get()->tablet_mode_controller()->SetEnabledForDev(true);
+  EXPECT_FALSE(GetTray()->GetVisible());
+
+  waiter.Wait();
+
   EXPECT_TRUE(display::Screen::GetScreen()->InTabletMode());
   EXPECT_TRUE(GetTray()->GetVisible());
 
-  // Disabling tablet mode is always synchronous.
+  // When disabling tablet mode, shelf state updates synchronously.
   Shell::Get()->tablet_mode_controller()->SetEnabledForDev(false);
   EXPECT_FALSE(display::Screen::GetScreen()->InTabletMode());
   EXPECT_FALSE(GetTray()->GetVisible());

ash/wm/tablet_mode/tablet_mode_controller.cc

@@ -908,6 +908,13 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
     Shell::Get()->display_manager()->SetTabletState(
         display::TabletState::kExitingTabletMode);
 
+    // Many events can lead to shelf config updates as a result of
+    // kInClamshellMode event. Update the shelf config during "ending"
+    // stage rather than the "ended", so `in_tablet_mode_` gets updated
+    // correctly, and the shelf bounds are stabilized early so as not to have
+    // multiple unnecessary work-area bounds changes.
+    ShelfConfig::Get()->UpdateForTabletMode(/*in_tablet_mode=*/false);
+
     if (tablet_mode_window_manager_) {
       tablet_mode_window_manager_->Shutdown(
           TabletModeWindowManager::ShutdownReason::kExitTabletUIMode);
@@ -1167,6 +1174,18 @@ void TabletModeController::FinishInitTabletMode() {
   DCHECK_EQ(display::TabletState::kEnteringTabletMode,
             display::Screen::GetScreen()->GetTabletState());
 
+  // Transition shelf to tablet mode state, now that the screenshot for tablet
+  // mode transition was taken. Taking screenshot recreates shelf container
+  // layer, and uses the original layer - changing shelf state before the
+  // screenshot is taken would change the shelf appearance, and could cause
+  // issues where the original shelf widget layer is not re-painted correctly in
+  // response to a paint schedule for tablet mode state change.
+  // Update the shelf state befire initiating tablet mode window state changes
+  // to avoid negative impact of window work-area changes (due to changes in
+  // shelf bounds) during window state transition on the animation smoothness
+  // https://crbug.com/1044316.
+  ShelfConfig::Get()->UpdateForTabletMode(/*in_tablet_mode=*/true);
+
   tablet_mode_window_manager_ = std::make_unique<TabletModeWindowManager>();
   tablet_mode_window_manager_->Init();