mus: Fix activation cycle direction.

components/mus/ws/focus_controller.h

Index: components/mus/ws/focus_controller.h
diff --git a/components/mus/ws/focus_controller.h b/components/mus/ws/focus_controller.h
index a86c86e3912ad44114cf3ee65ff1b1ecdcecf8bd..9d3643c5285689557c193f7675902a3f80f09c83 100644
--- a/components/mus/ws/focus_controller.h
+++ b/components/mus/ws/focus_controller.h
@@ -18,6 +18,7 @@ class FocusControllerDelegate;
 class FocusControllerObserver;
 class ServerWindow;
 class ServerWindowDrawnTracker;
+struct WindowId;
 
 // Describes the source of the change.
 enum class FocusControllerChangeSource {
@@ -44,7 +45,13 @@ class FocusController : public ServerWindowDrawnTrackerObserver {
   void RemoveObserver(FocusControllerObserver* observer);
 
  private:
-  void SetActiveWindow(ServerWindow* window);
+  enum class ActivationChangeReason {
+    UNKNONW,
+    CYCLE,  // Activation changed because of ActivateNextWindow().
+    FOCUS,  // Focus change required a different window to be activated.
+    DRAWN_STATE_CHANGED,  // Active window was hidden or destroyed.
+  };
+  void SetActiveWindow(ServerWindow* window, ActivationChangeReason reason);
 
   // Returns whether |window| can be focused or activated.
   bool CanBeFocused(ServerWindow* window) const;
@@ -71,6 +78,10 @@ class FocusController : public ServerWindowDrawnTrackerObserver {
   ServerWindow* root_;
   ServerWindow* focused_window_;
   ServerWindow* active_window_;
+  // Tracks what caused |active_window_| to be activated.
+  ActivationChangeReason activation_reason_;
+
+  std::vector<WindowId> cycle_windows_;

[sky, 2016/01/05 16:42:38]

As far as I can tell you don't update this as windows are destroyed or moved
around in the hierarchy. Doesn't it need to be updated when either of those
happen?

[sadrul, 2016/01/05 17:14:53]

We don't need to handle new windows, since those would either be activated
explicitly, thus breaking the cycle (|activation_reason_| would change), or will
be activated based on the usual stacking order traversal.

For windows that are destroyed: we won't find them as activation-candidate, so
even if they are in the list, the right window will be activated. Two caveats
though: (1) we would be cycling over all windows multiple times when we erase an
already destroyed window from |cycle_windows_|, and (2) cycle_windows_ keeps
growing until the activation-cycle is broken. To fix this: we could either
introduce a ServerWindowTracker and use that, instead of this std::vector<>, or
introduce a IsWindowAlive()/IsWindowValid() to FocusControllerDelegate, and use
that when removing from |cycle_windows_|, and keep removing until we remove a
valid window. I prefer the latter option; I think it's simpler. WDYT?

 
   base::ObserverList<FocusControllerObserver> observers_;