Do not auto snap window in clamshell split view mode.

Bug: 991406
Change-Id: I71c9d0a4ec11838cfc9d90b8cfdc78608c06426b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749647Reviewed-by: Avery Musbach <amusbach@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686571}

ash/wm/overview/overview_session_unittest.cc

@@ -4599,22 +4599,30 @@ TEST_F(SplitViewOverviewSessionInClamshellTest, BasicFunctionalitiesTest) {
             WindowStateType::kLeftSnapped);
   EXPECT_TRUE(overview_controller()->InOverviewSession());
   EXPECT_TRUE(split_view_controller()->InSplitViewMode());
+  // End overview, test that we'll not auto-snap a window to the right side of
+  // the screen.
+  EXPECT_EQ(WindowState::Get(window4.get())->GetStateType(),
+            WindowStateType::kDefault);
   ToggleOverview();
   EXPECT_EQ(WindowState::Get(window4.get())->GetStateType(),
-            WindowStateType::kRightSnapped);
+            WindowStateType::kDefault);
   EXPECT_FALSE(overview_controller()->InOverviewSession());
   EXPECT_FALSE(split_view_controller()->InSplitViewMode());
 
   // 5. Test if one window is snapped, the other window are showing in overview,
-  // activating an new window will open in splitview, which ends splitview and
-  // overview.
+  // activating an new window will not auto-snap the new window. Overview and
+  // splitview should be ended.
   ToggleOverview();
   overview_item1 = GetOverviewItemInGridWithWindow(grid_index, window1.get());
   DragWindowTo(overview_item1, gfx::PointF(0, 0));
   EXPECT_TRUE(overview_controller()->InOverviewSession());
   EXPECT_TRUE(split_view_controller()->InSplitViewMode());
   std::unique_ptr<aura::Window> window5(CreateWindow(bounds));
+  EXPECT_EQ(WindowState::Get(window5.get())->GetStateType(),
+            WindowStateType::kDefault);
   wm::ActivateWindow(window5.get());
+  EXPECT_EQ(WindowState::Get(window5.get())->GetStateType(),
+            WindowStateType::kDefault);
   EXPECT_FALSE(overview_controller()->InOverviewSession());
   EXPECT_FALSE(split_view_controller()->InSplitViewMode());
 
@@ -4633,6 +4641,22 @@ TEST_F(SplitViewOverviewSessionInClamshellTest, BasicFunctionalitiesTest) {
   EXPECT_FALSE(split_view_controller()->InSplitViewMode());
   // Overview bounds will adjust from snapped bounds to fullscreen bounds.
   EXPECT_EQ(GetGridBounds(), overview_bounds);
+
+  // 7. Test if split view mode is active, open the app list will not end
+  // overview and splitview.
+  overview_item3 = GetOverviewItemInGridWithWindow(grid_index, window3.get());
+  DragWindowTo(overview_item3, gfx::PointF(0, 0));
+  EXPECT_TRUE(overview_controller()->InOverviewSession());
+  EXPECT_TRUE(split_view_controller()->InSplitViewMode());
+  // Open app list.
+  AppListControllerImpl* app_list_controller =
+      Shell::Get()->app_list_controller();
+  app_list_controller->ToggleAppList(
+      display::Screen::GetScreen()->GetDisplayNearestWindow(window3.get()).id(),
+      app_list::AppListShowSource::kSearchKey, base::TimeTicks());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(overview_controller()->InOverviewSession());
+  EXPECT_TRUE(split_view_controller()->InSplitViewMode());
 }
 
 // Test that if app list is visible when overview is open, overview should

ash/wm/splitview/split_view_controller.cc

@@ -927,15 +927,6 @@ void SplitViewController::OnWindowActivated(ActivationReason reason,
     return;
   }
 
-  // This may be called while SnapWindow is still underway because SnapWindow
-  // will end the overview start animations which will cause the overview focus
-  // window to be activated.
-  aura::Window* overview_focus_window =
-      GetOverviewSession() ? GetOverviewSession()->GetOverviewFocusWindow()
-                           : nullptr;
-  DCHECK(InSplitViewMode() ||
-         (overview_focus_window && overview_focus_window == gained_active));
-
   // If |gained_active| was activated as a side effect of a window disposition
   // change, do nothing. For example, when a snapped window is closed, another
   // window will be activated before OnWindowDestroying() is called. We should
@@ -943,12 +934,26 @@ void SplitViewController::OnWindowActivated(ActivationReason reason,
   if (reason == ActivationReason::WINDOW_DISPOSITION_CHANGED)
     return;
 
-  // Only snap window that hasn't been snapped.
-  if (!gained_active || gained_active == left_window_ ||
-      gained_active == right_window_) {
+  // Only windows that are in the MRU list and are not already in split view can
+  // be auto-snapped.
+  if (!gained_active || IsWindowInSplitView(gained_active) ||
+      !base::Contains(
+          Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk),
+          gained_active)) {
+    return;
+  }
+
+  // We do not auto snap windows in clamshell splitview mode if a new window
+  // is activated when clamshell splitview mode is active. In this case we'll
+  // just end overview mode which will then end splitview mode.
+  // TODO(xdai): Handle this logic in OverivewSession::OnWindowActivating().
+  if (InClamshellSplitViewMode()) {
+    Shell::Get()->overview_controller()->EndOverview();
     return;
   }
 
+  DCHECK(InTabletSplitViewMode());
+
   // Do not snap the window if the activation change is caused by dragging a
   // window, or by dragging a tab. Note the two values WindowState::is_dragged()
   // and IsDraggingTabs() might not be exactly the same under certain
@@ -962,13 +967,6 @@ void SplitViewController::OnWindowActivated(ActivationReason reason,
     return;
   }
 
-  // Only windows in MRU list can be snapped.
-  if (!base::Contains(
-          Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk),
-          gained_active)) {
-    return;
-  }
-
   // If the divider is animating, then |gained_active| cannot be snapped (and is
   // not already snapped either, because then we would have bailed out by now).
   // Then if |gained_active| is user-positionable, we should end split view
@@ -1034,15 +1032,17 @@ void SplitViewController::OnOverviewModeEnding(
     return;
   }
 
+  // If we're in clamshell splitview mode, do not auto snap overview window
+  // when overview ends.
+  if (split_view_type_ == SplitViewType::kClamshellType) {
+    EndSplitView();
+    return;
+  }
+
   OverviewGrid* current_grid =
       overview_session->GetGridWithRootWindow(root_window);
-  if (!current_grid || current_grid->empty()) {
-    // If overview is ended with an empty overview grid, end split view as well
-    // if we're in clamshell splitview mode.
-    if (InClamshellSplitViewMode())
-      EndSplitView();
+  if (!current_grid || current_grid->empty())
     return;
-  }
 
   // If split view mode is active but only has one snapped window when overview
   // mode is ending, retrieve the first snappable window in the overview window