athena: Fix switching windows with title-drag in split-view.

athena/wm/window_manager_impl.cc

Index: athena/wm/window_manager_impl.cc
diff --git a/athena/wm/window_manager_impl.cc b/athena/wm/window_manager_impl.cc
index 5574905691e04ecd9a6f8e1c4d8b6f87a8e0d4b5..f4343d49821b7d05b77bdf83d57be18ea40c58eb 100644
--- a/athena/wm/window_manager_impl.cc
+++ b/athena/wm/window_manager_impl.cc
@@ -246,22 +246,66 @@ bool WindowManagerImpl::OnAcceleratorFired(int command_id,
 
 aura::Window* WindowManagerImpl::GetWindowBehind(aura::Window* window) {
   const aura::Window::Windows& windows = container_->children();

[Jun Mukai, 2014/08/13 23:38:59]

Possibly you should use mru_window_tracker_ here.

[mfomitchev, 2014/08/14 14:38:13]

+1

[sadrul, 2014/08/15 19:00:20]

Done.

-  aura::Window::Windows::const_iterator iter =
-      std::find(windows.begin(), windows.end(), window);
-  CHECK(iter != windows.end());
-  return (iter == windows.begin()) ? NULL : *(iter - 1);
+  aura::Window::Windows::const_reverse_iterator iter =
+      std::find(windows.rbegin(), windows.rend(), window);
+  CHECK(iter != windows.rend());
+  ++iter;
+  aura::Window* behind = NULL;
+  if (iter != windows.rend())
+    behind = *iter++;
+
+  if (split_view_controller_->IsSplitViewModeActive()) {
+    CHECK(window == split_view_controller_->left_window() ||
+          window == split_view_controller_->right_window());
+    if (window == split_view_controller_->left_window() &&
+        behind == split_view_controller_->right_window() &&
+        iter != windows.rend()) {
+      behind = *iter;

[mfomitchev, 2014/08/14 14:38:13]

We need to set behind to NULL if iter == windows.rend() and the other two
conditions hold. We can also simplify the code:

if (behind != right && behind != left)
  return behind
if (iter == windows.rend())
  return NULL;
// Optional: CHECK(iter != right && iter != left);
return iter;

[sadrul, 2014/08/15 19:00:20]

Good catch. Done.

[mfomitchev, 2014/08/15 19:15:03]

Nit: I still prefer the code I suggested above - less lines, less nested ifs,
and no unnecessary if checks after you've already set |behind|.
Also: would be good to have a unit test for GetWindowBehind()

[sadrul, 2014/08/15 19:34:54]

This doesn't work right because the CHECK would trigger often (e.g. you switch
to split mode and the left window is active. You start title-dragging the
left-window. It's going to return the right-window)
Indeed. I am working on a separate patch to expose WindowManagerImpl out of
window_manager_impl.cc, so that it can be tested. I will add the tests in that
CL.

[sadrul, 2014/08/15 19:46:43]

Mikhail pointed out that this actually does do the right thing. Thanks! Made
this change.

+    }
+
+    if (window == split_view_controller_->right_window() &&
+        behind == split_view_controller_->left_window() &&
+        iter != windows.rend()) {
+      behind = *iter;
+    }
+  }
+
+  return behind;
 }
 
 void WindowManagerImpl::OnTitleDragStarted(aura::Window* window) {
+  aura::Window* next_window = GetWindowBehind(window);
+  if (!next_window)
+    return;
+  // Make sure |window| is active. Also make sure that |next_window| is visible,
+  // and positioned to match the top-left edge of |window|.
+  wm::ActivateWindow(window);
+  next_window->Show();
+  int dx = window->bounds().x() - next_window->bounds().x();
+  if (dx) {
+    gfx::Transform transform;
+    transform.Translate(dx, 0);
+    next_window->SetTransform(transform);
+  }
 }
 
 void WindowManagerImpl::OnTitleDragCompleted(aura::Window* window) {
   aura::Window* next_window = GetWindowBehind(window);
-  if (next_window)
+  if (!next_window)
+    return;
+  if (split_view_controller_->IsSplitViewModeActive())
+    split_view_controller_->ReplaceWindow(window, next_window);
+  else
     OnSelectWindow(next_window);
+  wm::ActivateWindow(next_window);
 }
 
 void WindowManagerImpl::OnTitleDragCanceled(aura::Window* window) {
+  aura::Window* next_window = GetWindowBehind(window);
+  if (!next_window)
+    return;
+  next_window->SetTransform(gfx::Transform());
+  next_window->Hide();
 }
 
 AthenaContainerLayoutManager::AthenaContainerLayoutManager() {