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

athena/wm/split_view_controller.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "athena/wm/split_view_controller.h"
 
 #include <cmath>
 
 #include "athena/wm/public/window_list_provider.h"
 #include "athena/wm/public/window_manager.h"
@@ skipping 94 matching lines @@
   CHECK(IsSplitViewModeActive());
   CHECK(replace_with);
   CHECK(window == left_window_ || window == right_window_);
   CHECK(replace_with != left_window_ && replace_with != right_window_);
 #if !defined(NDEBUG)
   aura::Window::Windows windows = window_list_provider_->GetWindowList();
   DCHECK(std::find(windows.begin(), windows.end(), replace_with) !=
          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;
 }
 
 gfx::Rect SplitViewController::GetLeftTargetBounds() {
   gfx::Rect work_area =
       gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().work_area();
   return gfx::Rect(0, 0, container_->bounds().width() / 2, work_area.height());
 }
 
 gfx::Rect SplitViewController::GetRightTargetBounds() {
   gfx::Rect work_area =
       gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().work_area();
   int container_width = container_->bounds().width();
   return gfx::Rect(
       container_width / 2, 0, container_width / 2, work_area.height());
 }
 
 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);
-  }
-  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) {
-  if (animate) {
-    scoped_refptr<ui::LayerAnimator> animator = window->layer()->GetAnimator();
-    ui::ScopedLayerAnimationSettings settings(animator);
-    settings.SetPreemptionStrategy(
-        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-    settings.AddObserver(new ui::ClosureAnimationObserver(
-        base::Bind(&SplitViewController::OnAnimationCompleted,
-                   weak_factory_.GetWeakPtr(),
-                   window)));
-    window->SetTransform(transform);
-  } else {
-    window->SetTransform(transform);
+    SetWindowTransforms(left_transform, right_transform, animate);
   }
 }
 
-void SplitViewController::OnAnimationCompleted(aura::Window* window) {
+void SplitViewController::SetWindowTransforms(
+    const gfx::Transform& left_transform,
+    const gfx::Transform& right_transform,
+    bool animate) {
+  if (animate) {
+    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);
+    right_settings.AddObserver(new ui::ClosureAnimationObserver(
+        base::Bind(&SplitViewController::OnAnimationCompleted,
+                   weak_factory_.GetWeakPtr())));
+    right_window_->SetTransform(right_transform);
+  } else {
+    left_window_->SetTransform(left_transform);
+    right_window_->SetTransform(right_transform);
+  }
+}
+
+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;
   }
 }
 
 void SplitViewController::UpdateSeparatorPositionFromScrollDelta(float delta) {
   gfx::Screen* screen = gfx::Screen::GetScreenFor(container_);
   const gfx::Rect& display_bounds =
       screen->GetDisplayNearestWindow(container_).bounds();
   gfx::Rect container_bounds = container_->GetBoundsInScreen();
   separator_position_ =
       delta > 0 ? ((int)delta) + display_bounds.x() - container_bounds.x()
@@ skipping 62 matching lines @@
 }
 
 bool SplitViewController::CanScroll() {
   // TODO(mfomitchev): return false in vertical orientation, in full screen.
   bool result = (!IsSplitViewModeActive() &&
                  window_list_provider_->GetWindowList().size() >= 2);
   return result;
 }
 
 }  // namespace athena