fix: bind spellchecker receivers correctly in the renderer (#22015)

Co-authored-by: Samuel Attard <samuel.r.attard@gmail.com>

filenames.gni

@@ -579,6 +579,8 @@ filenames = {
     "shell/renderer/atom_renderer_client.h",
     "shell/renderer/atom_sandboxed_renderer_client.cc",
     "shell/renderer/atom_sandboxed_renderer_client.h",
+    "shell/renderer/browser_exposed_renderer_interfaces.cc",
+    "shell/renderer/browser_exposed_renderer_interfaces.h",
     "shell/renderer/content_settings_observer.cc",
     "shell/renderer/content_settings_observer.h",
     "shell/renderer/electron_api_service_impl.cc",

shell/renderer/browser_exposed_renderer_interfaces.cc

@@ -0,0 +1,39 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/renderer/browser_exposed_renderer_interfaces.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "build/build_config.h"
+#include "electron/buildflags/buildflags.h"
+#include "mojo/public/cpp/bindings/binder_map.h"
+#include "shell/renderer/renderer_client_base.h"
+
+#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
+#include "components/spellcheck/renderer/spellcheck.h"
+#endif
+
+namespace {
+#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
+void BindSpellChecker(
+    electron::RendererClientBase* client,
+    mojo::PendingReceiver<spellcheck::mojom::SpellChecker> receiver) {
+  if (client->GetSpellCheck())
+    client->GetSpellCheck()->BindReceiver(std::move(receiver));
+}
+#endif
+
+}  // namespace
+
+void ExposeElectronRendererInterfacesToBrowser(
+    electron::RendererClientBase* client,
+    mojo::BinderMap* binders) {
+#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
+  binders->Add(base::BindRepeating(&BindSpellChecker, client),
+               base::SequencedTaskRunnerHandle::Get());
+#endif
+}

shell/renderer/browser_exposed_renderer_interfaces.h

@@ -0,0 +1,22 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_RENDERER_BROWSER_EXPOSED_RENDERER_INTERFACES_H_
+#define SHELL_RENDERER_BROWSER_EXPOSED_RENDERER_INTERFACES_H_
+
+namespace mojo {
+class BinderMap;
+}  // namespace mojo
+
+namespace electron {
+class RendererClientBase;
+}  // namespace electron
+
+class ChromeContentRendererClient;
+
+void ExposeElectronRendererInterfacesToBrowser(
+    electron::RendererClientBase* client,
+    mojo::BinderMap* binders);
+
+#endif  // SHELL_RENDERER_BROWSER_EXPOSED_RENDERER_INTERFACES_H_

shell/renderer/renderer_client_base.cc

@@ -27,6 +27,7 @@
 #include "shell/common/options_switches.h"
 #include "shell/renderer/atom_autofill_agent.h"
 #include "shell/renderer/atom_render_frame_observer.h"
+#include "shell/renderer/browser_exposed_renderer_interfaces.h"
 #include "shell/renderer/content_settings_observer.h"
 #include "shell/renderer/electron_api_service_impl.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h"
@@ -219,6 +220,13 @@ void RendererClientBase::RenderThreadStarted() {
 #endif
 }
 
+void RendererClientBase::ExposeInterfacesToBrowser(mojo::BinderMap* binders) {
+  // NOTE: Do not add binders directly within this method. Instead, modify the
+  // definition of |ExposeElectronRendererInterfacesToBrowser()| to ensure
+  // security review coverage.
+  ExposeElectronRendererInterfacesToBrowser(this, binders);
+}
+
 void RendererClientBase::RenderFrameCreated(
     content::RenderFrame* render_frame) {
 #if defined(TOOLKIT_VIEWS)
@@ -280,15 +288,6 @@ void RendererClientBase::RenderFrameCreated(
 }
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
-void RendererClientBase::BindReceiverOnMainThread(
-    mojo::GenericPendingReceiver receiver) {
-  // TODO(crbug.com/977637): Get rid of the use of BinderRegistry here. This is
-  // only used to bind a spellcheck interface.
-  std::string interface_name = *receiver.interface_name();
-  auto pipe = receiver.PassPipe();
-  registry_.TryBindInterface(interface_name, &pipe);
-}
-
 void RendererClientBase::GetInterface(
     const std::string& interface_name,
     mojo::ScopedMessagePipeHandle interface_pipe) {

shell/renderer/renderer_client_base.h

@@ -52,8 +52,6 @@ class RendererClientBase : public content::ContentRendererClient
   // service_manager::LocalInterfaceProvider implementation.
   void GetInterface(const std::string& name,
                     mojo::ScopedMessagePipeHandle request_handle) override;
-
-  void BindReceiverOnMainThread(mojo::GenericPendingReceiver receiver) override;
 #endif
 
   virtual void DidCreateScriptContext(v8::Handle<v8::Context> context,
@@ -82,12 +80,17 @@ class RendererClientBase : public content::ContentRendererClient
   bool IsWebViewFrame(v8::Handle<v8::Context> context,
                       content::RenderFrame* render_frame) const;
 
+#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
+  SpellCheck* GetSpellCheck() { return spellcheck_.get(); }
+#endif
+
  protected:
   void AddRenderBindings(v8::Isolate* isolate,
                          v8::Local<v8::Object> binding_object);
 
   // content::ContentRendererClient:
   void RenderThreadStarted() override;
+  void ExposeInterfacesToBrowser(mojo::BinderMap* binders) override;
   void RenderFrameCreated(content::RenderFrame*) override;
   bool OverrideCreatePlugin(content::RenderFrame* render_frame,
                             const blink::WebPluginParams& params,
@@ -125,7 +128,6 @@ class RendererClientBase : public content::ContentRendererClient
 
 #if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
   std::unique_ptr<SpellCheck> spellcheck_;
-  service_manager::BinderRegistry registry_;
 #endif
 };