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

athena/wm/split_view_controller.cc

Index: athena/wm/split_view_controller.cc
diff --git a/athena/wm/split_view_controller.cc b/athena/wm/split_view_controller.cc
index e36b7a94e05623cbabd35e45564bdb0f691ae6e1..efb85fd65806f7c72bbaa7cf1b38b572251df1ff 100644
--- a/athena/wm/split_view_controller.cc
+++ b/athena/wm/split_view_controller.cc
@@ -112,19 +112,19 @@ 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());
 }
 
 void SplitViewController::DeactivateSplitMode() {
   CHECK_NE(SCROLLING, state_);
   state_ = INACTIVE;
+  UpdateLayout(false);
   left_window_ = right_window_ = NULL;
 }
 
@@ -146,73 +146,64 @@ void SplitViewController::UpdateLayout(bool animate) {
   if  (!left_window_)
     return;
   CHECK(right_window_);
-  gfx::Transform left_transform;
-  gfx::Transform right_transform;
-  int container_width = container_->GetBoundsInScreen().width();
   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 if (state_ == INACTIVE && !animate) {
+    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);
 }
 
-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);
+    left_window_->SetTransform(left_transform);
+
+    ui::ScopedLayerAnimationSettings right_settings(
+        right_window_->layer()->GetAnimator());
+    right_settings.SetPreemptionStrategy(
         ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-    settings.AddObserver(new ui::ClosureAnimationObserver(
+    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();

[pkotwicz, 2014/08/28 22:03:45]

I couldn't figure out why the window is hidden. Activating the window seems
sufficient

[sadrul, 2014/08/29 13:31:30]

I believe the idea here is to hide the window that's not visible anymore. We
should continue doing this. (and add a check for this in
BezelGestureToSwitchBetweenWindows)

[pkotwicz, 2014/08/29 14:44:45]

I have brought back the 'hide window' functionality. I have not expanded the
unit test because non-visible windows are not hidden by default and overview
mode does not hide windows. We should add / expand unit tests at that time

[sadrul, 2014/08/29 15:06:02]

We do hide non-visible windows in some cases (e.g. when switching windows using
title-drag), and we have test coverage for those. You should update the test for
this case too. (I have a fix in progress to hide the windows after exiting from
overview mode too, but that's independent of this behaviour).

[pkotwicz, 2014/08/29 18:07:42]

I have modified the unit test as requested

-      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;
   }
 }