RemoteMacViews: Update RWHVMac ownership to prepare for accessibility

Update the lifetime of RenderWidgetHostNSViewBridgeLocal. Previous to
this change there were two lifetime modes
- for in-process instances, RenderWidgetHostNSViewBridgeLocal was
  owned by a scoped_ptr
- for out-of-process instance, RenderWidgetHostNSViewBridgeLocal would
  be owned by its mojo connection to its parent

Change this so that the out-of-process version creates a
RenderWidgetHostNSViewBridgeOwner, which is owned by the mojo
connection, and which owns the RenderWidgetHostNSViewBridgeLocal
via a scoped_ptr.

Move the former ForwardingClientHelper to be part of
RenderWidgetHostNSViewBridgeOwner.

Note that this now mirrors structure and behavior in
views::BridgeFactoryImpl.

Change-Id: I1607df48b8e4ef26feaa180b2b46f49857678f66
Reviewed-on: https://chromium-review.googlesource.com/c/1347576
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610483}

content/browser/BUILD.gn

@@ -2380,7 +2380,6 @@ jumbo_source_set("browser") {
       "renderer_host/render_widget_host_ns_view_bridge_local.h",
       "renderer_host/render_widget_host_ns_view_bridge_local.mm",
       "renderer_host/render_widget_host_ns_view_client_helper.h",
-      "renderer_host/render_widget_host_ns_view_client_helper.mm",
       "renderer_host/render_widget_host_view_cocoa.h",
       "renderer_host/render_widget_host_view_cocoa.mm",
     ]

content/browser/ns_view_bridge_factory_impl.mm

@@ -8,12 +8,107 @@
 
 #include "base/macros.h"
 #include "base/no_destructor.h"
+#include "content/browser/renderer_host/input/web_input_event_builders_mac.h"
 #include "content/browser/renderer_host/render_widget_host_ns_view_bridge_local.h"
+#include "content/browser/renderer_host/render_widget_host_ns_view_client_helper.h"
 #include "content/browser/web_contents/web_contents_ns_view_bridge.h"
+#include "content/common/render_widget_host_ns_view.mojom.h"
+#include "content/public/browser/native_web_keyboard_event.h"
 #include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
 
 namespace content {
 
+namespace {
+
+class RenderWidgetHostNSViewBridgeOwner
+    : public RenderWidgetHostNSViewClientHelper {
+ public:
+  explicit RenderWidgetHostNSViewBridgeOwner(
+      mojom::RenderWidgetHostNSViewClientAssociatedPtr client,
+      mojom::RenderWidgetHostNSViewBridgeAssociatedRequest bridge_request)
+      : client_(std::move(client)) {
+    bridge_ = std::make_unique<RenderWidgetHostNSViewBridgeLocal>(client_.get(),
+                                                                  this);
+    bridge_->BindRequest(std::move(bridge_request));
+    client_.set_connection_error_handler(
+        base::BindOnce(&RenderWidgetHostNSViewBridgeOwner::OnConnectionError,
+                       base::Unretained(this)));
+  }
+
+ private:
+  void OnConnectionError() { delete this; }
+
+  std::unique_ptr<InputEvent> TranslateEvent(
+      const blink::WebInputEvent& web_event) {
+    return std::make_unique<InputEvent>(web_event, ui::LatencyInfo());
+  }
+
+  // RenderWidgetHostNSViewClientHelper implementation.
+  id GetRootBrowserAccessibilityElement() override { return nil; }
+  void ForwardKeyboardEvent(const NativeWebKeyboardEvent& key_event,
+                            const ui::LatencyInfo& latency_info) override {
+    const blink::WebKeyboardEvent* web_event =
+        static_cast<const blink::WebKeyboardEvent*>(&key_event);
+    std::unique_ptr<InputEvent> input_event =
+        std::make_unique<InputEvent>(*web_event, latency_info);
+    client_->ForwardKeyboardEvent(std::move(input_event),
+                                  key_event.skip_in_browser);
+  }
+  void ForwardKeyboardEventWithCommands(
+      const NativeWebKeyboardEvent& key_event,
+      const ui::LatencyInfo& latency_info,
+      const std::vector<EditCommand>& commands) override {
+    const blink::WebKeyboardEvent* web_event =
+        static_cast<const blink::WebKeyboardEvent*>(&key_event);
+    std::unique_ptr<InputEvent> input_event =
+        std::make_unique<InputEvent>(*web_event, latency_info);
+    client_->ForwardKeyboardEventWithCommands(
+        std::move(input_event), key_event.skip_in_browser, commands);
+  }
+  void RouteOrProcessMouseEvent(
+      const blink::WebMouseEvent& web_event) override {
+    client_->RouteOrProcessMouseEvent(TranslateEvent(web_event));
+  }
+  void RouteOrProcessTouchEvent(
+      const blink::WebTouchEvent& web_event) override {
+    client_->RouteOrProcessTouchEvent(TranslateEvent(web_event));
+  }
+  void RouteOrProcessWheelEvent(
+      const blink::WebMouseWheelEvent& web_event) override {
+    client_->RouteOrProcessWheelEvent(TranslateEvent(web_event));
+  }
+  void ForwardMouseEvent(const blink::WebMouseEvent& web_event) override {
+    client_->ForwardMouseEvent(TranslateEvent(web_event));
+  }
+  void ForwardWheelEvent(const blink::WebMouseWheelEvent& web_event) override {
+    client_->ForwardWheelEvent(TranslateEvent(web_event));
+  }
+  void GestureBegin(blink::WebGestureEvent begin_event,
+                    bool is_synthetically_injected) override {
+    // The gesture type is not yet known, but assign a type to avoid
+    // serialization asserts (the type will be stripped on the other side).
+    begin_event.SetType(blink::WebInputEvent::kGestureScrollBegin);
+    client_->GestureBegin(TranslateEvent(begin_event),
+                          is_synthetically_injected);
+  }
+  void GestureUpdate(blink::WebGestureEvent update_event) override {
+    client_->GestureUpdate(TranslateEvent(update_event));
+  }
+  void GestureEnd(blink::WebGestureEvent end_event) override {
+    client_->GestureEnd(TranslateEvent(end_event));
+  }
+  void SmartMagnify(const blink::WebGestureEvent& web_event) override {
+    client_->SmartMagnify(TranslateEvent(web_event));
+  }
+
+  mojom::RenderWidgetHostNSViewClientAssociatedPtr client_;
+  std::unique_ptr<RenderWidgetHostNSViewBridgeLocal> bridge_;
+
+  DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostNSViewBridgeOwner);
+};
+
+}
+
 // static
 NSViewBridgeFactoryImpl* NSViewBridgeFactoryImpl::Get() {
   static base::NoDestructor<NSViewBridgeFactoryImpl> instance;
@@ -39,9 +134,9 @@ void NSViewBridgeFactoryImpl::CreateRenderWidgetHostNSViewBridge(
   mojom::RenderWidgetHostNSViewBridgeAssociatedRequest bridge_request(
       stub_bridge_request.PassHandle());
 
-  // Create a RenderWidgetHostNSViewBridgeLocal. The resulting object will be
+  // Create a RenderWidgetHostNSViewBridgeOwner. The resulting object will be
   // destroyed when its underlying pipe is closed.
-  ignore_result(new RenderWidgetHostNSViewBridgeLocal(
+  ignore_result(new RenderWidgetHostNSViewBridgeOwner(
       std::move(client), std::move(bridge_request)));
 }
 

content/browser/renderer_host/render_widget_host_ns_view_bridge_local.h

@@ -18,29 +18,22 @@
 
 namespace content {
 
-// Bridge to a locally-hosted NSView -- this is always instantiated in the same
-// process as the NSView. The owner of this class may exist in another
-// process. Because the owner may exist in another process, this class must
-// be destroyed explicitly by its Destroy method.
+// Mojo bridge for a RenderWidgetHostViewMac's NSView. This class may be
+// instantiated in the same process as its RenderWidgetHostViewMac, or it may
+// be in a different process.
 class RenderWidgetHostNSViewBridgeLocal
     : public mojom::RenderWidgetHostNSViewBridge,
       public display::DisplayObserver {
  public:
-  // Create a bridge that will directly access its client in the same process
-  // via pointers. This object must be explicitly deleted.
   RenderWidgetHostNSViewBridgeLocal(
       mojom::RenderWidgetHostNSViewClient* client,
       RenderWidgetHostNSViewClientHelper* client_helper);
+  ~RenderWidgetHostNSViewBridgeLocal() override;
 
-  // Create a bridge that will access its client in another process via a mojo
-  // interface. This object will be deleted when |bridge_request|'s connection
-  // closes.
-  RenderWidgetHostNSViewBridgeLocal(
-      mojom::RenderWidgetHostNSViewClientAssociatedPtr client,
+  // Bind to a remote request for a bridge interface.
+  void BindRequest(
       mojom::RenderWidgetHostNSViewBridgeAssociatedRequest bridge_request);
 
-  ~RenderWidgetHostNSViewBridgeLocal() override;
-
   // TODO(ccameron): RenderWidgetHostViewMac and other functions currently use
   // this method to communicate directly with RenderWidgetHostViewCocoa. The
   // goal of this class is to eliminate this direct communication (so this
@@ -75,25 +68,12 @@ class RenderWidgetHostNSViewBridgeLocal
   void UnlockKeyboard() override;
 
  private:
-  void Initialize(mojom::RenderWidgetHostNSViewClient* client,
-                  RenderWidgetHostNSViewClientHelper* client_helper);
-
   bool IsPopup() const { return !!popup_window_; }
 
-  // Called on a mojo connection error, deletes |this|.
-  void OnConnectionError();
-
   // display::DisplayObserver implementation.
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t metrics) override;
 
-  // If the client for |this| is in another process and to be accessed via
-  // mojo, then |remote_client_| and |binding_| maintain this interface, and
-  // |remote_client_helper_| is a wrapper around |remote_client_|.
-  mojom::RenderWidgetHostNSViewClientAssociatedPtr remote_client_;
-  mojo::AssociatedBinding<mojom::RenderWidgetHostNSViewBridge> binding_;
-  std::unique_ptr<RenderWidgetHostNSViewClientHelper> remote_client_helper_;
-
   // The NSView used for input and display.
   base::scoped_nsobject<RenderWidgetHostViewCocoa> cocoa_view_;
 
@@ -112,6 +92,9 @@ class RenderWidgetHostNSViewBridgeLocal
   // Cached copy of the tooltip text, to avoid redundant calls.
   base::string16 tooltip_text_;
 
+  // The binding for this object (only used when remotely instantiated).
+  mojo::AssociatedBinding<mojom::RenderWidgetHostNSViewBridge> binding_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostNSViewBridgeLocal);
 };
 

content/browser/renderer_host/render_widget_host_ns_view_bridge_local.mm

@@ -21,29 +21,7 @@ namespace content {
 RenderWidgetHostNSViewBridgeLocal::RenderWidgetHostNSViewBridgeLocal(
     mojom::RenderWidgetHostNSViewClient* client,
     RenderWidgetHostNSViewClientHelper* client_helper)
-    : binding_(nullptr) {
-  Initialize(client, client_helper);
-}
-
-RenderWidgetHostNSViewBridgeLocal::RenderWidgetHostNSViewBridgeLocal(
-    mojom::RenderWidgetHostNSViewClientAssociatedPtr client,
-    mojom::RenderWidgetHostNSViewBridgeAssociatedRequest bridge_request)
-    : remote_client_(std::move(client)), binding_(this) {
-  binding_.Bind(std::move(bridge_request),
-                ui::WindowResizeHelperMac::Get()->task_runner());
-  // This object will be destroyed when its connection is closed.
-  binding_.set_connection_error_handler(
-      base::BindOnce(&RenderWidgetHostNSViewBridgeLocal::OnConnectionError,
-                     base::Unretained(this)));
-  remote_client_helper_ =
-      RenderWidgetHostNSViewClientHelper::CreateForMojoClient(
-          remote_client_.get());
-  Initialize(remote_client_.get(), remote_client_helper_.get());
-}
-
-void RenderWidgetHostNSViewBridgeLocal::Initialize(
-    mojom::RenderWidgetHostNSViewClient* client,
-    RenderWidgetHostNSViewClientHelper* client_helper) {
+    : binding_(this) {
   display::Screen::GetScreen()->AddObserver(this);
 
   cocoa_view_.reset([[RenderWidgetHostViewCocoa alloc]
@@ -71,8 +49,10 @@ RenderWidgetHostNSViewBridgeLocal::~RenderWidgetHostNSViewBridgeLocal() {
   popup_window_.reset();
 }
 
-void RenderWidgetHostNSViewBridgeLocal::OnConnectionError() {
-  delete this;
+void RenderWidgetHostNSViewBridgeLocal::BindRequest(
+    mojom::RenderWidgetHostNSViewBridgeAssociatedRequest bridge_request) {
+  binding_.Bind(std::move(bridge_request),
+                ui::WindowResizeHelperMac::Get()->task_runner());
 }
 
 RenderWidgetHostViewCocoa*

content/browser/renderer_host/render_widget_host_ns_view_client_helper.h

@@ -26,7 +26,6 @@ namespace mojom {
 class RenderWidgetHostNSViewClient;
 }  // namespace mojom
 
-class BrowserAccessibilityManager;
 struct EditCommand;
 struct NativeWebKeyboardEvent;
 
@@ -38,12 +37,6 @@ struct NativeWebKeyboardEvent;
 // types.
 class RenderWidgetHostNSViewClientHelper {
  public:
-  // Create a RenderWidgetHostNSViewClientHelper that will only implement
-  // functionality through mojo (this is in contrast with an in-process
-  // RenderWidgetHostNSViewClientHelper that would use raw pointer access).
-  static std::unique_ptr<RenderWidgetHostNSViewClientHelper>
-  CreateForMojoClient(content::mojom::RenderWidgetHostNSViewClient* client);
-
   RenderWidgetHostNSViewClientHelper() {}
   virtual ~RenderWidgetHostNSViewClientHelper() {}
 

content/browser/renderer_host/render_widget_host_ns_view_client_helper.mm

-// Copyright 2018 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 "render_widget_host_ns_view_client_helper.h"
-
-#include "content/browser/renderer_host/input/web_input_event_builders_mac.h"
-#include "content/common/render_widget_host_ns_view.mojom.h"
-#include "content/public/browser/native_web_keyboard_event.h"
-
-namespace content {
-
-namespace {
-
-class ForwardingClientHelper : public RenderWidgetHostNSViewClientHelper {
- public:
-  explicit ForwardingClientHelper(mojom::RenderWidgetHostNSViewClient* client)
-      : client_(client) {}
-
- private:
-  std::unique_ptr<InputEvent> TranslateEvent(
-      const blink::WebInputEvent& web_event) {
-    return std::make_unique<InputEvent>(web_event, ui::LatencyInfo());
-  }
-
-  // RenderWidgetHostNSViewClientHelper implementation.
-  id GetRootBrowserAccessibilityElement() override { return nil; }
-  void ForwardKeyboardEvent(const NativeWebKeyboardEvent& key_event,
-                            const ui::LatencyInfo& latency_info) override {
-    const blink::WebKeyboardEvent* web_event =
-        static_cast<const blink::WebKeyboardEvent*>(&key_event);
-    std::unique_ptr<InputEvent> input_event =
-        std::make_unique<InputEvent>(*web_event, latency_info);
-    client_->ForwardKeyboardEvent(std::move(input_event),
-                                  key_event.skip_in_browser);
-  }
-  void ForwardKeyboardEventWithCommands(
-      const NativeWebKeyboardEvent& key_event,
-      const ui::LatencyInfo& latency_info,
-      const std::vector<EditCommand>& commands) override {
-    const blink::WebKeyboardEvent* web_event =
-        static_cast<const blink::WebKeyboardEvent*>(&key_event);
-    std::unique_ptr<InputEvent> input_event =
-        std::make_unique<InputEvent>(*web_event, latency_info);
-    client_->ForwardKeyboardEventWithCommands(
-        std::move(input_event), key_event.skip_in_browser, commands);
-  }
-  void RouteOrProcessMouseEvent(
-      const blink::WebMouseEvent& web_event) override {
-    client_->RouteOrProcessMouseEvent(TranslateEvent(web_event));
-  }
-  void RouteOrProcessTouchEvent(
-      const blink::WebTouchEvent& web_event) override {
-    client_->RouteOrProcessTouchEvent(TranslateEvent(web_event));
-  }
-  void RouteOrProcessWheelEvent(
-      const blink::WebMouseWheelEvent& web_event) override {
-    client_->RouteOrProcessWheelEvent(TranslateEvent(web_event));
-  }
-  void ForwardMouseEvent(const blink::WebMouseEvent& web_event) override {
-    client_->ForwardMouseEvent(TranslateEvent(web_event));
-  }
-  void ForwardWheelEvent(const blink::WebMouseWheelEvent& web_event) override {
-    client_->ForwardWheelEvent(TranslateEvent(web_event));
-  }
-  void GestureBegin(blink::WebGestureEvent begin_event,
-                    bool is_synthetically_injected) override {
-    // The gesture type is not yet known, but assign a type to avoid
-    // serialization asserts (the type will be stripped on the other side).
-    begin_event.SetType(blink::WebInputEvent::kGestureScrollBegin);
-    client_->GestureBegin(TranslateEvent(begin_event),
-                          is_synthetically_injected);
-  }
-  void GestureUpdate(blink::WebGestureEvent update_event) override {
-    client_->GestureUpdate(TranslateEvent(update_event));
-  }
-  void GestureEnd(blink::WebGestureEvent end_event) override {
-    client_->GestureEnd(TranslateEvent(end_event));
-  }
-  void SmartMagnify(const blink::WebGestureEvent& web_event) override {
-    client_->SmartMagnify(TranslateEvent(web_event));
-  }
-
-  mojom::RenderWidgetHostNSViewClient* client_ = nullptr;
-
-  DISALLOW_COPY_AND_ASSIGN(ForwardingClientHelper);
-};
-
-}  // namespace
-
-// static
-std::unique_ptr<RenderWidgetHostNSViewClientHelper>
-RenderWidgetHostNSViewClientHelper::CreateForMojoClient(
-    content::mojom::RenderWidgetHostNSViewClient* client) {
-  return std::make_unique<ForwardingClientHelper>(client);
-}
-
-}  // namespace content

content/browser/renderer_host/render_widget_host_view_cocoa.mm

@@ -52,6 +52,38 @@ using blink::WebTouchEvent;
 
 namespace {
 
+// A dummy RenderWidgetHostNSViewClientHelper implementation which no-ops all
+// functions.
+class DummyClientHelper : public RenderWidgetHostNSViewClientHelper {
+ public:
+  explicit DummyClientHelper() {}
+
+ private:
+  // RenderWidgetHostNSViewClientHelper implementation.
+  id GetRootBrowserAccessibilityElement() override { return nil; }
+  void ForwardKeyboardEvent(const NativeWebKeyboardEvent& key_event,
+                            const ui::LatencyInfo& latency_info) override {}
+  void ForwardKeyboardEventWithCommands(
+      const NativeWebKeyboardEvent& key_event,
+      const ui::LatencyInfo& latency_info,
+      const std::vector<EditCommand>& commands) override {}
+  void RouteOrProcessMouseEvent(
+      const blink::WebMouseEvent& web_event) override {}
+  void RouteOrProcessTouchEvent(
+      const blink::WebTouchEvent& web_event) override {}
+  void RouteOrProcessWheelEvent(
+      const blink::WebMouseWheelEvent& web_event) override {}
+  void ForwardMouseEvent(const blink::WebMouseEvent& web_event) override {}
+  void ForwardWheelEvent(const blink::WebMouseWheelEvent& web_event) override {}
+  void GestureBegin(blink::WebGestureEvent begin_event,
+                    bool is_synthetically_injected) override {}
+  void GestureUpdate(blink::WebGestureEvent update_event) override {}
+  void GestureEnd(blink::WebGestureEvent end_event) override {}
+  void SmartMagnify(const blink::WebGestureEvent& web_event) override {}
+
+  DISALLOW_COPY_AND_ASSIGN(DummyClientHelper);
+};
+
 // Touch bar identifier.
 NSString* const kWebContentTouchBarId = @"web-content";
 
@@ -384,8 +416,7 @@ void ExtractUnderlines(NSAttributedString* string,
   // to forward messages to that client.
   content::mojom::RenderWidgetHostNSViewClientRequest dummyClientRequest =
       mojo::MakeRequest(&dummyClient_);
-  dummyClientHelper_ = RenderWidgetHostNSViewClientHelper::CreateForMojoClient(
-      dummyClient_.get());
+  dummyClientHelper_ = std::make_unique<DummyClientHelper>();
   client_ = dummyClient_.get();
   clientHelper_ = dummyClientHelper_.get();