Fix crash when using title drag on a window opened while split view is active.

This CL also centralizes setting the activity's bounds and transform while in
split view to SplitViewController::UpdateLayout(). In particular, the bounds of
an activity are now correct even if it is opened while some other activity is
animating into split view

BUG=405964
TEST=WindowManagerTest.NewWindowBounds

Review URL: https://codereview.chromium.org/513313003

Cr-Commit-Position: refs/heads/master@{#293019}

4 files changed, 81 insertions, 73 deletions

diff --git a/athena/wm/split_view_controller.cc b/athena/wm/split_view_controller.cc
index e36b7a9..3c019d4 100644
--- a/athena/wm/split_view_controller.cc
+++ b/athena/wm/split_view_controller.cc
@@ -112,19 +112,20 @@ void SplitViewController::ReplaceWindow(aura::Window* window,
          windows.end());
 #endif
 
-  replace_with->SetBounds(window->bounds());
-  replace_with->SetTransform(gfx::Transform());
   if (window == left_window_)
     left_window_ = replace_with;
   else
     right_window_ = replace_with;
   wm::ActivateWindow(replace_with);
+  UpdateLayout(false);
   window->SetTransform(gfx::Transform());
+  window->Hide();
 }
 
 void SplitViewController::DeactivateSplitMode() {
-  CHECK_NE(SCROLLING, state_);
+  CHECK_EQ(ACTIVE, state_);
   state_ = INACTIVE;
+  UpdateLayout(false);
   left_window_ = right_window_ = NULL;
 }
 
@@ -143,76 +144,80 @@ gfx::Rect SplitViewController::GetRightTargetBounds() {
 }
 
 void SplitViewController::UpdateLayout(bool animate) {
-  if  (!left_window_)
-    return;
+  CHECK(left_window_);
   CHECK(right_window_);
-  gfx::Transform left_transform;
-  gfx::Transform right_transform;
-  int container_width = container_->GetBoundsInScreen().width();
+
+  if (state_ == INACTIVE && !animate) {
+    if (!wm::IsActiveWindow(left_window_))
+      left_window_->Hide();
+    if (!wm::IsActiveWindow(right_window_))
+      right_window_->Hide();
+    SetWindowTransforms(gfx::Transform(), gfx::Transform(), false);
+    return;
+  }
+
+  left_window_->Show();
+  right_window_->Show();
   if (state_ == ACTIVE) {
-    // Windows should be resized via an animation when entering the ACTIVE
-    // state.
-    CHECK(animate);
-    // We scale the windows here, but when the animation finishes, we reset
-    // the scaling and update the window bounds to the proper size - see
-    // OnAnimationCompleted().
-    left_transform = GetTargetTransformForBoundsAnimation(
-        left_window_->bounds(), GetLeftTargetBounds());
-    right_transform = GetTargetTransformForBoundsAnimation(
-        right_window_->bounds(), GetRightTargetBounds());
+    if (animate) {
+      gfx::Transform left_transform = GetTargetTransformForBoundsAnimation(
+          left_window_->bounds(), GetLeftTargetBounds());
+      gfx::Transform right_transform = GetTargetTransformForBoundsAnimation(
+          right_window_->bounds(), GetRightTargetBounds());
+      SetWindowTransforms(left_transform, right_transform, true);
+    } else {
+      left_window_->SetBounds(GetLeftTargetBounds());
+      right_window_->SetBounds(GetRightTargetBounds());
+      SetWindowTransforms(gfx::Transform(), gfx::Transform(), false);
+    }
   } else {
-    left_transform.Translate(separator_position_ - container_width, 0);
+    gfx::Transform left_transform;
+    left_transform.Translate(separator_position_ - container_->bounds().width(),
+                             0);
+    gfx::Transform right_transform;
     right_transform.Translate(separator_position_, 0);
+    SetWindowTransforms(left_transform, right_transform, animate);
   }
-  left_window_->Show();
-  right_window_->Show();
-  SetWindowTransform(left_window_, left_transform, animate);
-  SetWindowTransform(right_window_, right_transform, animate);
+  // Note: |left_window_| and |right_window_| may be NULL if calling
+  // SetWindowTransforms():
+  // - caused the in-progress animation to abort.
+  // - started a zero duration animation.
 }
 
-void SplitViewController::SetWindowTransform(aura::Window* window,
-                                             const gfx::Transform& transform,
-                                             bool animate) {
+void SplitViewController::SetWindowTransforms(
+    const gfx::Transform& left_transform,
+    const gfx::Transform& right_transform,
+    bool animate) {
   if (animate) {
-    scoped_refptr<ui::LayerAnimator> animator = window->layer()->GetAnimator();
-    ui::ScopedLayerAnimationSettings settings(animator);
-    settings.SetPreemptionStrategy(
+    ui::ScopedLayerAnimationSettings left_settings(
+        left_window_->layer()->GetAnimator());
+    left_settings.SetPreemptionStrategy(
         ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-    settings.AddObserver(new ui::ClosureAnimationObserver(
+    left_window_->SetTransform(left_transform);
+
+    ui::ScopedLayerAnimationSettings right_settings(
+        right_window_->layer()->GetAnimator());
+    right_settings.SetPreemptionStrategy(
+        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+    right_settings.AddObserver(new ui::ClosureAnimationObserver(
         base::Bind(&SplitViewController::OnAnimationCompleted,
-                   weak_factory_.GetWeakPtr(),
-                   window)));
-    window->SetTransform(transform);
+                   weak_factory_.GetWeakPtr())));
+    right_window_->SetTransform(right_transform);
   } else {
-    window->SetTransform(transform);
+    left_window_->SetTransform(left_transform);
+    right_window_->SetTransform(right_transform);
   }
 }
 
-void SplitViewController::OnAnimationCompleted(aura::Window* window) {
+void SplitViewController::OnAnimationCompleted() {
   // Animation can be cancelled when deactivated.
   if (left_window_ == NULL)
     return;
-  DCHECK(window == left_window_ || window == right_window_);
-  if (state_ == ACTIVE) {
-    window->SetTransform(gfx::Transform());
-    if (window == left_window_)
-      left_window_->SetBounds(GetLeftTargetBounds());
-    else
-      right_window_->SetBounds(GetRightTargetBounds());
-  } else {
-    int container_width = container_->bounds().width();
-    window->SetTransform(gfx::Transform());
-    if (window == left_window_) {
-      if (separator_position_ == 0)
-        left_window_->Hide();
-      if (state_ == INACTIVE)
-        left_window_ = NULL;
-    } else {
-      if (separator_position_ == container_width)
-        right_window_->Hide();
-      if (state_ == INACTIVE)
-        right_window_ = NULL;
-    }
+  UpdateLayout(false);
+
+  if (state_ == INACTIVE) {
+    left_window_ = NULL;
+    right_window_ = NULL;
   }
 }
 
diff --git a/athena/wm/split_view_controller.h b/athena/wm/split_view_controller.h
index d0e7073..56c33c7 100644
--- a/athena/wm/split_view_controller.h
+++ b/athena/wm/split_view_controller.h
@@ -35,11 +35,12 @@ class ATHENA_EXPORT SplitViewController
   // |left| nor |right|) is selected instead.
   void ActivateSplitMode(aura::Window* left, aura::Window* right);
 
-  // Resets the internal state to an inactive state. Calling this does not
-  // change the window bounds/transforms etc. The caller must take care of
-  // making any necessary changes.
+  // Resets the internal state to an inactive state.
   void DeactivateSplitMode();
 
+  // Replaces |window| in split-view mode with |replace_with|. Adjusts
+  // |replace_with|'s visibility, transform and bounds. Resets |window|'s
+  // visibility and transform but does not change its bounds.
   void ReplaceWindow(aura::Window* window,
                      aura::Window* replace_with);
 
@@ -69,11 +70,12 @@ class ATHENA_EXPORT SplitViewController
 
   void UpdateLayout(bool animate);
 
-  void SetWindowTransform(aura::Window* left_window,
-                          const gfx::Transform& transform,
-                          bool animate);
+  void SetWindowTransforms(const gfx::Transform& left_transform,
+                           const gfx::Transform& right_transform,
+                           bool animate);
 
-  void OnAnimationCompleted(aura::Window* window);
+  // Called when the animation initiated by SetWindowTransforms() completes.
+  void OnAnimationCompleted();
 
   void UpdateSeparatorPositionFromScrollDelta(float delta);
 
diff --git a/athena/wm/window_manager_impl.cc b/athena/wm/window_manager_impl.cc
index 2b53961..6e42c17a 100644
--- a/athena/wm/window_manager_impl.cc
+++ b/athena/wm/window_manager_impl.cc
@@ -101,14 +101,14 @@ void AthenaContainerLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
   aura::Window::Windows list = instance->window_list_provider_->GetWindowList();
   if (std::find(list.begin(), list.end(), child) == list.end())
     return;
-  gfx::Size size;
   if (instance->split_view_controller_->IsSplitViewModeActive()) {
-    size = instance->split_view_controller_->left_window()->bounds().size();
+    instance->split_view_controller_->ReplaceWindow(
+        instance->split_view_controller_->left_window(), child);
   } else {
-    size =
+    gfx::Size size =
         gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().work_area().size();
+    child->SetBounds(gfx::Rect(size));
   }
-  child->SetBounds(gfx::Rect(size));
 }
 
 void AthenaContainerLayoutManager::OnWillRemoveWindowFromLayout(
diff --git a/athena/wm/window_manager_unittest.cc b/athena/wm/window_manager_unittest.cc
index 456e35c..0f03ae1 100644
--- a/athena/wm/window_manager_unittest.cc
+++ b/athena/wm/window_manager_unittest.cc
@@ -139,6 +139,8 @@ TEST_F(WindowManagerTest, BezelGestureToSwitchBetweenWindows) {
   scoped_ptr<aura::Window> first(CreateWindow(&delegate));
   scoped_ptr<aura::Window> second(CreateWindow(&delegate));
   scoped_ptr<aura::Window> third(CreateWindow(&delegate));
+  first->Hide();
+  second->Hide();
 
   test::WindowManagerImplTestApi wm_api;
   aura::client::ParentWindowWithContext(
@@ -163,6 +165,9 @@ TEST_F(WindowManagerTest, BezelGestureToSwitchBetweenWindows) {
   EXPECT_TRUE(wm::IsActiveWindow(second.get()));
   EXPECT_EQ(second.get(),
             wm_api.GetWindowListProvider()->GetWindowList().back());
+  EXPECT_FALSE(first->IsVisible());
+  EXPECT_TRUE(second->IsVisible());
+  EXPECT_FALSE(third->IsVisible());
 }
 
 TEST_F(WindowManagerTest, TitleDragSwitchBetweenWindows) {
@@ -300,16 +305,12 @@ TEST_F(WindowManagerTest, NewWindowBounds) {
   EXPECT_NE(work_area.ToString(),
             left_bounds.size().ToString());
 
+  // A new window should replace the left window when in split view.
   scoped_ptr<aura::Window> third(CreateWindow(&delegate));
   aura::client::ParentWindowWithContext(
       third.get(), ScreenManager::Get()->GetContext(), gfx::Rect());
-  EXPECT_NE(wm_api.GetSplitViewController()->left_window(), third.get());
+  EXPECT_EQ(wm_api.GetSplitViewController()->left_window(), third.get());
   EXPECT_EQ(left_bounds.ToString(), third->bounds().ToString());
-
-  third->Hide();
-  EXPECT_EQ(
-      left_bounds.ToString(),
-      wm_api.GetSplitViewController()->left_window()->bounds().ToString());
 }
 
 TEST_F(WindowManagerTest, SplitModeActivationByShortcut) {