full_restore: Use tablet bounds and state overrides when saving.

This will help us restore windows properly in the case we open the
window in tablet mode, then try to restore them when relogin in
clamshell mode.

Manually tested, opening a window in clamshell mode, then try to restore
it in tablet mode works as expected. Added a test case to catch
regressions.

Bug: 1164472
Test: manual, added test
Change-Id: I4aa96de8970451cbab1c168b4f340916b5b6c599
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2853776
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#876729}

ash/wm/full_restore/full_restore_controller.cc

@@ -8,6 +8,7 @@
 
 #include "ash/public/cpp/app_types.h"
 #include "ash/public/cpp/shell_window_ids.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
@@ -297,20 +298,30 @@ void FullRestoreController::SaveWindowImpl(
     // storage size.
     window_info.visible_on_all_workspaces = true;
   }
-  // If there are restore bounds, use those as current bounds. On restore, for
-  // states with restore bounds (maximized, minimized, snapped, etc), they will
-  // take the current bounds as their restore bounds and have the current bounds
-  // determined by the system.
-  window_info.current_bounds = window_state->HasRestoreBounds()
-                                   ? window_state->GetRestoreBoundsInScreen()
-                                   : window->GetBoundsInScreen();
-  // Full restore does not support restoring fullscreen windows. If a window is
-  // fullscreen save the pre-fullscreen window state instead.
-  window_info.window_state_type =
-      window_state->IsInImmersiveFullscreen()
-          ? chromeos::ToWindowStateType(
-                window->GetProperty(aura::client::kPreFullscreenShowStateKey))
-          : window_state->GetStateType();
+
+  // If override bounds and window state are available (in tablet mode), save
+  // those bounds.
+  gfx::Rect* override_bounds = window->GetProperty(kRestoreBoundsOverrideKey);
+  if (override_bounds) {
+    window_info.current_bounds = *override_bounds;
+    window_info.window_state_type =
+        window->GetProperty(kRestoreWindowStateTypeOverrideKey);
+  } else {
+    // If there are restore bounds, use those as current bounds. On restore, for
+    // states with restore bounds (maximized, minimized, snapped, etc), they
+    // will take the current bounds as their restore bounds and have the current
+    // bounds determined by the system.
+    window_info.current_bounds = window_state->HasRestoreBounds()
+                                     ? window_state->GetRestoreBoundsInScreen()
+                                     : window->GetBoundsInScreen();
+    // Full restore does not support restoring fullscreen windows. If a window
+    // is fullscreen save the pre-fullscreen window state instead.
+    window_info.window_state_type =
+        window_state->IsInImmersiveFullscreen()
+            ? chromeos::ToWindowStateType(
+                  window->GetProperty(aura::client::kPreFullscreenShowStateKey))
+            : window_state->GetStateType();
+  }
 
   window_info.display_id =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window).id();

ash/wm/full_restore/full_restore_controller_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "ash/accelerators/accelerator_controller_impl.h"
 #include "ash/public/cpp/ash_features.h"
+#include "ash/screen_util.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_widget_builder.h"
@@ -33,6 +34,13 @@ void PerformAcceleratorAction(AcceleratorAction action,
                                                                  accelerator);
 }
 
+void SetResizable(views::Widget* widget) {
+  widget->GetNativeWindow()->SetProperty(
+      aura::client::kResizeBehaviorKey,
+      aura::client::kResizeBehaviorCanResize |
+          aura::client::kResizeBehaviorCanMaximize);
+}
+
 }  // namespace
 
 class FullRestoreControllerTest : public AshTestBase, public aura::EnvObserver {
@@ -75,14 +83,21 @@ class FullRestoreControllerTest : public AshTestBase, public aura::EnvObserver {
       member.second.call_count = 0;
   }
 
-  // Returns the stored activation index for |window|.
-  int GetActivationIndex(aura::Window* window) const {
+  // Returns window info for `window`.
+  full_restore::WindowInfo* GetWindowInfo(aura::Window* window) const {
     const int32_t restore_window_id =
         window->GetProperty(full_restore::kRestoreWindowIdKey);
     if (!base::Contains(fake_full_restore_file_, restore_window_id))
+      return nullptr;
+    return fake_full_restore_file_.at(restore_window_id).info.get();
+  }
+
+  // Returns the stored activation index for |window|.
+  int GetActivationIndex(aura::Window* window) const {
+    full_restore::WindowInfo* window_info = GetWindowInfo(window);
+    if (!window_info)
       return -1;
-    base::Optional<int32_t> activation_index =
-        fake_full_restore_file_.at(restore_window_id).info->activation_index;
+    base::Optional<int32_t> activation_index = window_info->activation_index;
     return activation_index.value_or(-1);
   }
 
@@ -99,23 +114,22 @@ class FullRestoreControllerTest : public AshTestBase, public aura::EnvObserver {
     TestWidgetBuilder widget_builder;
     widget_builder.SetWidgetType(views::Widget::InitParams::TYPE_WINDOW)
         .SetBounds(bounds)
-        .SetContext(root_window);
-    widget_builder.SetWindowProperty(full_restore::kActivationIndexKey,
-                                     new int32_t(activation_index));
-    widget_builder.SetWindowProperty(full_restore::kLaunchedFromFullRestoreKey,
-                                     true);
+        .SetShow(false)
+        .SetContext(root_window)
+        .SetWindowProperty(full_restore::kActivationIndexKey,
+                           new int32_t(activation_index))
+        .SetWindowProperty(full_restore::kLaunchedFromFullRestoreKey, true);
     // If this is not given, the window will get assigned an id in
     // `OnWindowInitialized()`.
     if (restore_window_id) {
       widget_builder.SetWindowProperty(full_restore::kRestoreWindowIdKey,
                                        *restore_window_id);
     }
+
     views::Widget* widget = widget_builder.BuildOwnedByNativeWidget();
-    widget->GetNativeWindow()->SetProperty(
-        aura::client::kResizeBehaviorKey,
-        aura::client::kResizeBehaviorCanResize |
-            aura::client::kResizeBehaviorCanMaximize);
+    SetResizable(widget);
     FullRestoreController::Get()->OnWidgetInitialized(widget);
+    widget->Show();
     return widget;
   }
 
@@ -747,4 +761,76 @@ TEST_F(FullRestoreControllerTest, DisplaySizeChange) {
   EXPECT_TRUE(WindowState::Get(restored_window)->IsNormalStateType());
 }
 
+// Tests full restore behavior for when a window saved in clamshell mode is
+// restored as expected in tablet mode.
+TEST_F(FullRestoreControllerTest, ClamshellToTablet) {
+  constexpr int kRestoreId = 1;
+
+  // Add a normal window to the fake file.
+  const gfx::Rect clamshell_bounds(400, 400);
+  AddEntryToFakeFile(kRestoreId, clamshell_bounds,
+                     chromeos::WindowStateType::kNormal);
+
+  // Restore the window after entering tablet mode, it should be maximized.
+  TabletModeControllerTestApi().EnterTabletMode();
+  auto* restored_window =
+      CreateTestFullRestoredWidgetFromRestoreId(kRestoreId)->GetNativeWindow();
+  EXPECT_TRUE(WindowState::Get(restored_window)->IsMaximized());
+  EXPECT_EQ(screen_util::GetMaximizedWindowBoundsInParent(restored_window),
+            restored_window->GetBoundsInScreen());
+
+  // Leave tablet mode. The window should have the saved bounds.
+  TabletModeControllerTestApi().LeaveTabletMode();
+  EXPECT_FALSE(WindowState::Get(restored_window)->IsMaximized());
+  EXPECT_EQ(clamshell_bounds, restored_window->GetBoundsInScreen());
+}
+
+// Tests full restore behavior for when a window saved in tablet mode is
+// restored as expected in clamshell mode.
+TEST_F(FullRestoreControllerTest, TabletToClamshell) {
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  // The tablet mode window manager watches windows when they are added, then
+  // tracks them when the window is shown. They must be resizable when tracked,
+  // so we use a TestWidgetBuilder instead of `CreateTestWindow()`, which would
+  // show the window before we can make it resizable.
+  const gfx::Rect expected_bounds(300, 300);
+  TestWidgetBuilder builder;
+  views::Widget* widget =
+      builder.SetTestWidgetDelegate()
+          .SetBounds(expected_bounds)
+          .SetContext(Shell::GetPrimaryRootWindow())
+          .SetShow(false)
+          .SetWindowProperty(aura::client::kAppType,
+                             static_cast<int>(AppType::CHROME_APP))
+          .BuildOwnedByNativeWidget();
+  SetResizable(widget);
+  widget->Show();
+
+  aura::Window* window = widget->GetNativeWindow();
+  ASSERT_TRUE(WindowState::Get(window)->IsMaximized());
+  ASSERT_EQ(screen_util::GetMaximizedWindowBoundsInParent(window),
+            window->GetBoundsInScreen());
+
+  // Check that the values in the fake file can be restored in clamshell mode.
+  full_restore::WindowInfo* window_info = GetWindowInfo(window);
+  ASSERT_TRUE(window_info);
+  ASSERT_TRUE(window_info->current_bounds);
+  ASSERT_TRUE(window_info->window_state_type);
+  EXPECT_EQ(expected_bounds, *window_info->current_bounds);
+  EXPECT_EQ(chromeos::WindowStateType::kDefault,
+            *window_info->window_state_type);
+
+  const int restore_id = window->GetProperty(full_restore::kRestoreWindowIdKey);
+
+  // Leave tablet mode, and then mock creating the window from full restore
+  // file. Test that the state and bounds are as expected in clamshell mode.
+  widget->CloseWithReason(views::Widget::ClosedReason::kUnspecified);
+  TabletModeControllerTestApi().LeaveTabletMode();
+  auto* restored_window =
+      CreateTestFullRestoredWidgetFromRestoreId(restore_id)->GetNativeWindow();
+  EXPECT_TRUE(WindowState::Get(restored_window)->IsNormalStateType());
+  EXPECT_EQ(expected_bounds, window->GetBoundsInScreen());
+}
+
 }  // namespace ash