split view: Avoid stale value of is_previous_layout_right_side_up_

is_previous_layout_right_side_up_ shall be updated for clamshell/tablet
transition, even in cases where OnDisplayMetricsChanged is not called.

Fixed: 1029181
Change-Id: I9706c4273de1e632da01e019d54a825b39acd219
Bug: 1029181
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948918
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722664}

ash/wm/splitview/split_view_controller.cc

@@ -1158,6 +1158,8 @@ void SplitViewController::OnTabletModeStarting() {
 }
 
 void SplitViewController::OnTabletModeStarted() {
+  DCHECK_EQ(IsCurrentScreenOrientationPrimary(), IsLayoutRightSideUp());
+  is_previous_layout_right_side_up_ = IsCurrentScreenOrientationPrimary();
   // If splitview is active when tablet mode is starting, do the clamshell mode
   // splitview to tablet mode splitview transition by adding the split view
   // divider bar and also adjust the |divider_position_| so that it's on one of
@@ -1188,6 +1190,11 @@ void SplitViewController::OnTabletModeEnding() {
   }
 }
 
+void SplitViewController::OnTabletModeEnded() {
+  DCHECK(IsLayoutRightSideUp());
+  is_previous_layout_right_side_up_ = true;
+}
+
 void SplitViewController::OnTabletControllerDestroyed() {
   tablet_mode_observer_.RemoveAll();
 }

ash/wm/splitview/split_view_controller.h

@@ -242,6 +242,7 @@ class ASH_EXPORT SplitViewController : public aura::WindowObserver,
   void OnTabletModeStarting() override;
   void OnTabletModeStarted() override;
   void OnTabletModeEnding() override;
+  void OnTabletModeEnded() override;
   void OnTabletControllerDestroyed() override;
 
   // AccessibilityObserver:
@@ -472,6 +473,8 @@ class ASH_EXPORT SplitViewController : public aura::WindowObserver,
   SnapPosition default_snap_position_ = NONE;
 
   // Whether the previous layout is right-side-up (see |IsLayoutRightSideUp|).
+  // Consistent with |IsLayoutRightSideUp|, |is_previous_layout_right_side_up_|
+  // is always true in clamshell mode.
   bool is_previous_layout_right_side_up_ = true;
 
   // True when the divider is being dragged (not during its snap animation).

ash/wm/splitview/split_view_controller_unittest.cc

@@ -2187,6 +2187,59 @@ TEST_P(SplitViewControllerTest, DividerClosestRatioOnWorkArea) {
   EXPECT_EQ(divider_closest_ratio(), 0.33f);
 }
 
+// Tests that the divider closest position ratio is properly updated for display
+// rotation after a clamshell/tablet transition that does not trigger a call to
+// |SplitViewController::OnDisplayMetricsChanged|. The point here is that if
+// |SplitViewController::is_previous_layout_right_side_up_| is only ever updated
+// in |SplitViewController::OnDisplayMetricsChanged|, then a clamshell/tablet
+// transition can leave it with a stale value which can cause broken behavior.
+TEST_P(SplitViewControllerTest,
+       DividerClosestRatioUpdatedForClamshellTabletTransition) {
+  int64_t display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
+  display::DisplayManager* display_manager = Shell::Get()->display_manager();
+  display::test::ScopedSetInternalDisplayId set_internal(display_manager,
+                                                         display_id);
+  // Set the display orientation to landscape secondary (upside down).
+  ScreenOrientationControllerTestApi test_api(
+      Shell::Get()->screen_orientation_controller());
+  test_api.SetDisplayRotation(display::Display::ROTATE_180,
+                              display::Display::RotationSource::ACTIVE);
+  // Switch to clamshell mode.
+  TabletModeController* tablet_mode_controller =
+      Shell::Get()->tablet_mode_controller();
+  tablet_mode_controller->SetEnabledForTest(false);
+  // Set the display orientation to landscape secondary (upside down).
+  Shell::Get()->display_manager()->SetDisplayRotation(
+      display_id, display::Display::ROTATE_180,
+      display::Display::RotationSource::ACTIVE);
+  // Switch to tablet mode.
+  tablet_mode_controller->SetEnabledForTest(true);
+  // Enter split view.
+  const gfx::Rect bounds(0, 0, 200, 200);
+  std::unique_ptr<aura::Window> window(CreateWindow(bounds));
+  split_view_controller()->SnapWindow(window.get(), SplitViewController::LEFT);
+  // Drag the divider so that the snapped window spans only one third of the way
+  // across the work area.
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  const gfx::Rect divider_bounds =
+      split_view_divider()->GetDividerBoundsInScreen(false);
+  generator->set_current_screen_location(divider_bounds.CenterPoint());
+  const gfx::Rect workarea_bounds =
+      screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer(
+          window.get());
+  generator->DragMouseTo(
+      gfx::Point(workarea_bounds.width() * 0.33f, workarea_bounds.y()));
+  SkipDividerSnapAnimation();
+  // Expect that the divider closest position ratio is two thirds with the
+  // display upside down.
+  EXPECT_EQ(divider_closest_ratio(), 0.67f);
+  // Set the display orientation to landscape primary (right side up).
+  test_api.SetDisplayRotation(display::Display::ROTATE_0,
+                              display::Display::RotationSource::ACTIVE);
+  // Expect that the divider closest position ratio is updated to one third.
+  EXPECT_EQ(divider_closest_ratio(), 0.33f);
+}
+
 // Test that if we snap an always on top window in splitscreen, there should be
 // no crash and the window should stay always on top.
 TEST_P(SplitViewControllerTest, AlwaysOnTopWindow) {