tablet: Make the functionality of CL 1497477 work again

1. Snap the active window in clamshell mode
2. Switch to another user session
3. Switch to tablet mode
4. Switch back to the first user session

The snapped window should be carried over to tablet split view.

Test: manual
Change-Id: I830114212b595dc44c6513e9072141cd8282dadc
Fixed: 936610
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2021187
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735485}

ash/wm/splitview/split_view_controller.cc

@@ -470,10 +470,7 @@ void SplitViewController::SnapWindow(aura::Window* window,
         ->OnSelectorItemDragEnded(/*snap=*/true);
   }
 
-  // Update the divider position and window bounds before snapping a new
-  // window. Since the minimum size of |window| maybe larger than currently
-  // bounds in |snap_position|.
-  if (InTabletSplitViewMode()) {
+  if (split_view_type_ == SplitViewType::kTabletType) {
     divider_position_ = GetClosestFixedDividerPosition();
     UpdateSnappedWindowsAndDividerBounds();
   }
@@ -1389,8 +1386,6 @@ void SplitViewController::UpdateBlackScrim(
 }
 
 void SplitViewController::UpdateSnappedWindowsAndDividerBounds() {
-  DCHECK(InSplitViewMode());
-
   // Update the snapped windows' bounds.
   if (IsSnapped(left_window_)) {
     const WMEvent left_window_event(WM_EVENT_SNAP_LEFT);
@@ -1468,8 +1463,6 @@ void SplitViewController::UpdateDividerPosition(
 }
 
 int SplitViewController::GetClosestFixedDividerPosition() {
-  DCHECK(InSplitViewMode());
-
   // The values in |kFixedPositionRatios| represent the fixed position of the
   // center of the divider while |divider_position_| represent the origin of the
   // divider rectangle. So, before calling FindClosestFixedPositionRatio,

ash/wm/tablet_mode/tablet_mode_window_manager.cc

@@ -53,90 +53,6 @@ bool IsCarryOverCandidateForSplitView(
          SplitViewController::Get(root_window)->CanSnapWindow(windows[i]);
 }
 
-// Returns the windows that are going to be carried over to splitview during
-// clamshell <-> tablet transition or multi user switch transition.
-// TODO(xdai): Return eligible windows regardless of window zorders.
-base::flat_map<aura::Window*, WindowStateType>
-GetCarryOverWindowsInSplitView() {
-  base::flat_map<aura::Window*, WindowStateType> windows;
-  // Check the states of the topmost two non-overview windows to see if they are
-  // eligible to be carried over to splitscreen. A window must meet
-  // IsCarryOverCandidateForSplitView() to be carried over to splitscreen.
-  MruWindowTracker::WindowList mru_windows =
-      Shell::Get()->mru_window_tracker()->BuildWindowForCycleList(kActiveDesk);
-  mru_windows.erase(
-      std::remove_if(mru_windows.begin(), mru_windows.end(),
-                     [](aura::Window* window) {
-                       return window->GetProperty(kIsShowingInOverviewKey);
-                     }),
-      mru_windows.end());
-  aura::Window* root_window = Shell::GetPrimaryRootWindow();
-  if (IsCarryOverCandidateForSplitView(mru_windows, 0u, root_window)) {
-    if (WindowState::Get(mru_windows[0])->GetStateType() ==
-        WindowStateType::kLeftSnapped) {
-      windows.emplace(mru_windows[0], WindowStateType::kLeftSnapped);
-      if (IsCarryOverCandidateForSplitView(mru_windows, 1u, root_window) &&
-          WindowState::Get(mru_windows[1])->GetStateType() ==
-              WindowStateType::kRightSnapped) {
-        windows.emplace(mru_windows[1], WindowStateType::kRightSnapped);
-      }
-    } else if (WindowState::Get(mru_windows[0])->GetStateType() ==
-               WindowStateType::kRightSnapped) {
-      windows.emplace(mru_windows[0], WindowStateType::kRightSnapped);
-      if (IsCarryOverCandidateForSplitView(mru_windows, 1u, root_window) &&
-          WindowState::Get(mru_windows[1])->GetStateType() ==
-              WindowStateType::kLeftSnapped) {
-        windows.emplace(mru_windows[1], WindowStateType::kLeftSnapped);
-      }
-    }
-  }
-  return windows;
-}
-
-// Calculates the divider position of the splitscreen based on the snapped
-// window(s)'s positions. We'll try to keep the current snapped window(s)'
-// bounds as much as possible.
-int CalculateCarryOverDividerPostion(
-    base::flat_map<aura::Window*, WindowStateType> windows_in_splitview) {
-  aura::Window* left_window = nullptr;
-  aura::Window* right_window = nullptr;
-  for (auto& iter : windows_in_splitview) {
-    if (iter.second == WindowStateType::kLeftSnapped)
-      left_window = iter.first;
-    else if (iter.second == WindowStateType::kRightSnapped)
-      right_window = iter.first;
-  }
-  if (!left_window && !right_window)
-    return -1;
-
-  gfx::Rect work_area =
-      display::Screen::GetScreen()
-          ->GetDisplayNearestWindow(left_window ? left_window : right_window)
-          .work_area();
-  gfx::Rect left_window_bounds =
-      left_window ? left_window->GetBoundsInScreen() : gfx::Rect();
-  gfx::Rect right_window_bounds =
-      right_window ? right_window->GetBoundsInScreen() : gfx::Rect();
-
-  if (SplitViewController::IsLayoutHorizontal()) {
-    if (SplitViewController::IsLayoutRightSideUp()) {
-      return left_window ? left_window_bounds.width()
-                         : work_area.width() - right_window_bounds.width();
-    } else {
-      return left_window ? work_area.width() - left_window_bounds.width()
-                         : right_window_bounds.width();
-    }
-  } else {
-    if (SplitViewController::IsLayoutRightSideUp()) {
-      return left_window ? left_window_bounds.height()
-                         : work_area.height() - right_window_bounds.height();
-    } else {
-      return left_window ? work_area.height() - left_window_bounds.height()
-                         : right_window_bounds.height();
-    }
-  }
-}
-
 // Snap the carry over windows into splitview mode at |divider_position|.
 void DoSplitViewTransition(
     base::flat_map<aura::Window*, WindowStateType> windows,
@@ -319,7 +235,8 @@ void TabletModeWindowManager::Shutdown() {
 
     // TODO(xdai): Instead of caching snapped windows and their state here, we
     // should try to see if it can be done in the WindowState::State impl.
-    carryover_windows_in_splitview = GetCarryOverWindowsInSplitView();
+    carryover_windows_in_splitview =
+        GetCarryOverWindowsInSplitView(/*clamshell_to_tablet=*/false);
 
     // For case 2 and 3: End splitview mode for two snapped windows case or
     // single split case to match the clamshell split view behavior. (there is
@@ -581,9 +498,9 @@ void TabletModeWindowManager::OnActiveUserSessionChanged(
   bool refresh_snapped_windows = false;
   if (accounts_since_entering_tablet_.count(account_id) == 0u) {
     base::flat_map<aura::Window*, WindowStateType> windows_in_splitview =
-        GetCarryOverWindowsInSplitView();
-    int divider_position =
-        CalculateCarryOverDividerPostion(windows_in_splitview);
+        GetCarryOverWindowsInSplitView(/*clamshell_to_tablet=*/true);
+    const int divider_position = CalculateCarryOverDividerPosition(
+        windows_in_splitview, /*clamshell_to_tablet=*/true);
     DoSplitViewTransition(windows_in_splitview, divider_position);
     accounts_since_entering_tablet_.insert(account_id);
   } else {
@@ -593,14 +510,106 @@ void TabletModeWindowManager::OnActiveUserSessionChanged(
   MaybeRestoreSplitView(refresh_snapped_windows);
 }
 
-WindowStateType TabletModeWindowManager::GetDesktopWindowStateType(
-    aura::Window* window) const {
+gfx::Rect TabletModeWindowManager::GetWindowBoundsInScreen(
+    aura::Window* window,
+    bool from_clamshell) const {
   auto iter = window_state_map_.find(window);
-  return iter == window_state_map_.end()
+  return !from_clamshell || iter == window_state_map_.end()
+             ? window->GetBoundsInScreen()
+             : iter->second->old_window_bounds_in_screen();
+}
+
+WindowStateType TabletModeWindowManager::GetWindowStateType(
+    aura::Window* window,
+    bool from_clamshell) const {
+  auto iter = window_state_map_.find(window);
+  return !from_clamshell || iter == window_state_map_.end()
              ? WindowState::Get(window)->GetStateType()
              : iter->second->old_state()->GetType();
 }
 
+base::flat_map<aura::Window*, WindowStateType>
+TabletModeWindowManager::GetCarryOverWindowsInSplitView(
+    bool clamshell_to_tablet) const {
+  base::flat_map<aura::Window*, WindowStateType> windows;
+  // Check the states of the topmost two non-overview windows to see if they are
+  // eligible to be carried over to splitscreen. A window must meet
+  // IsCarryOverCandidateForSplitView() to be carried over to splitscreen.
+  MruWindowTracker::WindowList mru_windows =
+      Shell::Get()->mru_window_tracker()->BuildWindowForCycleList(kActiveDesk);
+  mru_windows.erase(
+      std::remove_if(mru_windows.begin(), mru_windows.end(),
+                     [](aura::Window* window) {
+                       return window->GetProperty(kIsShowingInOverviewKey);
+                     }),
+      mru_windows.end());
+  aura::Window* root_window = Shell::GetPrimaryRootWindow();
+  if (IsCarryOverCandidateForSplitView(mru_windows, 0u, root_window)) {
+    if (GetWindowStateType(mru_windows[0], clamshell_to_tablet) ==
+        WindowStateType::kLeftSnapped) {
+      windows.emplace(mru_windows[0], WindowStateType::kLeftSnapped);
+      if (IsCarryOverCandidateForSplitView(mru_windows, 1u, root_window) &&
+          GetWindowStateType(mru_windows[1], clamshell_to_tablet) ==
+              WindowStateType::kRightSnapped) {
+        windows.emplace(mru_windows[1], WindowStateType::kRightSnapped);
+      }
+    } else if (GetWindowStateType(mru_windows[0], clamshell_to_tablet) ==
+               WindowStateType::kRightSnapped) {
+      windows.emplace(mru_windows[0], WindowStateType::kRightSnapped);
+      if (IsCarryOverCandidateForSplitView(mru_windows, 1u, root_window) &&
+          GetWindowStateType(mru_windows[1], clamshell_to_tablet) ==
+              WindowStateType::kLeftSnapped) {
+        windows.emplace(mru_windows[1], WindowStateType::kLeftSnapped);
+      }
+    }
+  }
+  return windows;
+}
+
+int TabletModeWindowManager::CalculateCarryOverDividerPosition(
+    const base::flat_map<aura::Window*, WindowStateType>& windows_in_splitview,
+    bool clamshell_to_tablet) const {
+  aura::Window* left_window = nullptr;
+  aura::Window* right_window = nullptr;
+  for (auto& iter : windows_in_splitview) {
+    if (iter.second == WindowStateType::kLeftSnapped)
+      left_window = iter.first;
+    else if (iter.second == WindowStateType::kRightSnapped)
+      right_window = iter.first;
+  }
+  if (!left_window && !right_window)
+    return -1;
+
+  gfx::Rect work_area =
+      display::Screen::GetScreen()
+          ->GetDisplayNearestWindow(left_window ? left_window : right_window)
+          .work_area();
+  gfx::Rect left_window_bounds =
+      left_window ? GetWindowBoundsInScreen(left_window, clamshell_to_tablet)
+                  : gfx::Rect();
+  gfx::Rect right_window_bounds =
+      right_window ? GetWindowBoundsInScreen(right_window, clamshell_to_tablet)
+                   : gfx::Rect();
+
+  if (SplitViewController::IsLayoutHorizontal()) {
+    if (SplitViewController::IsLayoutRightSideUp()) {
+      return left_window ? left_window_bounds.width()
+                         : work_area.width() - right_window_bounds.width();
+    } else {
+      return left_window ? work_area.width() - left_window_bounds.width()
+                         : right_window_bounds.width();
+    }
+  } else {
+    if (SplitViewController::IsLayoutRightSideUp()) {
+      return left_window ? left_window_bounds.height()
+                         : work_area.height() - right_window_bounds.height();
+    } else {
+      return left_window ? work_area.height() - left_window_bounds.height()
+                         : right_window_bounds.height();
+    }
+  }
+}
+
 void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
   // If clamshell split view mode is not enabled, still keep the old behavior:
   // end overview if it's active. And carry over snapped windows to
@@ -632,8 +641,9 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
   // Determine which windows are to be carried over to splitview from clamshell
   // mode to tablet mode.
   base::flat_map<aura::Window*, WindowStateType> windows_in_splitview =
-      GetCarryOverWindowsInSplitView();
-  int divider_position = CalculateCarryOverDividerPostion(windows_in_splitview);
+      GetCarryOverWindowsInSplitView(/*clamshell_to_tablet=*/true);
+  const int divider_position = CalculateCarryOverDividerPosition(
+      windows_in_splitview, /*clamshell_to_tablet=*/true);
 
   // If split view is not appropriate, then maximize all windows and bail out.
   if (windows_in_splitview.empty()) {
@@ -663,7 +673,8 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
 void TabletModeWindowManager::ArrangeWindowsForClamshellMode(
     base::flat_map<aura::Window*, WindowStateType> windows_in_splitview,
     bool was_in_overview) {
-  int divider_position = CalculateCarryOverDividerPostion(windows_in_splitview);
+  const int divider_position = CalculateCarryOverDividerPosition(
+      windows_in_splitview, /*clamshell_to_tablet=*/false);
 
   while (window_state_map_.size()) {
     aura::Window* window = window_state_map_.begin()->first;

ash/wm/tablet_mode/tablet_mode_window_manager.h

@@ -113,9 +113,25 @@ class ASH_EXPORT TabletModeWindowManager : public aura::WindowObserver,
  private:
   using WindowToState = std::map<aura::Window*, TabletModeWindowState*>;
 
-  // Returns the state type that |window| had before tablet mode started. If
-  // |window| is not yet tracked, returns the current state type of |window|.
-  WindowStateType GetDesktopWindowStateType(aura::Window* window) const;
+  // If |from_clamshell| is true, returns the bounds or state type that |window|
+  // had before tablet mode started. If |from_clamshell| is false, returns the
+  // current bounds or state type of |window|.
+  gfx::Rect GetWindowBoundsInScreen(aura::Window* window,
+                                    bool from_clamshell) const;
+  WindowStateType GetWindowStateType(aura::Window* window,
+                                     bool from_clamshell) const;
+
+  // Returns the windows that are going to be carried over to split view during
+  // clamshell <-> tablet transition or multi-user switch transition.
+  base::flat_map<aura::Window*, WindowStateType> GetCarryOverWindowsInSplitView(
+      bool clamshell_to_tablet) const;
+
+  // Calculates the split view divider position that will best preserve the
+  // bounds of the windows.
+  int CalculateCarryOverDividerPosition(
+      const base::flat_map<aura::Window*, WindowStateType>&
+          windows_in_splitview,
+      bool clamshell_to_tablet) const;
 
   // Maximizes all windows, except that snapped windows shall carry over to
   // split view as determined by GetCarryOverWindowsInSplitView().

ash/wm/tablet_mode/tablet_mode_window_state.cc

@@ -196,6 +196,7 @@ TabletModeWindowState::TabletModeWindowState(aura::Window* window,
       window, entering_tablet_mode && !IsTopWindow(window)
                   ? WindowState::BoundsChangeAnimationType::STEP_END
                   : WindowState::BoundsChangeAnimationType::DEFAULT);
+  old_window_bounds_in_screen_ = window->GetBoundsInScreen();
   old_state_.reset(
       state->SetStateObject(std::unique_ptr<State>(this)).release());
 }

ash/wm/tablet_mode/tablet_mode_window_state.h

@@ -9,6 +9,7 @@
 
 #include "ash/wm/window_state.h"
 #include "base/macros.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace ash {
 class TabletModeWindowManager;
@@ -51,6 +52,9 @@ class TabletModeWindowState : public WindowState::State {
                    WindowState::State* previous_state) override;
   void DetachState(WindowState* window_state) override;
 
+  gfx::Rect old_window_bounds_in_screen() const {
+    return old_window_bounds_in_screen_;
+  }
   WindowState::State* old_state() { return old_state_.get(); }
 
  private:
@@ -77,7 +81,8 @@ class TabletModeWindowState : public WindowState::State {
   // window state. If |animated| is set we animate the change.
   void UpdateBounds(WindowState* window_state, bool animated);
 
-  // The original state object of the window.
+  // The original bounds and state object of the window.
+  gfx::Rect old_window_bounds_in_screen_;
   std::unique_ptr<WindowState::State> old_state_;
 
   // The window whose WindowState owns this instance.