[Athena] Fix switching activities by swiping from the right bezel

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 82167232d8ce7b19707e3da1de73d35263db510e..5711c3f3db5d8fc6b8b7ff6f0cf4a8bcd35dfe13 100644
--- a/athena/wm/split_view_controller.cc
+++ b/athena/wm/split_view_controller.cc
@@ -95,9 +95,12 @@ void SplitViewController::ActivateSplitMode(aura::Window* left,
   if (right_window_ && right_window_ != left && right_window_ != right)
     to_hide_.push_back(right_window_);
 
-  SetState(ACTIVE);
   right_window_ = right;
   left_window_ = left;
+
+  window_list_provider_->MoveToFront(right_window_);
+  window_list_provider_->MoveToFront(left_window_);
+  SetState(ACTIVE);
   UpdateLayout(true);
 }
 
@@ -118,6 +121,8 @@ void SplitViewController::ReplaceWindow(aura::Window* window,
   else
     right_window_ = replace_with;
   wm::ActivateWindow(replace_with);
+  window_list_provider_->MoveToFront(right_window_);
+  window_list_provider_->MoveToFront(left_window_);

[mfomitchev, 2014/09/18 02:36:29]

ActivateWindow already moves the window to front with our window list provider
implementation, so this seems a bit excessive. We should be able to get rid of
one of the MoveToFront calls. Also, |replace_with| should probably be on top of
the other split view window, since |replace_with| is the active one. So the
Activate call should be after the MoveToFront call.

[pkotwicz, 2014/09/18 13:54:45]

I have removed the call to wm::ActivateWindow(). This is consistent with the
remainder of SplitViewController which does not do anything with activation.

We do not explicitly call wm::ActivateWindow() prior to calling ReplaceWindow()
in WindowManagerImpl::OnWindowAddedToLayout(). In practice, |child|, being a
views::Widget is activated when views::Widget::Show() is called. Ideally we
would wait till the new window is activated prior to replacing the split view
window.

w.r.t. to the relative z-order of the left and right windows, we do not care
about it in the case of ActivateSplitMode() so I do not think that we should
care about it in the ReplaceWindow() case either. In patch set #1 of
https://codereview.chromium.org/550043002/#ps1 I attempted to change
ActivateSplitMode() such that one of the passed in windows had to be active but
sadrul@ did not like that.

[mfomitchev, 2014/09/18 18:11:52]

We still use ActivateWindow in ScrollEnd - that's how we ensure that the window
that ends up displayed when is on top in the window list.

I think making sure the invariant "active window is the top one in the stack"
always holds would be a good thing.

In this particular case, the call is typically done when you swipe on the header
of one of the visible windows. If I do this on the left window, I'd expect the
left window to remain active. E.g. if I start typing, I'd expect keyboard input
to go to that window.

Maybe Sadrul, you, and I could have a quick chat on this.

   UpdateLayout(false);
   window->SetTransform(gfx::Transform());
   window->Hide();
@@ -170,8 +175,6 @@ void SplitViewController::UpdateLayout(bool animate) {
 
   left_window_->Show();
   right_window_->Show();
-  window_list_provider_->MoveToFront(right_window_);
-  window_list_provider_->MoveToFront(left_window_);
 
   if (state_ == ACTIVE) {
     if (animate) {