mus: Fix activation cycle direction.

components/mus/ws/focus_controller.h

 // Copyright 2015 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.
 
 #ifndef COMPONENTS_MUS_WS_FOCUS_CONTROLLER_H_
 #define COMPONENTS_MUS_WS_FOCUS_CONTROLLER_H_
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "components/mus/ws/server_window_drawn_tracker_observer.h"
 
 namespace mus {
 
 namespace ws {
 
 class FocusControllerDelegate;
 class FocusControllerObserver;
 class ServerWindow;
 class ServerWindowDrawnTracker;
+struct WindowId;
 
 // Describes the source of the change.
 enum class FocusControllerChangeSource {
   EXPLICIT,
   DRAWN_STATE_CHANGED,
 };
 
 // Tracks the focused window. Focus is moved to another window when the drawn
 // state of the focused window changes.
 class FocusController : public ServerWindowDrawnTrackerObserver {
  public:
   FocusController(FocusControllerDelegate* delegate, ServerWindow* root);
   ~FocusController() override;
 
   // Sets the focused window. Does nothing if |window| is currently focused.
   // This does not notify the delegate.
   void SetFocusedWindow(ServerWindow* window);
   ServerWindow* GetFocusedWindow();
 
   // Moves activation to the next activatable window.
   void ActivateNextWindow();
 
   void AddObserver(FocusControllerObserver* observer);
   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;
   bool CanBeActivated(ServerWindow* window) const;
 
   // Returns the closest activatable ancestor of |window|. Returns nullptr if
   // there is no such ancestor.
   ServerWindow* GetActivatableAncestorOf(ServerWindow* window) const;
 
   // Implementation of SetFocusedWindow().
   void SetFocusedWindowImpl(FocusControllerChangeSource change_source,
                             ServerWindow* window);
 
   // ServerWindowDrawnTrackerObserver:
   void OnDrawnStateWillChange(ServerWindow* ancestor,
                               ServerWindow* window,
                               bool is_drawn) override;
   void OnDrawnStateChanged(ServerWindow* ancestor,
                            ServerWindow* window,
                            bool is_drawn) override;
 
   FocusControllerDelegate* delegate_;
 
   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_;
 
   // Keeps track of the visibility of the focused and active window.
   scoped_ptr<ServerWindowDrawnTracker> drawn_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusController);
 };
 
 }  // namespace ws
 
 }  // namespace mus
 
 #endif  // COMPONENTS_MUS_WS_FOCUS_CONTROLLER_H_