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}
diff --git a/ash/public/cpp/shelf_config.h b/ash/public/cpp/shelf_config.h
index ba256dae..e19edd6 100644
--- a/ash/public/cpp/shelf_config.h
+++ b/ash/public/cpp/shelf_config.h
@@ -75,7 +75,6 @@
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 @@
// 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;
diff --git a/ash/shelf/shelf_config.cc b/ash/shelf/shelf_config.cc
index edbfcf2..61dc446 100644
--- a/ash/shelf/shelf_config.cc
+++ b/ash/shelf/shelf_config.cc
@@ -212,36 +212,11 @@
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;
}
}
diff --git a/ash/wm/tablet_mode/tablet_mode_controller.cc b/ash/wm/tablet_mode/tablet_mode_controller.cc
index dd479eb..4aa6391 100644
--- a/ash/wm/tablet_mode/tablet_mode_controller.cc
+++ b/ash/wm/tablet_mode/tablet_mode_controller.cc
@@ -908,6 +908,13 @@
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 @@
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();
