tablet: Refactor carrying over snapped windows from desktop to tablet.

The present CL was originally for Issue 926602, but has become focused
on code refactoring (Issue 932207), with a low-risk fix for
Issue 926602 separated out into CL 1471013. The refactoring in the
present CL seems to have incidentally fixed the additional functional
problem shown in this video (an ugly animation):
https://photos.app.goo.gl/xGnVGQgexPgxMpRcA

Bug: 932207
Change-Id: Ia191e7d3495c09ec07237c57596de43106e7d548
Reviewed-on: https://chromium-review.googlesource.com/c/1446368
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633773}

ash/wm/tablet_mode/tablet_mode_window_manager.cc

@@ -68,7 +68,7 @@ void TabletModeWindowManager::AddWindow(aura::Window* window) {
     return;
   }
 
-  MaximizeAndTrackWindow(window);
+  TrackWindow(window);
 }
 
 void TabletModeWindowManager::WindowStateDestroyed(aura::Window* window) {
@@ -183,7 +183,7 @@ void TabletModeWindowManager::OnWindowHierarchyChanged(
       }
       return;
     }
-    MaximizeAndTrackWindow(params.target);
+    TrackWindow(params.target);
     // When the state got added, the "WM_EVENT_ADDED_TO_WORKSPACE" event got
     // already sent and we have to notify our state again.
     if (base::ContainsKey(window_state_map_, params.target)) {
@@ -234,7 +234,7 @@ void TabletModeWindowManager::OnWindowVisibilityChanged(aura::Window* window,
       base::ContainsKey(added_windows_, window) && visible) {
     added_windows_.erase(window);
     window->RemoveObserver(this);
-    MaximizeAndTrackWindow(window);
+    TrackWindow(window);
     // When the state got added, the "WM_EVENT_ADDED_TO_WORKSPACE" event got
     // already sent and we have to notify our state again.
     if (base::ContainsKey(window_state_map_, window)) {
@@ -321,56 +321,45 @@ void TabletModeWindowManager::ArrangeWindowsForTabletMode() {
       (active_window_state_type != mojom::WindowStateType::LEFT_SNAPPED &&
        active_window_state_type != mojom::WindowStateType::RIGHT_SNAPPED)) {
     for (auto* window : windows)
-      MaximizeAndTrackWindow(window);
+      TrackWindow(window);
     return;
   }
 
-  // The snapped active window will be represented by split view, which will be
-  // activated after maximizing all windows. The split view layout is decided
-  // here by examining window states before all those states become maximized.
+  // Carry over the snapped active window to split view, along with the previous
+  // window if it exists, is snapped on the opposite side, and is not ARC.
   const bool prev_win_not_arc =
       windows.size() > 1u && static_cast<ash::AppType>(windows[1]->GetProperty(
                                  aura::client::kAppType)) != AppType::ARC_APP;
-  SplitViewController::SnapPosition curr_win_snap_pos =
-      SplitViewController::NONE;
-  SplitViewController::SnapPosition prev_win_snap_pos =
-      SplitViewController::NONE;
+  std::vector<SplitViewController::SnapPosition> snap_positions;
   if (active_window_state_type == mojom::WindowStateType::LEFT_SNAPPED) {
     // The active window snapped on the left shall go there in split view.
-    curr_win_snap_pos = SplitViewController::LEFT;
+    snap_positions.push_back(SplitViewController::LEFT);
 
     if (prev_win_not_arc && wm::GetWindowState(windows[1])->GetStateType() ==
                                 mojom::WindowStateType::RIGHT_SNAPPED) {
       // The previous window snapped on the right shall go there in split view.
-      prev_win_snap_pos = SplitViewController::RIGHT;
+      snap_positions.push_back(SplitViewController::RIGHT);
     }
   } else {
     DCHECK_EQ(mojom::WindowStateType::RIGHT_SNAPPED, active_window_state_type);
 
     // The active window snapped on the right shall go there in split view.
-    curr_win_snap_pos = SplitViewController::RIGHT;
+    snap_positions.push_back(SplitViewController::RIGHT);
 
     if (prev_win_not_arc && wm::GetWindowState(windows[1])->GetStateType() ==
                                 mojom::WindowStateType::LEFT_SNAPPED) {
       // The previous window snapped on the left shall go there in split view.
-      prev_win_snap_pos = SplitViewController::LEFT;
+      snap_positions.push_back(SplitViewController::LEFT);
     }
   }
-
-  // Use |defer_bounds_update| to suppress the maximizing animation which would
-  // look weird here, especially when overview appears beside the active window.
-  for (auto* window : windows)
-    MaximizeAndTrackWindow(window, /*defer_bounds_update=*/true);
-
-  // Implement the previously decided split view layout.
   SplitViewController* split_view_controller =
       Shell::Get()->split_view_controller();
-  split_view_controller->SnapWindow(windows[0], curr_win_snap_pos);
-  if (prev_win_snap_pos != SplitViewController::NONE)
-    split_view_controller->SnapWindow(windows[1], prev_win_snap_pos);
-
-  for (auto* window : windows)
-    SetDeferBoundsUpdates(window, false);
+  for (size_t i = 0u; i < windows.size(); ++i) {
+    const bool snap = i < snap_positions.size();
+    TrackWindow(windows[i], snap, /*animate_bounds_on_attach=*/false);
+    if (snap)
+      split_view_controller->SnapWindow(windows[i], snap_positions[i]);
+  }
 }
 
 void TabletModeWindowManager::ArrangeWindowsForDesktopMode() {
@@ -385,9 +374,9 @@ void TabletModeWindowManager::SetDeferBoundsUpdates(aura::Window* window,
     iter->second->SetDeferBoundsUpdates(defer_bounds_updates);
 }
 
-void TabletModeWindowManager::MaximizeAndTrackWindow(
-    aura::Window* window,
-    bool defer_bounds_updates) {
+void TabletModeWindowManager::TrackWindow(aura::Window* window,
+                                          bool snap,
+                                          bool animate_bounds_on_attach) {
   if (!ShouldHandleWindow(window))
     return;
 
@@ -397,7 +386,7 @@ void TabletModeWindowManager::MaximizeAndTrackWindow(
   // We create and remember a tablet mode state which will attach itself to
   // the provided state object.
   window_state_map_[window] =
-      new TabletModeWindowState(window, this, defer_bounds_updates);
+      new TabletModeWindowState(window, this, snap, animate_bounds_on_attach);
 }
 
 void TabletModeWindowManager::ForgetWindow(aura::Window* window,

ash/wm/tablet_mode/tablet_mode_window_manager.h

@@ -113,13 +113,13 @@ class ASH_EXPORT TabletModeWindowManager
   // will be updated as they may be stale.
   void SetDeferBoundsUpdates(aura::Window* window, bool defer_bounds_updates);
 
-  // If the given window should be handled by us, this function will maximize it
-  // and add it to the list of known windows (remembering the initial show
-  // state).
+  // If the given window should be handled by us, this function will add it to
+  // the list of known windows (remembering the initial show state).
   // Note: If the given window cannot be handled by us the function will return
   // immediately.
-  void MaximizeAndTrackWindow(aura::Window* window,
-                              bool defer_bounds_updates = false);
+  void TrackWindow(aura::Window* window,
+                   bool snap = false,
+                   bool animate_bounds_on_attach = true);
 
   // Remove a window from our tracking list. If the window is going to be
   // destroyed, do not restore its old previous window state object as it will

ash/wm/tablet_mode/tablet_mode_window_state.cc

@@ -168,14 +168,21 @@ void TabletModeWindowState::UpdateWindowPosition(wm::WindowState* window_state,
 
 TabletModeWindowState::TabletModeWindowState(aura::Window* window,
                                              TabletModeWindowManager* creator,
-                                             bool defer_bounds_updates)
+                                             bool snap,
+                                             bool animate_bounds_on_attach)
     : window_(window),
       creator_(creator),
-      current_state_type_(wm::GetWindowState(window)->GetStateType()),
-      defer_bounds_updates_(defer_bounds_updates) {
-  old_state_.reset(wm::GetWindowState(window)
-                       ->SetStateObject(std::unique_ptr<State>(this))
-                       .release());
+      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);
+  old_state_.reset(
+      state->SetStateObject(std::unique_ptr<State>(this)).release());
 }
 
 TabletModeWindowState::~TabletModeWindowState() {
@@ -341,14 +348,13 @@ void TabletModeWindowState::AttachState(
                               window_state->GetShowState());
   }
 
-  // Initialize the state to a good preset.
   if (current_state_type_ != mojom::WindowStateType::MAXIMIZED &&
       current_state_type_ != mojom::WindowStateType::MINIMIZED &&
       current_state_type_ != mojom::WindowStateType::FULLSCREEN &&
       current_state_type_ != mojom::WindowStateType::PINNED &&
       current_state_type_ != mojom::WindowStateType::TRUSTED_PINNED) {
-    UpdateWindow(window_state, GetMaximizedOrCenteredWindowType(window_state),
-                 true /* animated */);
+    UpdateWindow(window_state, state_type_on_attach_,
+                 animate_bounds_on_attach_);
   }
 }
 

ash/wm/tablet_mode/tablet_mode_window_state.h

@@ -23,12 +23,18 @@ class TabletModeWindowState : public wm::WindowState::State {
   static void UpdateWindowPosition(wm::WindowState* window_state, bool animate);
 
   // The |window|'s state object will be modified to use this new window mode
-  // state handler. Upon destruction it will restore the previous state handler
-  // and call |creator::WindowStateDestroyed()| to inform that the window mode
-  // was reverted to the old window manager.
+  // state handler. |snap| is for carrying over a snapped state from clamshell
+  // mode to tablet mode. If |snap| is false, then the window will be maximized,
+  // unless the original state was MAXIMIZED, MINIMIZED, FULLSCREEN, PINNED, or
+  // TRUSTED_PINNED. Use |animate_bounds_on_attach| to specify whether to
+  // animate the corresponding bounds update. Call LeaveTabletMode() to restore
+  // the previous state handler, whereupon ~TabletModeWindowState() will call
+  // |creator::WindowStateDestroyed()| to inform that the window mode was
+  // reverted to the old window manager.
   TabletModeWindowState(aura::Window* window,
                         TabletModeWindowManager* creator,
-                        bool defer_bounds_updates);
+                        bool snap,
+                        bool animate_bounds_on_attach);
   ~TabletModeWindowState() override;
 
   void set_ignore_wm_events(bool ignore) { ignore_wm_events_ = ignore; }
@@ -90,6 +96,15 @@ class TabletModeWindowState : public wm::WindowState::State {
   // The creator which needs to be informed when this state goes away.
   TabletModeWindowManager* creator_;
 
+  // The state type to be established in AttachState(), unless
+  // previous_state->GetType() is MAXIMIZED, MINIMIZED, FULLSCREEN, PINNED, or
+  // TRUSTED_PINNED.
+  mojom::WindowStateType state_type_on_attach_;
+
+  // Whether to animate in case of a bounds update when switching to
+  // |state_type_on_attach_|.
+  bool animate_bounds_on_attach_;
+
   // The current state type. Due to the nature of this state, this can only be
   // WM_STATE_TYPE{NORMAL, MINIMIZED, MAXIMIZED}.
   mojom::WindowStateType current_state_type_;