snap-groups: Fix crash after transition to tablet

SplitViewOverviewSession is created in clamshell mode, which expects
the window to start resizing and be dragged before we call
`CHECK(presentation_time_recorder_)`. This will:
1. `CHECK(presentation_time_recorder_)` only if we started a resize
2. Bail out if we're not in clamshell mode
since SplitViewOverviewSession will be removed in tablet mode anyway
(b/307631336).

Bug: b/308216746
Test: added, verified it fails in https://crrev.com/c/4986692
Change-Id: I486bb4d6f7c67eca58bc7401faf19cf3c2de0018
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4986547
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Sophie Wen <sophiewen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216539}

ash/wm/snap_group/snap_group_unittest.cc

@@ -74,6 +74,7 @@
 #include "ui/base/hit_test.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/layer.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/display/test/display_manager_test_api.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/keycodes/keyboard_codes_posix.h"
@@ -481,6 +482,39 @@ TEST_F(FasterSplitScreenTest, MultiDisplay) {
   base::RunLoop().RunUntilIdle();
 }
 
+// Verifies that there will be no crash when transitioning the
+// `SplitViewOverviewSession` between clamshell and tablet mode.
+TEST_F(FasterSplitScreenTest, ClamshellTabletTransition) {
+  std::unique_ptr<aura::Window> w1(CreateTestWindow());
+  SnapOneTestWindow(w1.get(), chromeos::WindowStateType::kPrimarySnapped);
+  VerifySplitViewOverviewSession(w1.get());
+
+  SwitchToTabletMode();
+  TabletModeControllerTestApi().LeaveTabletMode();
+}
+
+// Tests that double tap to swap windows doesn't crash after transition to
+// tablet mode (b/308216746).
+TEST_F(FasterSplitScreenTest, NoCrashWhenDoubleTapAfterTransition) {
+  // Use non-zero to start an animation, which will notify
+  // `SplitViewOverviewSession::OnWindowBoundsChanged()`.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+  std::unique_ptr<aura::Window> w1(CreateAppWindow());
+  SnapOneTestWindow(w1.get(), chromeos::WindowStateType::kPrimarySnapped);
+  SwitchToTabletMode();
+  EXPECT_TRUE(split_view_divider());
+
+  // Double tap on the divider. This will start a drag and notify
+  // SplitViewOverviewSession.
+  const gfx::Point divider_center =
+      split_view_divider()
+          ->GetDividerBoundsInScreen(/*is_dragging=*/false)
+          .CenterPoint();
+  GetEventGenerator()->GestureTapAt(divider_center);
+  GetEventGenerator()->GestureTapAt(divider_center);
+}
+
 // Tests the histograms for the split view overview session exit points are
 // recorded correctly in clamshell.
 TEST_F(FasterSplitScreenTest,

ash/wm/splitview/split_view_controller.cc

@@ -1247,7 +1247,6 @@ void SplitViewController::InitDividerPositionForTransition(
     int divider_position) {
   // This should only be called before the actual carry-over happens.
   DCHECK(!InSplitViewMode());
-  DCHECK_EQ(divider_position_, -1);
   divider_position_ = divider_position;
 }
 

ash/wm/splitview/split_view_overview_session.cc

@@ -43,13 +43,20 @@ constexpr char kClamshellSplitViewResizeWithOverviewMaxLatencyHistogram[] =
     "Ash.SplitViewResize.PresentationTime.MaxLatency.ClamshellMode."
     "WithOverview";
 
+// Normally if we are not in clamshell or overview has ended,
+// SplitViewOverviewSession would have been ended, however this can be notified
+// during mid-drag or mid-resize, so bail out here.
+// TODO(b/307631336): Eventually this will be removed in tablet mode.
 bool InClamshellSplitViewMode(SplitViewController* controller) {
   // If `kFasterSplitScreenSetup` is enabled, clamshell split view does *not*
   // have to be active.
   // TODO(sophiewen): Consolidate with `kSnapGroup` flag.
-  return (features::IsFasterSplitScreenSetupEnabled() ||
-          (controller && controller->InClamshellSplitViewMode())) &&
-         GetOverviewSession();
+  if (features::IsFasterSplitScreenSetupEnabled()) {
+    return chromeos::TabletState::Get()->state() ==
+           display::TabletState::kInClamshellMode;
+  }
+  return controller && controller->InClamshellSplitViewMode() &&
+         IsInOverviewSession();
 }
 
 }  // namespace
@@ -146,6 +153,7 @@ void SplitViewOverviewSession::OnResizeLoopStarted(aura::Window* window) {
     return;
   }
 
+  is_resizing_ = true;
   if (IsSnapGroupEnabledInClamshellMode() &&
       split_view_controller->state() ==
           SplitViewController::State::kBothSnapped) {
@@ -188,6 +196,7 @@ void SplitViewOverviewSession::OnResizeLoopEnded(aura::Window* window) {
   if (!window_util::IsFasterSplitScreenOrSnapGroupArm1Enabled()) {
     split_view_controller->MaybeEndOverviewOnWindowResize(window);
   }
+  is_resizing_ = false;
 }
 
 void SplitViewOverviewSession::OnWindowBoundsChanged(
@@ -221,6 +230,9 @@ void SplitViewOverviewSession::OnWindowBoundsChanged(
           OverviewEndAction::kSplitView);
       return;
     }
+    if (!is_resizing_) {
+      return;
+    }
     CHECK(window_state->drag_details()->bounds_change &
           WindowResizer::kBoundsChange_Resizes);
     CHECK(presentation_time_recorder_);

ash/wm/splitview/split_view_overview_session.h

@@ -98,6 +98,9 @@ class ASH_EXPORT SplitViewOverviewSession : public aura::WindowObserver,
   }
 
  private:
+  // True while we are processing a window resize event.
+  bool is_resizing_ = false;
+
   // Records the presentation time of resize operation in clamshell split view
   // mode.
   std::unique_ptr<ui::PresentationTimeRecorder> presentation_time_recorder_;