mus: Fix activation cycle direction.

components/mus/ws/focus_controller.cc

Index: components/mus/ws/focus_controller.cc
diff --git a/components/mus/ws/focus_controller.cc b/components/mus/ws/focus_controller.cc
index 5cf98446e9a526548a9dd4164bbcb01010906942..9e871b80ae1a41f60d9846c145a4871e5986fc9a 100644
--- a/components/mus/ws/focus_controller.cc
+++ b/components/mus/ws/focus_controller.cc
@@ -16,9 +16,9 @@ namespace ws {
 
 namespace {
 
-ServerWindow* GetDeepestFirstDescendant(ServerWindow* window) {
+ServerWindow* GetDeepestLastDescendant(ServerWindow* window) {
   while (!window->children().empty())
-    window = window->children()[0];
+    window = window->children().back();
   return window;
 }
 
@@ -31,18 +31,18 @@ class WindowTreeIterator {
 
   ServerWindow* GetNext(ServerWindow* window) const {
     if (window == root_ || window == nullptr)
-      return GetDeepestFirstDescendant(root_);
+      return GetDeepestLastDescendant(root_);
 
     // Return the next sibling.
     ServerWindow* parent = window->parent();
     if (parent) {
       const ServerWindow::Windows& siblings = parent->children();
-      ServerWindow::Windows::const_iterator iter =
-          std::find(siblings.begin(), siblings.end(), window);
-      DCHECK(iter != siblings.end());
+      ServerWindow::Windows::const_reverse_iterator iter =
+          std::find(siblings.rbegin(), siblings.rend(), window);
+      DCHECK(iter != siblings.rend());
       ++iter;
-      if (iter != siblings.end())
-        return GetDeepestFirstDescendant(*iter);
+      if (iter != siblings.rend())
+        return GetDeepestLastDescendant(*iter);
     }
 
     // All children and siblings have been explored. Next is the parent.
@@ -62,12 +62,15 @@ FocusController::FocusController(FocusControllerDelegate* delegate,
     : delegate_(delegate),
       root_(root),
       focused_window_(nullptr),
-      active_window_(nullptr) {
+      active_window_(nullptr),
+      activation_reason_(ActivationChangeReason::UNKNONW) {
   DCHECK(delegate_);
   DCHECK(root_);
 }
 
-FocusController::~FocusController() {}
+FocusController::~FocusController() {
+  ClearCycleWindows();
+}
 
 void FocusController::SetFocusedWindow(ServerWindow* window) {
   if (GetFocusedWindow() == window)
@@ -83,10 +86,28 @@ ServerWindow* FocusController::GetFocusedWindow() {
 void FocusController::ActivateNextWindow() {
   WindowTreeIterator iter(root_);
   ServerWindow* activate = active_window_;
-  do {
+  while (true) {
     activate = iter.GetNext(activate);
-  } while (activate != active_window_ && !CanBeActivated(activate));
-  SetActiveWindow(activate);
+    if (activation_reason_ == ActivationChangeReason::CYCLE) {
+      if (activate == active_window_) {
+        // We have cycled over all the activatable windows. Remove the oldest
+        // window that was cycled.
+        if (!cycle_windows_.empty()) {
+          cycle_windows_.front()->RemoveObserver(this);
+          cycle_windows_.erase(cycle_windows_.begin());
+          continue;
+        }
+      } else if (std::find(cycle_windows_.begin(), cycle_windows_.end(),

[sky, 2016/01/06 21:02:39]

FYI: you can use ContainsValue(cycle_windows_, activate) if you prefer (recently
added to stl_util.h).

+                           activate) != cycle_windows_.end()) {
+        // We are cycling between activated windows, and this window has already
+        // been through the cycle. So skip over it.
+        continue;
+      }
+    }
+    if (activate == active_window_ || CanBeActivated(activate))
+      break;
+  }
+  SetActiveWindow(activate, ActivationChangeReason::CYCLE);
 
   if (active_window_) {
     // Do not shift focus if the focused window already lives in the active
@@ -113,15 +134,29 @@ void FocusController::RemoveObserver(FocusControllerObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void FocusController::SetActiveWindow(ServerWindow* window) {
+void FocusController::SetActiveWindow(ServerWindow* window,
+                                      ActivationChangeReason reason) {
   DCHECK(!window || CanBeActivated(window));
   if (active_window_ == window)
     return;
+  if (reason != ActivationChangeReason::CYCLE) {
+    ClearCycleWindows();
+  } else if (activation_reason_ != ActivationChangeReason::CYCLE) {
+    DCHECK(cycle_windows_.empty());
+    if (active_window_) {
+      active_window_->AddObserver(this);
+      cycle_windows_.push_back(active_window_);
+    }
+  }
+
   ServerWindow* old_active = active_window_;
   active_window_ = window;
-  if (old_active != active_window_) {
-    FOR_EACH_OBSERVER(FocusControllerObserver, observers_,
-                      OnActivationChanged(old_active, active_window_));
+  activation_reason_ = reason;
+  FOR_EACH_OBSERVER(FocusControllerObserver, observers_,
+                    OnActivationChanged(old_active, active_window_));
+  if (active_window_ && activation_reason_ == ActivationChangeReason::CYCLE) {
+    active_window_->AddObserver(this);

[sky, 2016/01/06 21:02:39]

nit: add an AddWindowToCycleWindows (or something similarly named) and use it on
147-148 as well.

+    cycle_windows_.push_back(active_window_);
   }
 }
 
@@ -187,7 +222,8 @@ void FocusController::SetFocusedWindowImpl(
   // Activate the closest activatable ancestor window.
   // TODO(sad): The window to activate doesn't necessarily have to be a direct
   // ancestor (e.g. could be a transient parent).
-  SetActiveWindow(GetActivatableAncestorOf(window));
+  SetActiveWindow(GetActivatableAncestorOf(window),
+                  ActivationChangeReason::FOCUS);
 
   FOR_EACH_OBSERVER(FocusControllerObserver, observers_,
                     OnFocusChanged(change_source, old_focused, window));
@@ -207,6 +243,13 @@ void FocusController::SetFocusedWindowImpl(
     drawn_tracker_.reset();
 }
 
+void FocusController::ClearCycleWindows() {
+  while (!cycle_windows_.empty()) {
+    cycle_windows_.front()->RemoveObserver(this);
+    cycle_windows_.erase(cycle_windows_.begin());
+  }
+}
+
 void FocusController::OnDrawnStateWillChange(ServerWindow* ancestor,
                                              ServerWindow* window,
                                              bool is_drawn) {
@@ -229,7 +272,7 @@ void FocusController::OnDrawnStateWillChange(ServerWindow* ancestor,
              (will_be_hidden(activate) || !CanBeActivated(activate)));
     if (activate == window)
       activate = nullptr;
-    SetActiveWindow(activate);
+    SetActiveWindow(activate, ActivationChangeReason::DRAWN_STATE_CHANGED);
 
     // Now make sure focus is in the active window.
     ServerWindow* focus = nullptr;
@@ -266,5 +309,10 @@ void FocusController::OnDrawnStateChanged(ServerWindow* ancestor,
   DCHECK(false);
 }
 
+void FocusController::OnWillDestroyWindow(ServerWindow* window) {
+  cycle_windows_.erase(
+      std::find(cycle_windows_.begin(), cycle_windows_.end(), window));

[sky, 2016/01/06 21:02:39]

also remove the observer here too.

+}
+
 }  // namespace ws
 }  // namespace mus