tablet: Call CanSnapInSplitview() before transferring snap from desktop.

CanSnapInSplitview() checks stricter requirements than
WindowState::CanSnap(), meaning that a window snapped in clamshell mode
might not be appropriate to carry over to split view on entering tablet
mode. Now the tablet mode window manager shall seek approval from
CanSnapInSplitview() before attempting to carry over a snapped window to
split view on entering tablet mode.
TabletModeWindowState::TabletModeWindowState() shall no longer call
TabletModeWindowState::GetSnappedWindowStateType(), because the whole
purpose of that function is to check CanSnapInSplitview() which shall
have already been checked in the tablet mode window manager.

Test: ash_unittests TabletModeControllerTest.StartTablet*DesktopOnly*
Bug: 936478
Change-Id: If4f732d7276d7cae438fe08a919a13f170986f7d
Reviewed-on: https://chromium-review.googlesource.com/c/1492351
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636556}

ash/wm/tablet_mode/tablet_mode_controller_unittest.cc

@@ -18,9 +18,11 @@
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/tablet_mode.h"
 #include "ash/public/cpp/window_properties.h"
+#include "ash/screen_util.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/overview/overview_controller.h"
+#include "ash/wm/splitview/split_view_utils.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
 #include "ash/wm/wm_event.h"
 #include "base/command_line.h"
@@ -32,6 +34,7 @@
 #include "chromeos/dbus/fake_power_manager_client.h"
 #include "services/ws/public/cpp/input_devices/input_device_client_test_api.h"
 #include "ui/aura/client/aura_constants.h"
+#include "ui/aura/test/test_window_delegate.h"
 #include "ui/base/hit_test.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
@@ -1321,6 +1324,136 @@ TEST_F(TabletModeControllerTest,
   EXPECT_FALSE(GetDeferBoundsUpdates(snapped_window.get()));
 }
 
+// Test that if before tablet mode, the active window is snapped on the left but
+// does not meet the requirements to be snapped in split view, and the previous
+// window is snapped on the right, then split view is not activated.
+TEST_F(TabletModeControllerTest,
+       StartTabletActiveDesktopOnlyLeftSnapPreviousRightSnap) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  aura::test::TestWindowDelegate left_window_delegate;
+  std::unique_ptr<aura::Window> left_window(CreateTestWindowInShellWithDelegate(
+      &left_window_delegate, /*id=*/-1, /*bounds=*/gfx::Rect(0, 0, 400, 400)));
+  const gfx::Rect display_bounds =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForDefaultContainer(
+          left_window.get());
+  left_window_delegate.set_minimum_size(
+      gfx::Size(display_bounds.width() * 0.67f, display_bounds.height()));
+  wm::WindowState* left_window_state = wm::GetWindowState(left_window.get());
+  ASSERT_TRUE(left_window_state->CanSnap());
+  ASSERT_FALSE(CanSnapInSplitview(left_window.get()));
+  wm::WMEvent snap_to_left(wm::WM_EVENT_CYCLE_SNAP_LEFT);
+  left_window_state->OnWMEvent(&snap_to_left);
+  std::unique_ptr<aura::Window> right_window =
+      CreateDesktopWindowSnappedRight();
+  ::wm::ActivateWindow(right_window.get());
+  ::wm::ActivateWindow(left_window.get());
+  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 right
+// but does not meet the requirements to be snapped in split view, and the
+// previous window is snapped on the left, then split view is not activated.
+TEST_F(TabletModeControllerTest,
+       StartTabletActiveDesktopOnlyRightSnapPreviousLeftSnap) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  std::unique_ptr<aura::Window> left_window = CreateDesktopWindowSnappedLeft();
+  aura::test::TestWindowDelegate right_window_delegate;
+  std::unique_ptr<aura::Window> right_window(
+      CreateTestWindowInShellWithDelegate(
+          &right_window_delegate, /*id=*/-1,
+          /*bounds=*/gfx::Rect(0, 0, 400, 400)));
+  const gfx::Rect display_bounds =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForDefaultContainer(
+          right_window.get());
+  right_window_delegate.set_minimum_size(
+      gfx::Size(display_bounds.width() * 0.67f, display_bounds.height()));
+  wm::WindowState* right_window_state = wm::GetWindowState(right_window.get());
+  ASSERT_TRUE(right_window_state->CanSnap());
+  ASSERT_FALSE(CanSnapInSplitview(right_window.get()));
+  wm::WMEvent snap_to_right(wm::WM_EVENT_CYCLE_SNAP_RIGHT);
+  right_window_state->OnWMEvent(&snap_to_right);
+  ::wm::ActivateWindow(left_window.get());
+  ::wm::ActivateWindow(right_window.get());
+  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 and
+// the previous window is snapped on the right but does not meet the
+// requirements to be snapped in split view, then split view is activated with
+// the active window on the left.
+TEST_F(TabletModeControllerTest,
+       StartTabletActiveLeftSnapPreviousDesktopOnlyRightSnap) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  std::unique_ptr<aura::Window> left_window = CreateDesktopWindowSnappedLeft();
+  aura::test::TestWindowDelegate right_window_delegate;
+  std::unique_ptr<aura::Window> right_window(
+      CreateTestWindowInShellWithDelegate(
+          &right_window_delegate, /*id=*/-1,
+          /*bounds=*/gfx::Rect(0, 0, 400, 400)));
+  const gfx::Rect display_bounds =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForDefaultContainer(
+          right_window.get());
+  right_window_delegate.set_minimum_size(
+      gfx::Size(display_bounds.width() * 0.67f, display_bounds.height()));
+  wm::WindowState* right_window_state = wm::GetWindowState(right_window.get());
+  ASSERT_TRUE(right_window_state->CanSnap());
+  ASSERT_FALSE(CanSnapInSplitview(right_window.get()));
+  wm::WMEvent snap_to_right(wm::WM_EVENT_CYCLE_SNAP_RIGHT);
+  right_window_state->OnWMEvent(&snap_to_right);
+  ::wm::ActivateWindow(right_window.get());
+  ::wm::ActivateWindow(left_window.get());
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  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()));
+}
+
+// Test that if before tablet mode, the active window is snapped on the right
+// and the previous window is snapped on the left but does not meet the
+// requirements to be snapped in split view, then split view is activated with
+// the active window on the right.
+TEST_F(TabletModeControllerTest,
+       StartTabletActiveRightSnapPreviousDesktopOnlyLeftSnap) {
+  SplitViewController* split_view_controller =
+      Shell::Get()->split_view_controller();
+  aura::test::TestWindowDelegate left_window_delegate;
+  std::unique_ptr<aura::Window> left_window(CreateTestWindowInShellWithDelegate(
+      &left_window_delegate, /*id=*/-1, /*bounds=*/gfx::Rect(0, 0, 400, 400)));
+  const gfx::Rect display_bounds =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForDefaultContainer(
+          left_window.get());
+  left_window_delegate.set_minimum_size(
+      gfx::Size(display_bounds.width() * 0.67f, display_bounds.height()));
+  wm::WindowState* left_window_state = wm::GetWindowState(left_window.get());
+  ASSERT_TRUE(left_window_state->CanSnap());
+  ASSERT_FALSE(CanSnapInSplitview(left_window.get()));
+  wm::WMEvent snap_to_left(wm::WM_EVENT_CYCLE_SNAP_LEFT);
+  left_window_state->OnWMEvent(&snap_to_left);
+  std::unique_ptr<aura::Window> right_window =
+      CreateDesktopWindowSnappedRight();
+  ::wm::ActivateWindow(left_window.get());
+  ::wm::ActivateWindow(right_window.get());
+  tablet_mode_controller()->EnableTabletModeWindowManager(true);
+  EXPECT_EQ(SplitViewController::RIGHT_SNAPPED, split_view_controller->state());
+  EXPECT_EQ(right_window.get(), split_view_controller->right_window());
+  EXPECT_TRUE(Shell::Get()->overview_controller()->IsSelecting());
+  EXPECT_TRUE(GetDeferBoundsUpdates(left_window.get()));
+  EXPECT_FALSE(GetDeferBoundsUpdates(right_window.get()));
+}
+
 // Test that if overview is triggered on entering tablet mode, then the app list
 // can still be successfully shown and actually seen.
 TEST_F(TabletModeControllerTest, AppListWorksAfterEnteringTabletForOverview) {

ash/wm/tablet_mode/tablet_mode_window_manager.cc

@@ -14,6 +14,7 @@
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_session.h"
+#include "ash/wm/splitview/split_view_utils.h"
 #include "ash/wm/tablet_mode/scoped_skip_user_session_blocked_check.h"
 #include "ash/wm/tablet_mode/tablet_mode_backdrop_delegate_impl.h"
 #include "ash/wm/tablet_mode/tablet_mode_event_handler.h"
@@ -310,9 +311,10 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
   MruWindowTracker::WindowList activatable_windows =
       Shell::Get()->mru_window_tracker()->BuildWindowListIgnoreModal();
 
-  // If split_view_eligible_windows[0] does not exist or is ARC or not snapped,
-  // then just maximize all windows.
+  // If split_view_eligible_windows[0] does not exist, cannot be snapped in
+  // split view, or is ARC or not snapped, then just maximize all windows.
   if (split_view_eligible_windows.empty() ||
+      !CanSnapInSplitview(split_view_eligible_windows[0]) ||
       static_cast<ash::AppType>(split_view_eligible_windows[0]->GetProperty(
           aura::client::kAppType)) == AppType::ARC_APP ||
       !wm::GetWindowState(split_view_eligible_windows[0])->IsSnapped()) {
@@ -323,9 +325,10 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
 
   // Carry over split_view_eligible_windows[0] to split view, along with
   // split_view_eligible_windows[1] if it exists, is snapped on the opposite
-  // side, and is not ARC.
-  const bool prev_win_not_arc =
+  // side, can be snapped in split view, and is not ARC.
+  const bool prev_win_eligible =
       split_view_eligible_windows.size() > 1u &&
+      CanSnapInSplitview(split_view_eligible_windows[1]) &&
       static_cast<ash::AppType>(split_view_eligible_windows[1]->GetProperty(
           aura::client::kAppType)) != AppType::ARC_APP;
   std::vector<SplitViewController::SnapPosition> snap_positions;
@@ -334,7 +337,7 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
     // split_view_eligible_windows[0] goes on the left.
     snap_positions.push_back(SplitViewController::LEFT);
 
-    if (prev_win_not_arc &&
+    if (prev_win_eligible &&
         wm::GetWindowState(split_view_eligible_windows[1])->GetStateType() ==
             mojom::WindowStateType::RIGHT_SNAPPED) {
       // split_view_eligible_windows[1] goes on the right.
@@ -348,7 +351,7 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
     // split_view_eligible_windows[0] goes on the right.
     snap_positions.push_back(SplitViewController::RIGHT);
 
-    if (prev_win_not_arc &&
+    if (prev_win_eligible &&
         wm::GetWindowState(split_view_eligible_windows[1])->GetStateType() ==
             mojom::WindowStateType::LEFT_SNAPPED) {
       // split_view_eligible_windows[1] goes on the left.

ash/wm/tablet_mode/tablet_mode_window_state.cc

@@ -177,12 +177,9 @@ TabletModeWindowState::TabletModeWindowState(aura::Window* window,
       animate_bounds_on_attach_(animate_bounds_on_attach) {
   wm::WindowState* state = wm::GetWindowState(window);
   current_state_type_ = state->GetStateType();
-  // The /*target_state=*/current_state_type_ part is because if |snap| is true,
-  // then we are carrying over a snapped state from desktop mode to tablet mode.
-  state_type_on_attach_ = snap
-                              ? GetSnappedWindowStateType(
-                                    state, /*target_state=*/current_state_type_)
-                              : GetMaximizedOrCenteredWindowType(state);
+  DCHECK(!snap || CanSnapInSplitview(window));
+  state_type_on_attach_ =
+      snap ? current_state_type_ : GetMaximizedOrCenteredWindowType(state);
   old_state_.reset(
       state->SetStateObject(std::unique_ptr<State>(this)).release());
 }