MacViews: Move focus logic to NativeWidgetMac

The logic to add FocusManager observers was smeared across
NativeWidgetMac and NativeWidgetMacNSWindowHost. Change this so that
all of the logic is in NativeWidgetMac::SetFocusManager. In the
process, cut-and-paste a handful of functionality over to
NativeWidgetMac.

This is part of the larger trend of merging NativeWidgetMac and
NativeWidgetMacNSWindowHost (preserving the latter perhaps, only as
mojo-implementing friend).

The motivation for this is that, when reparenting a non-top-level
NativeWidgetMac, we do not move the FocusManager observers, which
causes ConstrainedWindowViewTest.TabMoveTest to crash.

This patch should have no functional change. The next patch in the
sequence will update the observers as needed.

Bug: 957362
Change-Id: Id5a1d6183e446df647fa877239cb9dfb549e18a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983343Reviewed-by: Leonard Grey <lgrey@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727645}

ui/views/cocoa/bridged_native_widget_unittest.mm

@@ -326,7 +326,7 @@ class MockNativeWidgetMac : public NativeWidgetMac {
     delegate()->OnNativeWidgetCreated();
 
     // To allow events to dispatch to a view, it needs a way to get focus.
-    GetNSWindowHost()->SetFocusManager(GetWidget()->GetFocusManager());
+    SetFocusManager(GetWidget()->GetFocusManager());
   }
 
   void ReorderNativeViews() override {
@@ -855,7 +855,7 @@ TEST_F(BridgedNativeWidgetTest, ViewSizeTracksWindow) {
 }
 
 TEST_F(BridgedNativeWidgetTest, GetInputMethodShouldNotReturnNull) {
-  EXPECT_TRUE(GetNSWindowHost()->GetInputMethod());
+  EXPECT_TRUE(native_widget_mac_->GetInputMethod());
 }
 
 // A simpler test harness for testing initialization flows.

ui/views/cocoa/native_widget_mac_ns_window_host.h

@@ -19,11 +19,9 @@
 #include "mojo/public/cpp/bindings/associated_remote.h"
 #include "ui/accelerated_widget_mac/accelerated_widget_mac.h"
 #include "ui/base/cocoa/accessibility_focus_overrider.h"
-#include "ui/base/ime/input_method_delegate.h"
 #include "ui/compositor/layer_owner.h"
 #include "ui/display/mac/display_link_mac.h"
 #include "ui/views/cocoa/drag_drop_client_mac.h"
-#include "ui/views/focus/focus_manager.h"
 #include "ui/views/views_export.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_observer.h"
@@ -54,8 +52,6 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
       public remote_cocoa::ApplicationHost::Observer,
       public remote_cocoa::mojom::NativeWidgetNSWindowHost,
       public DialogObserver,
-      public FocusChangeListener,
-      public ui::internal::InputMethodDelegate,
       public ui::AccessibilityFocusOverrider::Client,
       public ui::LayerDelegate,
       public ui::LayerOwner,
@@ -162,10 +158,6 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
   // Initialize the ui::Compositor and ui::Layer.
   void CreateCompositor(const Widget::InitParams& params);
 
-  // Sets or clears the focus manager to use for tracking focused views.
-  // This does NOT take ownership of |focus_manager|.
-  void SetFocusManager(FocusManager* focus_manager);
-
   // Set the window's title, returning true if the title has changed.
   bool SetWindowTitle(const base::string16& title);
 
@@ -176,9 +168,6 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
   // Return true if the event is handled.
   bool RedispatchKeyEvent(NSEvent* event);
 
-  // See widget.h for documentation.
-  ui::InputMethod* GetInputMethod();
-
   // Geometry of the window, in DIPs.
   const gfx::Rect& GetWindowBoundsInScreen() const {
     return window_bounds_in_screen_;
@@ -373,13 +362,6 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
   // DialogObserver:
   void OnDialogChanged() override;
 
-  // FocusChangeListener:
-  void OnWillChangeFocus(View* focused_before, View* focused_now) override;
-  void OnDidChangeFocus(View* focused_before, View* focused_now) override;
-
-  // ui::internal::InputMethodDelegate:
-  ui::EventDispatchDetails DispatchKeyEventPostIME(ui::KeyEvent* key) override;
-
   // ui::AccessibilityFocusOverrider::Client:
   id GetAccessibilityFocusedUIElement() override;
 
@@ -449,9 +431,7 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
       in_process_view_id_mapping_;
 
   std::unique_ptr<TooltipManager> tooltip_manager_;
-  std::unique_ptr<ui::InputMethod> input_method_;
   std::unique_ptr<TextInputHost> text_input_host_;
-  FocusManager* focus_manager_ = nullptr;  // Weak. Owned by our Widget.
 
   base::string16 window_title_;
 

ui/views/cocoa/native_widget_mac_ns_window_host.mm

@@ -20,7 +20,6 @@
 #include "ui/base/cocoa/remote_accessibility_api.h"
 #include "ui/base/cocoa/remote_layer_api.h"
 #include "ui/base/hit_test.h"
-#include "ui/base/ime/init/input_method_factory.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/compositor/recyclable_compositor_mac.h"
 #include "ui/display/screen.h"
@@ -271,7 +270,6 @@ NativeWidgetMacNSWindowHost::~NativeWidgetMacNSWindowHost() {
   // TODO(ccameron): When all communication from |bridge_| to this goes through
   // the BridgedNativeWidgetHost, this can be replaced with closing that pipe.
   in_process_ns_window_bridge_.reset();
-  SetFocusManager(nullptr);
   DestroyCompositor();
 }
 
@@ -551,26 +549,6 @@ void NativeWidgetMacNSWindowHost::DestroyCompositor() {
       std::move(compositor_));
 }
 
-void NativeWidgetMacNSWindowHost::SetFocusManager(FocusManager* focus_manager) {
-  if (focus_manager_ == focus_manager)
-    return;
-
-  if (focus_manager_) {
-    // Only the destructor can replace the focus manager (and it passes null).
-    DCHECK(!focus_manager);
-    if (View* old_focus = focus_manager_->GetFocusedView())
-      OnDidChangeFocus(old_focus, nullptr);
-    focus_manager_->RemoveFocusChangeListener(this);
-    focus_manager_ = nullptr;
-    return;
-  }
-
-  focus_manager_ = focus_manager;
-  focus_manager_->AddFocusChangeListener(this);
-  if (View* new_focus = focus_manager_->GetFocusedView())
-    OnDidChangeFocus(nullptr, new_focus);
-}
-
 bool NativeWidgetMacNSWindowHost::SetWindowTitle(const base::string16& title) {
   if (window_title_ == title)
     return false;
@@ -597,16 +575,6 @@ bool NativeWidgetMacNSWindowHost::RedispatchKeyEvent(NSEvent* event) {
   return true;
 }
 
-ui::InputMethod* NativeWidgetMacNSWindowHost::GetInputMethod() {
-  if (!input_method_) {
-    input_method_ = ui::CreateInputMethod(this, gfx::kNullAcceleratedWidget);
-    // For now, use always-focused mode on Mac for the input method.
-    // TODO(tapted): Move this to OnWindowKeyStatusChangedTo() and balance.
-    input_method_->OnFocus();
-  }
-  return input_method_.get();
-}
-
 gfx::Rect NativeWidgetMacNSWindowHost::GetRestoredBounds() const {
   if (target_fullscreen_state_ || in_fullscreen_transition_)
     return window_bounds_before_fullscreen_;
@@ -1379,40 +1347,6 @@ void NativeWidgetMacNSWindowHost::OnDialogChanged() {
   GetNSWindowMojo()->ClearTouchBar();
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// NativeWidgetMacNSWindowHost, FocusChangeListener:
-
-void NativeWidgetMacNSWindowHost::OnWillChangeFocus(View* focused_before,
-                                                    View* focused_now) {}
-
-void NativeWidgetMacNSWindowHost::OnDidChangeFocus(View* focused_before,
-                                                   View* focused_now) {
-  ui::InputMethod* input_method =
-      native_widget_mac_->GetWidget()->GetInputMethod();
-  if (!input_method)
-    return;
-
-  ui::TextInputClient* new_text_input_client =
-      input_method->GetTextInputClient();
-  // Sanity check: When focus moves away from the widget (i.e. |focused_now|
-  // is nil), then the textInputClient will be cleared.
-  DCHECK(!!focused_now || !new_text_input_client);
-  text_input_host_->SetTextInputClient(new_text_input_client);
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// NativeWidgetMacNSWindowHost, internal::InputMethodDelegate:
-
-ui::EventDispatchDetails NativeWidgetMacNSWindowHost::DispatchKeyEventPostIME(
-    ui::KeyEvent* key) {
-  DCHECK(focus_manager_);
-  if (!focus_manager_->OnKeyEvent(*key))
-    key->StopPropagation();
-  else
-    native_widget_mac_->GetWidget()->OnKeyEvent(key);
-  return ui::EventDispatchDetails();
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // NativeWidgetMacNSWindowHost, AccessibilityFocusOverrider::Client:
 

ui/views/test/widget_test_mac.mm

@@ -86,7 +86,8 @@ ui::EventSink* WidgetTest::GetEventSink(Widget* widget) {
 ui::internal::InputMethodDelegate* WidgetTest::GetInputMethodDelegateForWidget(
     Widget* widget) {
   return NativeWidgetMacNSWindowHost::GetFromNativeWindow(
-      widget->GetNativeWindow());
+             widget->GetNativeWindow())
+      ->native_widget_mac();
 }
 
 // static

ui/views/widget/native_widget_mac.h

@@ -6,6 +6,7 @@
 #define UI_VIEWS_WIDGET_NATIVE_WIDGET_MAC_H_
 
 #include "base/macros.h"
+#include "ui/base/ime/input_method_delegate.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/views/widget/native_widget_private.h"
@@ -34,7 +35,9 @@ class WidgetTest;
 }
 class NativeWidgetMacNSWindowHost;
 
-class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
+class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate,
+                                     public FocusChangeListener,
+                                     public ui::internal::InputMethodDelegate {
  public:
   explicit NativeWidgetMac(internal::NativeWidgetDelegate* delegate);
   ~NativeWidgetMac() override;
@@ -225,13 +228,22 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
     return ns_window_host_.get();
   }
 
+  // Unregister focus listeners from previous focus manager, and register them
+  // with the |new_focus_manager|. Updates |focus_manager_|.
+  void SetFocusManager(FocusManager* new_focus_manager);
+
+  // FocusChangeListener:
+  void OnWillChangeFocus(View* focused_before, View* focused_now) override;
+  void OnDidChangeFocus(View* focused_before, View* focused_now) override;
+
+  // ui::internal::InputMethodDelegate:
+  ui::EventDispatchDetails DispatchKeyEventPostIME(ui::KeyEvent* key) override;
+
  private:
   friend class test::MockNativeWidgetMac;
   friend class test::HitTestNativeWidgetMac;
   friend class views::test::WidgetTest;
-
   class ZoomFocusMonitor;
-  std::unique_ptr<ZoomFocusMonitor> zoom_focus_monitor_;
 
   internal::NativeWidgetDelegate* delegate_;
   std::unique_ptr<NativeWidgetMacNSWindowHost> ns_window_host_;
@@ -245,6 +257,12 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
 
   Widget::InitParams::Type type_;
 
+  // Weak pointer to the FocusManager with with |zoom_focus_monitor_| and
+  // |ns_window_host_| are registered.
+  FocusManager* focus_manager_ = nullptr;
+  std::unique_ptr<ui::InputMethod> input_method_;
+  std::unique_ptr<ZoomFocusMonitor> zoom_focus_monitor_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeWidgetMac);
 };
 

ui/views/widget/native_widget_mac.mm

@@ -23,6 +23,8 @@
 #import "components/remote_cocoa/app_shim/views_nswindow_delegate.h"
 #import "ui/base/cocoa/constrained_window/constrained_window_animation.h"
 #import "ui/base/cocoa/window_size_constants.h"
+#include "ui/base/ime/init/input_method_factory.h"
+#include "ui/base/ime/input_method.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/gestures/gesture_recognizer_impl_mac.h"
@@ -32,6 +34,7 @@
 #include "ui/native_theme/native_theme_mac.h"
 #import "ui/views/cocoa/drag_drop_client_mac.h"
 #import "ui/views/cocoa/native_widget_mac_ns_window_host.h"
+#include "ui/views/cocoa/text_input_host.h"
 #include "ui/views/widget/drop_helper.h"
 #include "ui/views/widget/widget_aura_utils.h"
 #include "ui/views/widget/widget_delegate.h"
@@ -111,8 +114,7 @@ class NativeWidgetMac::ZoomFocusMonitor : public FocusChangeListener {
 // NativeWidgetMac, public:
 
 NativeWidgetMac::NativeWidgetMac(internal::NativeWidgetDelegate* delegate)
-    : zoom_focus_monitor_(std::make_unique<ZoomFocusMonitor>()),
-      delegate_(delegate),
+    : delegate_(delegate),
       ns_window_host_(new NativeWidgetMacNSWindowHost(this)),
       ownership_(Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET) {}
 
@@ -124,14 +126,13 @@ NativeWidgetMac::~NativeWidgetMac() {
 }
 
 void NativeWidgetMac::WindowDestroying() {
-  if (auto* focus_manager = GetWidget()->GetFocusManager())
-    focus_manager->RemoveFocusChangeListener(zoom_focus_monitor_.get());
   OnWindowDestroying(GetNativeWindow());
   delegate_->OnNativeWidgetDestroying();
 }
 
 void NativeWidgetMac::WindowDestroyed() {
   DCHECK(GetNSWindowMojo());
+  SetFocusManager(nullptr);
   ns_window_host_.reset();
   // |OnNativeWidgetDestroyed| may delete |this| if the object does not own
   // itself.
@@ -214,12 +215,12 @@ void NativeWidgetMac::InitNativeWidget(Widget::InitParams params) {
                                        GetWidget()->GetRootView()->bounds());
   if (auto* focus_manager = GetWidget()->GetFocusManager()) {
     GetNSWindowMojo()->MakeFirstResponder();
-    ns_window_host_->SetFocusManager(focus_manager);
-    // Non-top-level widgets use the the top level widget's focus manager.
-    if (GetWidget() == GetTopLevelWidget())
-      focus_manager->AddFocusChangeListener(zoom_focus_monitor_.get());
+    // Only one ZoomFocusMonitor is needed per FocusManager, so create one only
+    // for top-level widgets.
+    if (GetWidget()->is_top_level())
+      zoom_focus_monitor_ = std::make_unique<ZoomFocusMonitor>();
+    SetFocusManager(focus_manager);
   }
-
   ns_window_host_->CreateCompositor(params);
 
   if (g_init_native_widget_callback)
@@ -330,7 +331,13 @@ bool NativeWidgetMac::HasCapture() const {
 }
 
 ui::InputMethod* NativeWidgetMac::GetInputMethod() {
-  return ns_window_host_ ? ns_window_host_->GetInputMethod() : nullptr;
+  if (!input_method_) {
+    input_method_ = ui::CreateInputMethod(this, gfx::kNullAcceleratedWidget);
+    // For now, use always-focused mode on Mac for the input method.
+    // TODO(tapted): Move this to OnWindowKeyStatusChangedTo() and balance.
+    input_method_->OnFocus();
+  }
+  return input_method_.get();
 }
 
 void NativeWidgetMac::CenterWindow(const gfx::Size& size) {
@@ -815,6 +822,60 @@ NativeWidgetMac::GetInProcessNSWindowBridge() const {
                          : nullptr;
 }
 
+void NativeWidgetMac::SetFocusManager(FocusManager* new_focus_manager) {
+  if (focus_manager_) {
+    if (View* old_focus = focus_manager_->GetFocusedView())
+      OnDidChangeFocus(old_focus, nullptr);
+    focus_manager_->RemoveFocusChangeListener(this);
+    if (zoom_focus_monitor_)
+      focus_manager_->RemoveFocusChangeListener(zoom_focus_monitor_.get());
+  }
+  focus_manager_ = new_focus_manager;
+  if (focus_manager_) {
+    if (View* new_focus = focus_manager_->GetFocusedView())
+      OnDidChangeFocus(nullptr, new_focus);
+    focus_manager_->AddFocusChangeListener(this);
+    if (zoom_focus_monitor_)
+      focus_manager_->AddFocusChangeListener(zoom_focus_monitor_.get());
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NativeWidgetMac, FocusChangeListener:
+
+void NativeWidgetMac::OnWillChangeFocus(View* focused_before,
+                                        View* focused_now) {}
+
+void NativeWidgetMac::OnDidChangeFocus(View* focused_before,
+                                       View* focused_now) {
+  ui::InputMethod* input_method = GetWidget()->GetInputMethod();
+  if (!input_method)
+    return;
+
+  ui::TextInputClient* new_text_input_client =
+      input_method->GetTextInputClient();
+  // Sanity check: When focus moves away from the widget (i.e. |focused_now|
+  // is nil), then the textInputClient will be cleared.
+  DCHECK(!!focused_now || !new_text_input_client);
+  if (ns_window_host_) {
+    ns_window_host_->text_input_host()->SetTextInputClient(
+        new_text_input_client);
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// NativeWidgetMac, internal::InputMethodDelegate:
+
+ui::EventDispatchDetails NativeWidgetMac::DispatchKeyEventPostIME(
+    ui::KeyEvent* key) {
+  DCHECK(focus_manager_);
+  if (!focus_manager_->OnKeyEvent(*key))
+    key->StopPropagation();
+  else
+    GetWidget()->OnKeyEvent(key);
+  return ui::EventDispatchDetails();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Widget, public: