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());
+  window->Hide();
 }
 
 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_ == INACTIVE && !animate) {
+    if (!wm::IsActiveWindow(left_window_))
+      left_window_->Hide();
+    if (!wm::IsActiveWindow(right_window_))
+      right_window_->Hide();

[sadrul, 2014/08/29 19:53:53]

Can you move this code with the block in line 174 below?

[pkotwicz, 2014/08/29 20:18:41]

Done.

+  } else {
+    left_window_->Show();

[sadrul, 2014/08/29 19:53:53]

Do we ever call UpdateLayout() with state_ == INACTIVE, animate == true?

[pkotwicz, 2014/08/29 20:18:40]

Yes, from ScrollEnd()

+    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 if (state_ == INACTIVE && !animate) {
+    SetWindowTransforms(gfx::Transform(), gfx::Transform(), false);
   } else {
-    left_transform.Translate(separator_position_ - container_width, 0);
+    gfx::Transform left_transform;

[sadrul, 2014/08/29 19:53:53]

Do we expect state_ here to always be SCROLLING? If so, add a [D]CHECK_EQ?

[pkotwicz, 2014/08/29 20:18:40]

We do not. We want to go through here when called from ScrollEnd()

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