Converts focusmanager from delegate to observer

I'm going to need more than one object to detect focus changes.

BUG=548424
TEST=covered by tests
R=ben@chromium.org

Review URL: https://codereview.chromium.org/1465143004

Cr-Commit-Position: refs/heads/master@{#361150}

components/mus/gles2/gpu_state.cc

 // 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.
-//
 
 #include "components/mus/gles2/gpu_state.h"
 

components/mus/ws/BUILD.gn

@@ -27,7 +27,7 @@ source_set("lib") {
     "event_dispatcher_delegate.h",
     "focus_controller.cc",
     "focus_controller.h",
-    "focus_controller_delegate.h",
+    "focus_controller_observer.h",
     "forwarding_window_manager.cc",
     "forwarding_window_manager.h",
     "operation.cc",
@@ -64,7 +64,7 @@ source_set("lib") {
     "//cc/surfaces",
     "//cc/surfaces:surface_id",
     "//components/mus/common",
-    "//components/mus/gles2:gles2",
+    "//components/mus/gles2",
     "//components/mus/public/interfaces",
     "//components/mus/surfaces",
     "//mojo/application/public/cpp",
@@ -79,10 +79,10 @@ source_set("lib") {
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",
-    "//ui/gl:gl",
+    "//ui/gl",
     "//ui/mojo/geometry:interfaces",
+    "//ui/platform_window",
     "//ui/platform_window:platform_impls",
-    "//ui/platform_window:platform_window",
   ]
 }
 

components/mus/ws/connection_manager.h

@@ -14,7 +14,6 @@
 #include "components/mus/public/interfaces/window_tree.mojom.h"
 #include "components/mus/public/interfaces/window_tree_host.mojom.h"
 #include "components/mus/surfaces/surfaces_state.h"
-#include "components/mus/ws/focus_controller_delegate.h"
 #include "components/mus/ws/ids.h"
 #include "components/mus/ws/operation.h"
 #include "components/mus/ws/server_window_delegate.h"
@@ -25,7 +24,6 @@
 #include "mojo/public/cpp/bindings/binding.h"
 
 namespace mus {
-
 namespace ws {
 
 class ClientConnection;
@@ -265,7 +263,6 @@ class ConnectionManager : public ServerWindowDelegate,
 };
 
 }  // namespace ws
-
 }  // namespace mus
 
 #endif  // COMPONENTS_MUS_WS_CONNECTION_MANAGER_H_

components/mus/ws/focus_controller.cc

@@ -4,16 +4,14 @@
 
 #include "components/mus/ws/focus_controller.h"
 
-#include "components/mus/ws/focus_controller_delegate.h"
+#include "components/mus/ws/focus_controller_observer.h"
 #include "components/mus/ws/server_window.h"
 #include "components/mus/ws/server_window_drawn_tracker.h"
 
 namespace mus {
-
 namespace ws {
 
-FocusController::FocusController(FocusControllerDelegate* delegate)
-    : delegate_(delegate) {}
+FocusController::FocusController() {}
 
 FocusController::~FocusController() {}
 
@@ -21,13 +19,21 @@ void FocusController::SetFocusedWindow(ServerWindow* window) {
   if (GetFocusedWindow() == window)
     return;
 
-  SetFocusedWindowImpl(window, CHANGE_SOURCE_EXPLICIT);
+  SetFocusedWindowImpl(FocusControllerChangeSource::EXPLICIT, window);
 }
 
 ServerWindow* FocusController::GetFocusedWindow() {
   return drawn_tracker_ ? drawn_tracker_->window() : nullptr;
 }
 
+void FocusController::AddObserver(FocusControllerObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void FocusController::RemoveObserver(FocusControllerObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 bool FocusController::CanBeFocused(ServerWindow* window) const {
   // All ancestors of |window| must be drawn, and be focusable.
   for (ServerWindow* w = window; w; w = w->parent()) {
@@ -51,8 +57,9 @@ bool FocusController::CanBeActivated(ServerWindow* window) const {
   return true;
 }
 
-void FocusController::SetFocusedWindowImpl(ServerWindow* window,
-                                           ChangeSource change_source) {
+void FocusController::SetFocusedWindowImpl(
+    FocusControllerChangeSource change_source,
+    ServerWindow* window) {
   if (window && !CanBeFocused(window))
     return;
   ServerWindow* old = GetFocusedWindow();
@@ -65,17 +72,17 @@ void FocusController::SetFocusedWindowImpl(ServerWindow* window,
   else
     drawn_tracker_.reset();
 
-  if (change_source == CHANGE_SOURCE_DRAWN_STATE_CHANGED)
-    delegate_->OnFocusChanged(old, window);
+  FOR_EACH_OBSERVER(FocusControllerObserver, observers_,
+                    OnFocusChanged(change_source, old, window));
 }
 
 void FocusController::OnDrawnStateChanged(ServerWindow* ancestor,
                                           ServerWindow* window,
                                           bool is_drawn) {
   DCHECK(!is_drawn);  // We only observe when drawn.
-  SetFocusedWindowImpl(ancestor, CHANGE_SOURCE_DRAWN_STATE_CHANGED);
+  SetFocusedWindowImpl(FocusControllerChangeSource::DRAWN_STATE_CHANGED,
+                       ancestor);
 }
 
 }  // namespace ws
-
 }  // namespace mus

components/mus/ws/focus_controller.h

@@ -6,21 +6,28 @@
 #define COMPONENTS_MUS_WS_FOCUS_CONTROLLER_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;
 
-// Tracks a focused window. Focus is moved to another window when the drawn
-// state of the focused window changes and the delegate is notified.
+// 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:
-  explicit FocusController(FocusControllerDelegate* delegate);
+  FocusController();
   ~FocusController() override;
 
   // Sets the focused window. Does nothing if |window| is currently focused.
@@ -31,26 +38,24 @@ class FocusController : public ServerWindowDrawnTrackerObserver {
   // Moves activation to the next activatable window.
   void CycleActivationForward();
 
- private:
-  // Describes the source of the change.
-  enum ChangeSource {
-    CHANGE_SOURCE_EXPLICIT,
-    CHANGE_SOURCE_DRAWN_STATE_CHANGED,
-  };
+  void AddObserver(FocusControllerObserver* observer);
+  void RemoveObserver(FocusControllerObserver* observer);
 
+ private:
   // Returns whether |window| can be focused or activated.
   bool CanBeFocused(ServerWindow* window) const;
   bool CanBeActivated(ServerWindow* window) const;
 
   // Implementation of SetFocusedWindow().
-  void SetFocusedWindowImpl(ServerWindow* window, ChangeSource change_source);
+  void SetFocusedWindowImpl(FocusControllerChangeSource change_source,
+                            ServerWindow* window);
 
   // ServerWindowDrawnTrackerObserver:
   void OnDrawnStateChanged(ServerWindow* ancestor,
                            ServerWindow* window,
                            bool is_drawn) override;
 
-  FocusControllerDelegate* delegate_;
+  base::ObserverList<FocusControllerObserver> observers_;
   scoped_ptr<ServerWindowDrawnTracker> drawn_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusController);

components/mus/ws/focus_controller_delegate.h → components/mus/ws/focus_controller_observer.h

@@ -6,22 +6,22 @@
 #define COMPONENTS_MUS_WS_FOCUS_CONTROLLER_DELEGATE_H_
 
 namespace mus {
-
 namespace ws {
 
+enum class FocusControllerChangeSource;
 class ServerWindow;
 
-class FocusControllerDelegate {
+class FocusControllerObserver {
  public:
-  virtual void OnFocusChanged(ServerWindow* old_focused_window,
+  virtual void OnFocusChanged(FocusControllerChangeSource change_source,
+                              ServerWindow* old_focused_window,
                               ServerWindow* new_focused_window) = 0;
 
  protected:
-  ~FocusControllerDelegate() {}
+  ~FocusControllerObserver() {}
 };
 
 }  // namespace ws
-
 }  // namespace mus
 
 #endif  // COMPONENTS_MUS_WS_FOCUS_CONTROLLER_DELEGATE_H_

components/mus/ws/focus_controller_unittest.cc

@@ -4,7 +4,7 @@
 
 #include "components/mus/ws/focus_controller.h"
 
-#include "components/mus/ws/focus_controller_delegate.h"
+#include "components/mus/ws/focus_controller_observer.h"
 #include "components/mus/ws/server_window.h"
 #include "components/mus/ws/test_server_window_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -14,9 +14,9 @@ namespace mus {
 namespace ws {
 namespace {
 
-class TestFocusControllerDelegate : public FocusControllerDelegate {
+class TestFocusControllerObserver : public FocusControllerObserver {
  public:
-  TestFocusControllerDelegate()
+  TestFocusControllerObserver()
       : change_count_(0u),
         old_focused_window_(nullptr),
         new_focused_window_(nullptr) {}
@@ -31,9 +31,13 @@ class TestFocusControllerDelegate : public FocusControllerDelegate {
   ServerWindow* new_focused_window() { return new_focused_window_; }
 
  private:
-  // FocusControllerDelegate:
-  void OnFocusChanged(ServerWindow* old_focused_window,
+  // FocusControllerObserver:
+  void OnFocusChanged(FocusControllerChangeSource source,
+                      ServerWindow* old_focused_window,
                       ServerWindow* new_focused_window) override {
+    if (source == FocusControllerChangeSource::EXPLICIT)
+      return;
+
     change_count_++;
     old_focused_window_ = old_focused_window;
     new_focused_window_ = new_focused_window;
@@ -43,7 +47,7 @@ class TestFocusControllerDelegate : public FocusControllerDelegate {
   ServerWindow* old_focused_window_;
   ServerWindow* new_focused_window_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestFocusControllerDelegate);
+  DISALLOW_COPY_AND_ASSIGN(TestFocusControllerObserver);
 };
 
 }  // namespace
@@ -60,50 +64,52 @@ TEST(FocusControllerTest, Basic) {
   child_child.SetVisible(true);
   child.Add(&child_child);
 
-  TestFocusControllerDelegate focus_delegate;
-  FocusController focus_controller(&focus_delegate);
+  TestFocusControllerObserver focus_observer;
+  FocusController focus_controller;
+  focus_controller.AddObserver(&focus_observer);
 
   focus_controller.SetFocusedWindow(&child_child);
-  EXPECT_EQ(0u, focus_delegate.change_count());
+  EXPECT_EQ(0u, focus_observer.change_count());
 
   // Remove the ancestor of the focused window, focus should go to the |root|.
   root.Remove(&child);
-  EXPECT_EQ(1u, focus_delegate.change_count());
-  EXPECT_EQ(&root, focus_delegate.new_focused_window());
-  EXPECT_EQ(&child_child, focus_delegate.old_focused_window());
-  focus_delegate.ClearAll();
+  EXPECT_EQ(1u, focus_observer.change_count());
+  EXPECT_EQ(&root, focus_observer.new_focused_window());
+  EXPECT_EQ(&child_child, focus_observer.old_focused_window());
+  focus_observer.ClearAll();
 
   // Make the focused window invisible. Focus is lost in this case (as no one
   // to give focus to).
   root.SetVisible(false);
-  EXPECT_EQ(1u, focus_delegate.change_count());
-  EXPECT_EQ(nullptr, focus_delegate.new_focused_window());
-  EXPECT_EQ(&root, focus_delegate.old_focused_window());
-  focus_delegate.ClearAll();
+  EXPECT_EQ(1u, focus_observer.change_count());
+  EXPECT_EQ(nullptr, focus_observer.new_focused_window());
+  EXPECT_EQ(&root, focus_observer.old_focused_window());
+  focus_observer.ClearAll();
 
   // Go back to initial state and focus |child_child|.
   root.SetVisible(true);
   root.Add(&child);
   focus_controller.SetFocusedWindow(&child_child);
-  EXPECT_EQ(0u, focus_delegate.change_count());
+  EXPECT_EQ(0u, focus_observer.change_count());
 
   // Hide the focused window, focus should go to parent.
   child_child.SetVisible(false);
-  EXPECT_EQ(1u, focus_delegate.change_count());
-  EXPECT_EQ(&child, focus_delegate.new_focused_window());
-  EXPECT_EQ(&child_child, focus_delegate.old_focused_window());
-  focus_delegate.ClearAll();
+  EXPECT_EQ(1u, focus_observer.change_count());
+  EXPECT_EQ(&child, focus_observer.new_focused_window());
+  EXPECT_EQ(&child_child, focus_observer.old_focused_window());
+  focus_observer.ClearAll();
 
   child_child.SetVisible(true);
   focus_controller.SetFocusedWindow(&child_child);
-  EXPECT_EQ(0u, focus_delegate.change_count());
+  EXPECT_EQ(0u, focus_observer.change_count());
 
   // Hide the parent of the focused window.
   child.SetVisible(false);
-  EXPECT_EQ(1u, focus_delegate.change_count());
-  EXPECT_EQ(&root, focus_delegate.new_focused_window());
-  EXPECT_EQ(&child_child, focus_delegate.old_focused_window());
-  focus_delegate.ClearAll();
+  EXPECT_EQ(1u, focus_observer.change_count());
+  EXPECT_EQ(&root, focus_observer.new_focused_window());
+  EXPECT_EQ(&child_child, focus_observer.old_focused_window());
+  focus_observer.ClearAll();
+  focus_controller.RemoveObserver(&focus_observer);
 }
 
 }  // namespace ws

components/mus/ws/window_tree_host_impl.cc

@@ -30,8 +30,9 @@ WindowTreeHostImpl::WindowTreeHostImpl(
       event_dispatcher_(this),
       display_manager_(
           DisplayManager::Create(app_impl, gpu_state, surfaces_state)),
-      focus_controller_(new FocusController(this)),
+      focus_controller_(new FocusController),
       window_manager_(window_manager.Pass()) {
+  focus_controller_->AddObserver(this);
   display_manager_->Init(this);
   if (client_) {
     client_.set_connection_error_handler(base::Bind(
@@ -40,6 +41,7 @@ WindowTreeHostImpl::WindowTreeHostImpl(
 }
 
 WindowTreeHostImpl::~WindowTreeHostImpl() {
+  DestroyFocusController();
   for (ServerWindow* window : windows_needing_frame_destruction_)
     window->RemoveObserver(this);
 }
@@ -89,8 +91,6 @@ void WindowTreeHostImpl::SetFocusedWindow(ServerWindow* new_focused_window) {
     return;
   DCHECK(root_window()->Contains(new_focused_window));
   focus_controller_->SetFocusedWindow(new_focused_window);
-  // TODO(beng): have the FocusController notify us via FocusControllerDelegate.
-  OnFocusChanged(old_focused_window, new_focused_window);
 }
 
 ServerWindow* WindowTreeHostImpl::GetFocusedWindow() {
@@ -98,6 +98,10 @@ ServerWindow* WindowTreeHostImpl::GetFocusedWindow() {
 }
 
 void WindowTreeHostImpl::DestroyFocusController() {
+  if (!focus_controller_)
+    return;
+
+  focus_controller_->RemoveObserver(this);
   focus_controller_.reset();
 }
 
@@ -185,8 +189,10 @@ void WindowTreeHostImpl::OnCompositorFrameDrawn() {
   }
 }
 
-void WindowTreeHostImpl::OnFocusChanged(ServerWindow* old_focused_window,
-                                        ServerWindow* new_focused_window) {
+void WindowTreeHostImpl::OnFocusChanged(
+    FocusControllerChangeSource change_source,
+    ServerWindow* old_focused_window,
+    ServerWindow* new_focused_window) {
   // There are up to four connections that need to be notified:
   // . the connection containing |old_focused_window|.
   // . the connection with |old_focused_window| as its root.

components/mus/ws/window_tree_host_impl.h

@@ -12,7 +12,7 @@
 #include "components/mus/ws/display_manager.h"
 #include "components/mus/ws/event_dispatcher.h"
 #include "components/mus/ws/event_dispatcher_delegate.h"
-#include "components/mus/ws/focus_controller_delegate.h"
+#include "components/mus/ws/focus_controller_observer.h"
 #include "components/mus/ws/server_window.h"
 #include "components/mus/ws/server_window_observer.h"
 
@@ -31,7 +31,7 @@ class WindowTreeImpl;
 // deleted.
 class WindowTreeHostImpl : public DisplayManagerDelegate,
                            public mojom::WindowTreeHost,
-                           public FocusControllerDelegate,
+                           public FocusControllerObserver,
                            public EventDispatcherDelegate,
                            public ServerWindowObserver {
  public:
@@ -108,8 +108,9 @@ class WindowTreeHostImpl : public DisplayManagerDelegate,
   void OnTopLevelSurfaceChanged(cc::SurfaceId surface_id) override;
   void OnCompositorFrameDrawn() override;
 
-  // FocusControllerDelegate:
-  void OnFocusChanged(ServerWindow* old_focused_window,
+  // FocusControllerObserver:
+  void OnFocusChanged(FocusControllerChangeSource change_source,
+                      ServerWindow* old_focused_window,
                       ServerWindow* new_focused_window) override;
 
   // EventDispatcherDelegate: