RemoteMacViews: Move FocusChangeListener and InputMethodDelegate

Move the interfaces FocusChangeListener and InputMethodDelegate from
being implemented by BridgedNativeWidget to being implemented in
BridgedNativeWidgetHostImpl. This has the effect of moving them from
the app shim process to the browser process.

This is in preparaton for removing access to TextInputClient from
BridgedContentView (that operation is delicate). Add a temporary
method on BridgedNativeWidget to set the TextInputClient for
BridgedContentView from BridgedNativeWidgetHostImpl (where we now
receive the OnDidChangeFocus notification).

Bug: 859152
Change-Id: I1f5639448cc9f7b4b00058444ab64914b9193df3
Reviewed-on: https://chromium-review.googlesource.com/1170582Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582238}

ui/views/cocoa/bridged_native_widget.h

@@ -15,7 +15,7 @@
 #include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "ui/accelerated_widget_mac/ca_transaction_observer.h"
 #include "ui/accelerated_widget_mac/display_ca_layer_tree.h"
-#include "ui/base/ime/input_method_delegate.h"
+#include "ui/base/ime/text_input_client.h"
 #import "ui/views/cocoa/bridged_native_widget_owner.h"
 #import "ui/views/cocoa/cocoa_mouse_capture_delegate.h"
 #import "ui/views/focus/focus_manager.h"
@@ -27,10 +27,6 @@
 @class ModalShowAnimationWithLayer;
 @class ViewsNSWindowDelegate;
 
-namespace ui {
-class InputMethod;
-}
-
 namespace views {
 namespace test {
 class BridgedNativeWidgetTestApi;
@@ -48,9 +44,7 @@ class View;
 // NativeWidgetMac to the Cocoa window. Behaves a bit like an aura::Window.
 class VIEWS_EXPORT BridgedNativeWidget
     : public ui::CATransactionCoordinator::PreCommitObserver,
-      public ui::internal::InputMethodDelegate,
       public CocoaMouseCaptureDelegate,
-      public FocusChangeListener,
       public BridgedNativeWidgetOwner,
       public DialogObserver {
  public:
@@ -81,10 +75,6 @@ class VIEWS_EXPORT BridgedNativeWidget
   // Invoked at the end of Widget::Init().
   void OnWidgetInitDone();
 
-  // 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);
-
   // Changes the bounds of the window and the hosted layer if present. The
   // origin is a location in screen coordinates except for "child" windows,
   // which are positioned relative to their parent(). SetBounds() considers a
@@ -164,9 +154,6 @@ class VIEWS_EXPORT BridgedNativeWidget
   // itself.
   void OnShowAnimationComplete();
 
-  // See widget.h for documentation.
-  ui::InputMethod* GetInputMethod();
-
   // The restored bounds will be derived from the current NSWindow frame unless
   // fullscreen or transitioning between fullscreen states.
   gfx::Rect GetRestoredBounds() const;
@@ -224,9 +211,6 @@ class VIEWS_EXPORT BridgedNativeWidget
   bool ShouldWaitInPreCommit() override;
   base::TimeDelta PreCommitTimeout() override;
 
-  // ui::internal::InputMethodDelegate:
-  ui::EventDispatchDetails DispatchKeyEventPostIME(ui::KeyEvent* key) override;
-
   // views::BridgedNativeWidget:
   // TODO(ccameron): Rename BridgedNativeWidget to BridgedNativeWidgetImpl, and
   // make these methods be exposed via the BridgedNativeWidget interface.
@@ -238,6 +222,10 @@ class VIEWS_EXPORT BridgedNativeWidget
   // Specify the content to draw in the NSView.
   void SetCALayerParams(const gfx::CALayerParams& ca_layer_params);
 
+  // TODO(ccameron): This method exists temporarily as we move all direct access
+  // of TextInputClient out of BridgedContentView.
+  void SetTextInputClient(ui::TextInputClient* text_input_client);
+
  private:
   friend class test::BridgedNativeWidgetTestApi;
 
@@ -270,12 +258,6 @@ class VIEWS_EXPORT BridgedNativeWidget
   // Creates and attaches a new instance if not found.
   NSMutableDictionary* GetWindowProperties() const;
 
-  // FocusChangeListener:
-  void OnWillChangeFocus(View* focused_before,
-                         View* focused_now) override;
-  void OnDidChangeFocus(View* focused_before,
-                        View* focused_now) override;
-
   // BridgedNativeWidgetOwner:
   NSWindow* GetNSWindow() override;
   gfx::Vector2d GetChildWindowOffset() const override;
@@ -296,12 +278,10 @@ class VIEWS_EXPORT BridgedNativeWidget
   base::scoped_nsobject<ViewsNSWindowDelegate> window_delegate_;
   base::scoped_nsobject<BridgedContentView> bridged_view_;
   base::scoped_nsobject<ModalShowAnimationWithLayer> show_animation_;
-  std::unique_ptr<ui::InputMethod> input_method_;
   std::unique_ptr<CocoaMouseCapture> mouse_capture_;
   std::unique_ptr<CocoaWindowMoveLoop> window_move_loop_;
   std::unique_ptr<TooltipManager> tooltip_manager_;
   std::unique_ptr<DragDropClientMac> drag_drop_client_;
-  FocusManager* focus_manager_;  // Weak. Owned by our Widget.
   Widget::InitParams::Type widget_type_;
   bool is_translucent_window_ = false;
 

ui/views/cocoa/bridged_native_widget.mm

@@ -19,8 +19,6 @@
 #include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
 #import "ui/base/cocoa/constrained_window/constrained_window_animation.h"
 #include "ui/base/hit_test.h"
-#include "ui/base/ime/input_method.h"
-#include "ui/base/ime/input_method_factory.h"
 #include "ui/base/layout.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/geometry/dip_util.h"
@@ -225,7 +223,6 @@ BridgedNativeWidget::BridgedNativeWidget(BridgedNativeWidgetHost* host,
                                          NativeWidgetMac* parent)
     : host_(host),
       native_widget_mac_(parent),
-      focus_manager_(nullptr),
       widget_type_(Widget::InitParams::TYPE_WINDOW),  // Updated in Init().
       parent_(nullptr),
       target_fullscreen_state_(false),
@@ -247,7 +244,6 @@ BridgedNativeWidget::~BridgedNativeWidget() {
   ui::CATransactionCoordinator::Get().RemovePreCommitObserver(this);
   RemoveOrDestroyChildren();
   DCHECK(child_windows_.empty());
-  SetFocusManager(nullptr);
   SetRootView(nullptr);
 }
 
@@ -371,27 +367,6 @@ void BridgedNativeWidget::OnWidgetInitDone() {
     dialog->AddObserver(this);
 }
 
-void BridgedNativeWidget::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(![window_ delegate]);
-    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);
-}
-
 void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
   Widget* widget = native_widget_mac_->GetWidget();
   // -[NSWindow contentMinSize] is only checked by Cocoa for user-initiated
@@ -857,16 +832,6 @@ void BridgedNativeWidget::OnShowAnimationComplete() {
   show_animation_.reset();
 }
 
-ui::InputMethod* BridgedNativeWidget::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 BridgedNativeWidget::GetRestoredBounds() const {
   if (target_fullscreen_state_ || in_fullscreen_transition_)
     return bounds_before_fullscreen_;
@@ -1011,19 +976,6 @@ base::TimeDelta BridgedNativeWidget::PreCommitTimeout() {
   return kUIPaintTimeout;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// BridgedNativeWidget, internal::InputMethodDelegate:
-
-ui::EventDispatchDetails BridgedNativeWidget::DispatchKeyEventPostIME(
-    ui::KeyEvent* key) {
-  DCHECK(focus_manager_);
-  if (!focus_manager_->OnKeyEvent(*key))
-    key->StopPropagation();
-  else
-    native_widget_mac_->GetWidget()->OnKeyEvent(key);
-  return ui::EventDispatchDetails();
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidget, CocoaMouseCaptureDelegate:
 
@@ -1039,26 +991,6 @@ NSWindow* BridgedNativeWidget::GetWindow() const {
   return window_;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// BridgedNativeWidget, FocusChangeListener:
-
-void BridgedNativeWidget::OnWillChangeFocus(View* focused_before,
-                                            View* focused_now) {
-}
-
-void BridgedNativeWidget::OnDidChangeFocus(View* focused_before,
-                                           View* focused_now) {
-  ui::InputMethod* input_method =
-      native_widget_mac_->GetWidget()->GetInputMethod();
-  if (input_method) {
-    ui::TextInputClient* 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 || !input_client);
-    [bridged_view_ setTextInputClient:input_client];
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // TODO(ccameron): Update class names to:
 // BridgedNativeWidgetImpl, BridgedNativeWidget:
@@ -1086,6 +1018,11 @@ void BridgedNativeWidget::SetCALayerParams(
   }
 }
 
+void BridgedNativeWidget::SetTextInputClient(
+    ui::TextInputClient* text_input_client) {
+  [bridged_view_ setTextInputClient:text_input_client];
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidget, BridgedNativeWidgetOwner:
 

ui/views/cocoa/bridged_native_widget_host_impl.h

@@ -9,14 +9,16 @@
 
 #include "base/macros.h"
 #include "ui/accelerated_widget_mac/accelerated_widget_mac.h"
+#include "ui/base/ime/input_method_delegate.h"
 #include "ui/compositor/layer_owner.h"
 #include "ui/views/cocoa/bridged_native_widget_host.h"
+#include "ui/views/focus/focus_manager.h"
 #include "ui/views/views_export.h"
 #include "ui/views/widget/widget.h"
 
 namespace ui {
 class RecyclableCompositorMac;
-}
+}  // namespace ui
 
 namespace views {
 
@@ -28,6 +30,8 @@ class NativeWidgetMac;
 // APIs, and which may live in an app shim process.
 class VIEWS_EXPORT BridgedNativeWidgetHostImpl
     : public BridgedNativeWidgetHost,
+      public FocusChangeListener,
+      public ui::internal::InputMethodDelegate,
       public ui::LayerDelegate,
       public ui::LayerOwner,
       public ui::AcceleratedWidgetMacNSView {
@@ -47,6 +51,13 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // 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);
+
+  // See widget.h for documentation.
+  ui::InputMethod* GetInputMethod();
+
  private:
   void DestroyCompositor();
 
@@ -69,6 +80,13 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
                  gfx::DecoratedText* decorated_word,
                  gfx::Point* baseline_point) 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::LayerDelegate:
   void OnPaintLayer(const ui::PaintContext& context) override;
   void OnDeviceScaleFactorChanged(float old_device_scale_factor,
@@ -86,6 +104,9 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // instance that may be in another process.
   std::unique_ptr<BridgedNativeWidget> bridge_;
 
+  std::unique_ptr<ui::InputMethod> input_method_;
+  FocusManager* focus_manager_ = nullptr;  // Weak. Owned by our Widget.
+
   std::unique_ptr<ui::RecyclableCompositorMac> compositor_;
 
   DISALLOW_COPY_AND_ASSIGN(BridgedNativeWidgetHostImpl);

ui/views/cocoa/bridged_native_widget_host_impl.mm

@@ -5,6 +5,8 @@
 #include "ui/views/cocoa/bridged_native_widget_host_impl.h"
 
 #include "ui/base/hit_test.h"
+#include "ui/base/ime/input_method.h"
+#include "ui/base/ime/input_method_factory.h"
 #include "ui/compositor/recyclable_compositor_mac.h"
 #include "ui/gfx/geometry/dip_util.h"
 #include "ui/views/cocoa/bridged_native_widget.h"
@@ -25,6 +27,7 @@ BridgedNativeWidgetHostImpl::~BridgedNativeWidgetHostImpl() {
   // TODO(ccameron): When all communication from |bridge_| to this goes through
   // the BridgedNativeWidgetHost, this can be replaced with closing that pipe.
   bridge_.reset();
+  SetFocusManager(nullptr);
   DestroyCompositor();
 }
 
@@ -88,6 +91,36 @@ void BridgedNativeWidgetHostImpl::DestroyCompositor() {
       std::move(compositor_));
 }
 
+void BridgedNativeWidgetHostImpl::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);
+}
+
+ui::InputMethod* BridgedNativeWidgetHostImpl::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();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidgetHostImpl, views::BridgedNativeWidgetHost:
 
@@ -195,6 +228,38 @@ void BridgedNativeWidgetHostImpl::GetWordAt(
   *found_word = true;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// BridgedNativeWidget, FocusChangeListener:
+
+void BridgedNativeWidgetHostImpl::OnWillChangeFocus(View* focused_before,
+                                                    View* focused_now) {}
+
+void BridgedNativeWidgetHostImpl::OnDidChangeFocus(View* focused_before,
+                                                   View* focused_now) {
+  ui::InputMethod* input_method =
+      native_widget_mac_->GetWidget()->GetInputMethod();
+  if (input_method) {
+    ui::TextInputClient* 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 || !input_client);
+    bridge_->SetTextInputClient(input_client);
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// BridgedNativeWidget, internal::InputMethodDelegate:
+
+ui::EventDispatchDetails BridgedNativeWidgetHostImpl::DispatchKeyEventPostIME(
+    ui::KeyEvent* key) {
+  DCHECK(focus_manager_);
+  if (!focus_manager_->OnKeyEvent(*key))
+    key->StopPropagation();
+  else
+    native_widget_mac_->GetWidget()->OnKeyEvent(key);
+  return ui::EventDispatchDetails();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidgetHostImpl, LayerDelegate:
 

ui/views/cocoa/bridged_native_widget_unittest.mm

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

ui/views/test/widget_test_mac.mm

@@ -10,6 +10,7 @@
 #import "base/mac/scoped_objc_class_swizzler.h"
 #include "base/macros.h"
 #import "ui/views/cocoa/bridged_native_widget.h"
+#include "ui/views/cocoa/bridged_native_widget_host_impl.h"
 #include "ui/views/widget/native_widget_mac.h"
 #include "ui/views/widget/root_view.h"
 
@@ -81,7 +82,9 @@ ui::EventSink* WidgetTest::GetEventSink(Widget* widget) {
 // static
 ui::internal::InputMethodDelegate* WidgetTest::GetInputMethodDelegateForWidget(
     Widget* widget) {
-  return NativeWidgetMac::GetBridgeForNativeWindow(widget->GetNativeWindow());
+  return NativeWidgetMac::GetBridgeForNativeWindow(widget->GetNativeWindow())
+      ->native_widget_mac()
+      ->bridge_host_for_testing();
 }
 
 // static

ui/views/widget/native_widget_mac.h

@@ -19,6 +19,7 @@ namespace views {
 namespace test {
 class HitTestNativeWidgetMac;
 class MockNativeWidgetMac;
+class WidgetTest;
 }
 
 class BridgedNativeWidget;
@@ -162,6 +163,7 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate {
  private:
   friend class test::MockNativeWidgetMac;
   friend class test::HitTestNativeWidgetMac;
+  friend class views::test::WidgetTest;
 
   internal::NativeWidgetDelegate* delegate_;
   std::unique_ptr<BridgedNativeWidgetHostImpl> bridge_host_;

ui/views/widget/native_widget_mac.mm

@@ -144,7 +144,7 @@ void NativeWidgetMac::InitNativeWidget(const Widget::InitParams& params) {
   bridge_host_->SetRootView(GetWidget()->GetRootView());
   if (auto* focus_manager = GetWidget()->GetFocusManager()) {
     [window makeFirstResponder:bridge()->ns_view()];
-    bridge()->SetFocusManager(focus_manager);
+    bridge_host_->SetFocusManager(focus_manager);
   }
 
   bridge_host_->CreateCompositor(params);
@@ -253,7 +253,7 @@ bool NativeWidgetMac::HasCapture() const {
 }
 
 ui::InputMethod* NativeWidgetMac::GetInputMethod() {
-  return bridge() ? bridge()->GetInputMethod() : nullptr;
+  return bridge_host_ ? bridge_host_->GetInputMethod() : nullptr;
 }
 
 void NativeWidgetMac::CenterWindow(const gfx::Size& size) {