Convert render process component common Spellcheck IPC to mojo

Add a render process mojo interface exposed to the browser for
receiving process-wide spellcheck control and updates from the
browser process.

Fold the enable IPC into the Initialize() mojo API (the enable
IPC was only ever called right after an Init IPC) by adding an
|enable| argument. Mojo does not support set<> so use vector<>
instead on the mojo APIs. Use a binding set on the server-side
to automate mojo request disconnect and clean-up.

Update the browser test to use mojo instead of IPC in the mock
renderer and add a test for a custom dictionary update. Change
the InitSpellcheck helper: spellcheck->InitForRenderer() isn't
needed (creating a SpellcheckService does that for us).

Bug:714480

Change-Id: I5365c42fc10c7766054c5b80fb366ceaa4d59159
Reviewed-on: https://chromium-review.googlesource.com/487341Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471125}

chrome/browser/chrome_content_renderer_manifest_overlay.json

@@ -4,7 +4,8 @@
     "service_manager:connector": {
       "provides": {
         "browser": [
-          "chrome::mojom::ResourceUsageReporter"
+          "chrome::mojom::ResourceUsageReporter",
+          "spellcheck::mojom::SpellChecker"
         ]
       }
     },

chrome/browser/spellchecker/spellcheck_service.cc

@@ -20,9 +20,8 @@
 #include "components/spellcheck/browser/spellcheck_host_metrics.h"
 #include "components/spellcheck/browser/spellcheck_platform.h"
 #include "components/spellcheck/browser/spelling_service_client.h"
-#include "components/spellcheck/common/spellcheck_bdict_language.h"
+#include "components/spellcheck/common/spellcheck.mojom.h"
 #include "components/spellcheck/common/spellcheck_common.h"
-#include "components/spellcheck/common/spellcheck_messages.h"
 #include "components/spellcheck/spellcheck_build_features.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
@@ -31,7 +30,6 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
-#include "ipc/ipc_platform_file.h"
 
 using content::BrowserThread;
 
@@ -174,27 +172,27 @@ void SpellcheckService::InitForRenderer(content::RenderProcessHost* process) {
   if (SpellcheckServiceFactory::GetForContext(context) != this)
     return;
 
-  PrefService* prefs = user_prefs::UserPrefs::Get(context);
-  std::vector<SpellCheckBDictLanguage> bdict_languages;
+  const PrefService* prefs = user_prefs::UserPrefs::Get(context);
+  std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries;
 
   for (const auto& hunspell_dictionary : hunspell_dictionaries_) {
-    bdict_languages.push_back(SpellCheckBDictLanguage());
-    bdict_languages.back().language = hunspell_dictionary->GetLanguage();
-    bdict_languages.back().file =
-        hunspell_dictionary->GetDictionaryFile().IsValid()
-            ? IPC::GetPlatformFileForTransit(
-                  hunspell_dictionary->GetDictionaryFile().GetPlatformFile(),
-                  false)
-            : IPC::InvalidPlatformFileForTransit();
+    dictionaries.push_back(spellcheck::mojom::SpellCheckBDictLanguage::New(
+        hunspell_dictionary->GetDictionaryFile().Duplicate(),
+        hunspell_dictionary->GetLanguage()));
   }
 
-  bool enabled =
-      prefs->GetBoolean(spellcheck::prefs::kEnableSpellcheck) &&
-      !bdict_languages.empty();
-  process->Send(new SpellCheckMsg_Init(
-      bdict_languages,
-      enabled ? custom_dictionary_->GetWords() : std::set<std::string>()));
-  process->Send(new SpellCheckMsg_EnableSpellCheck(enabled));
+  bool enable = prefs->GetBoolean(spellcheck::prefs::kEnableSpellcheck) &&
+                !dictionaries.empty();
+
+  std::vector<std::string> custom_words;
+  if (enable) {
+    custom_words.assign(custom_dictionary_->GetWords().begin(),
+                        custom_dictionary_->GetWords().end());
+  }
+
+  spellcheck::mojom::SpellCheckerPtr spellchecker;
+  content::BindInterface(process, &spellchecker);
+  spellchecker->Initialize(std::move(dictionaries), custom_words, enable);
 }
 
 SpellCheckHostMetrics* SpellcheckService::GetMetrics() const {
@@ -257,13 +255,20 @@ void SpellcheckService::OnCustomDictionaryLoaded() {
 }
 
 void SpellcheckService::OnCustomDictionaryChanged(
-    const SpellcheckCustomDictionary::Change& dictionary_change) {
-  for (content::RenderProcessHost::iterator i(
-          content::RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->Send(new SpellCheckMsg_CustomDictionaryChanged(
-        dictionary_change.to_add(),
-        dictionary_change.to_remove()));
+    const SpellcheckCustomDictionary::Change& change) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  auto process_hosts(content::RenderProcessHost::AllHostsIterator());
+
+  const std::vector<std::string> additions(change.to_add().begin(),
+                                           change.to_add().end());
+  const std::vector<std::string> deletions(change.to_remove().begin(),
+                                           change.to_remove().end());
+  while (!process_hosts.IsAtEnd()) {
+    spellcheck::mojom::SpellCheckerPtr spellchecker;
+    content::BindInterface(process_hosts.GetCurrentValue(), &spellchecker);
+    spellchecker->CustomDictionaryChanged(additions, deletions);
+    process_hosts.Advance();
   }
 }
 

chrome/browser/spellchecker/spellcheck_service.h

@@ -130,7 +130,7 @@ class SpellcheckService : public KeyedService,
   // SpellcheckCustomDictionary::Observer implementation.
   void OnCustomDictionaryLoaded() override;
   void OnCustomDictionaryChanged(
-      const SpellcheckCustomDictionary::Change& dictionary_change) override;
+      const SpellcheckCustomDictionary::Change& change) override;
 
   // SpellcheckHunspellDictionary::Observer implementation.
   void OnHunspellDictionaryInitialized(const std::string& language) override;

components/spellcheck/common/BUILD.gn

@@ -2,9 +2,10 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//mojo/public/tools/bindings/mojom.gni")
+
 source_set("common") {
   sources = [
-    "spellcheck_bdict_language.h",
     "spellcheck_common.cc",
     "spellcheck_common.h",
     "spellcheck_features.cc",
@@ -18,11 +19,23 @@ source_set("common") {
   ]
 
   public_deps = [
+    ":interfaces",
     "//components/spellcheck:build_features",
   ]
+
   deps = [
     "//base:i18n",
     "//ipc",
     "//third_party/icu",
   ]
 }
+
+mojom("interfaces") {
+  sources = [
+    "spellcheck.mojom",
+  ]
+
+  public_deps = [
+    "//mojo/common:common_custom_types",
+  ]
+}

components/spellcheck/common/OWNERS

 per-file *_messages*.h=set noparent
 per-file *_messages*.h=file://ipc/SECURITY_OWNERS
+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS

components/spellcheck/common/spellcheck.mojom

+// Copyright 2017 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.
+
+module spellcheck.mojom;
+
+import "mojo/common/file.mojom";
+
+// Render process interface exposed to the browser for receiving process-
+// wide spellcheck control and updates from the browser process.
+//
+interface SpellChecker {
+  // Initialize the render process spellchecker. Called after startup and
+  // also in response to a render process RequestDictionary request.
+  Initialize(array<SpellCheckBDictLanguage> dictionaries,
+             array<string> custom_words,
+             bool enable);
+
+  // Custom dictionary words have been added or removed: update the local
+  // custom word list.
+  CustomDictionaryChanged(array<string> words_added,
+                          array<string> words_removed);
+};
+
+struct SpellCheckBDictLanguage {
+  mojo.common.mojom.File? file;
+  string language;
+};

components/spellcheck/common/spellcheck_bdict_language.h

-// 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.
-
-#ifndef COMPONENTS_SPELLCHECK_COMMON_SPELLCHECK_BDICT_LANGUAGE_H_
-#define COMPONENTS_SPELLCHECK_COMMON_SPELLCHECK_BDICT_LANGUAGE_H_
-
-#include <string>
-
-#include "ipc/ipc_platform_file.h"
-
-struct SpellCheckBDictLanguage {
-  IPC::PlatformFileForTransit file;
-  std::string language;
-};
-
-#endif  // COMPONENTS_SPELLCHECK_COMMON_SPELLCHECK_BDICT_LANGUAGE_H_

components/spellcheck/common/spellcheck_messages.h

@@ -7,11 +7,9 @@
 
 #include <stdint.h>
 
-#include "components/spellcheck/common/spellcheck_bdict_language.h"
 #include "components/spellcheck/common/spellcheck_result.h"
 #include "components/spellcheck/spellcheck_build_features.h"
 #include "ipc/ipc_message_macros.h"
-#include "ipc/ipc_platform_file.h"
 
 #if !BUILDFLAG(ENABLE_SPELLCHECK)
 #error "Spellcheck should be enabled"
@@ -28,28 +26,8 @@ IPC_STRUCT_TRAITS_BEGIN(SpellCheckResult)
   IPC_STRUCT_TRAITS_MEMBER(replacement)
 IPC_STRUCT_TRAITS_END()
 
-IPC_STRUCT_TRAITS_BEGIN(SpellCheckBDictLanguage)
-  IPC_STRUCT_TRAITS_MEMBER(file)
-  IPC_STRUCT_TRAITS_MEMBER(language)
-IPC_STRUCT_TRAITS_END()
-
 // Messages sent from the browser to the renderer.
 
-IPC_MESSAGE_CONTROL1(SpellCheckMsg_EnableSpellCheck, bool)
-
-// Passes some initialization params from the browser to the renderer's
-// spellchecker. This can be called directly after startup or in (async)
-// response to a RequestDictionary ViewHost message.
-IPC_MESSAGE_CONTROL2(SpellCheckMsg_Init,
-                     std::vector<SpellCheckBDictLanguage> /* bdict_languages */,
-                     std::set<std::string> /* custom_dict_words */)
-
-// Words have been added and removed in the custom dictionary; update the local
-// custom word list.
-IPC_MESSAGE_CONTROL2(SpellCheckMsg_CustomDictionaryChanged,
-                     std::set<std::string> /* words_added */,
-                     std::set<std::string> /* words_removed */)
-
 #if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 // Sends text-check results from the Spelling service when the service finishes
 // checking text received by a SpellCheckHostMsg_CallSpellingService message.

components/spellcheck/renderer/BUILD.gn

@@ -46,8 +46,10 @@ source_set("renderer") {
   deps = [
     "//base:i18n",
     "//components/spellcheck/common",
+    "//content/public/common",
     "//content/public/renderer",
     "//ipc",
+    "//services/service_manager/public/cpp",
     "//third_party/WebKit/public:blink",
     "//third_party/icu",
   ]

components/spellcheck/renderer/DEPS

 include_rules = [
+  "+content/public/common",
   "+content/public/renderer",
   "+ipc",
-  "+third_party/WebKit/public",
+  "+mojo/public/cpp/bindings",
+  "+services/service_manager/public/cpp",
   "+third_party/hunspell",
+  "+third_party/WebKit/public",
 ]

components/spellcheck/renderer/spellcheck.cc

@@ -20,16 +20,18 @@
 #include "build/build_config.h"
 #include "components/spellcheck/common/spellcheck_common.h"
 #include "components/spellcheck/common/spellcheck_features.h"
-#include "components/spellcheck/common/spellcheck_messages.h"
 #include "components/spellcheck/common/spellcheck_result.h"
 #include "components/spellcheck/common/spellcheck_switches.h"
 #include "components/spellcheck/renderer/spellcheck_language.h"
 #include "components/spellcheck/renderer/spellcheck_provider.h"
 #include "components/spellcheck/spellcheck_build_features.h"
+#include "content/public/common/service_manager_connection.h"
+#include "content/public/common/simple_connection_filter.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_frame_visitor.h"
 #include "content/public/renderer/render_thread.h"
-#include "ipc/ipc_platform_file.h"
+#include "services/service_manager/public/cpp/bind_source_info.h"
+#include "services/service_manager/public/cpp/binder_registry.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
@@ -149,7 +151,22 @@ class SpellCheck::SpellcheckRequest {
 // and as such the SpellCheckProviders will never be notified of different
 // values.
 // TODO(groby): Simplify this.
-SpellCheck::SpellCheck() : spellcheck_enabled_(true) {}
+SpellCheck::SpellCheck() : spellcheck_enabled_(true) {
+  if (!content::ChildThread::Get())
+    return;  // Can be NULL in tests.
+
+  auto* service_manager_connection =
+      content::ChildThread::Get()->GetServiceManagerConnection();
+  DCHECK(service_manager_connection);
+
+  auto registry = base::MakeUnique<service_manager::BinderRegistry>();
+  registry->AddInterface(
+      base::Bind(&SpellCheck::SpellCheckerRequest, base::Unretained(this)),
+      base::ThreadTaskRunnerHandle::Get());
+
+  service_manager_connection->AddConnectionFilter(
+      base::MakeUnique<content::SimpleConnectionFilter>(std::move(registry)));
+}
 
 SpellCheck::~SpellCheck() {
 }
@@ -183,51 +200,46 @@ void SpellCheck::FillSuggestions(
   }
 }
 
-bool SpellCheck::OnControlMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(SpellCheck, message)
-    IPC_MESSAGE_HANDLER(SpellCheckMsg_Init, OnInit)
-    IPC_MESSAGE_HANDLER(SpellCheckMsg_CustomDictionaryChanged,
-                        OnCustomDictionaryChanged)
-    IPC_MESSAGE_HANDLER(SpellCheckMsg_EnableSpellCheck, OnEnableSpellCheck)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
+void SpellCheck::SpellCheckerRequest(
+    const service_manager::BindSourceInfo& source_info,
+    spellcheck::mojom::SpellCheckerRequest request) {
+  spellchecker_bindings_.AddBinding(this, std::move(request));
 }
 
-void SpellCheck::OnInit(
-    const std::vector<SpellCheckBDictLanguage>& bdict_languages,
-    const std::set<std::string>& custom_words) {
+void SpellCheck::Initialize(
+    std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
+    const std::vector<std::string>& custom_words,
+    bool enable) {
   languages_.clear();
-  for (const auto& bdict_language : bdict_languages) {
-    AddSpellcheckLanguage(
-        IPC::PlatformFileForTransitToFile(bdict_language.file),
-        bdict_language.language);
-  }
 
-  custom_dictionary_.Init(custom_words);
+  for (const auto& dictionary : dictionaries)
+    AddSpellcheckLanguage(std::move(dictionary->file), dictionary->language);
+
+  custom_dictionary_.Init(
+      std::set<std::string>(custom_words.begin(), custom_words.end()));
 #if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   PostDelayedSpellCheckTask(pending_request_param_.release());
 #endif
-}
 
-void SpellCheck::OnCustomDictionaryChanged(
-    const std::set<std::string>& words_added,
-    const std::set<std::string>& words_removed) {
-  custom_dictionary_.OnCustomDictionaryChanged(words_added, words_removed);
-  if (words_added.empty())
-    return;
-  DocumentMarkersRemover markersRemover(words_added);
-  content::RenderFrame::ForEach(&markersRemover);
-}
-
-void SpellCheck::OnEnableSpellCheck(bool enable) {
   spellcheck_enabled_ = enable;
   UpdateSpellcheckEnabled updater(enable);
   content::RenderFrame::ForEach(&updater);
 }
 
+void SpellCheck::CustomDictionaryChanged(
+    const std::vector<std::string>& words_added,
+    const std::vector<std::string>& words_removed) {
+  const std::set<std::string> added(words_added.begin(), words_added.end());
+
+  custom_dictionary_.OnCustomDictionaryChanged(
+      added, std::set<std::string>(words_removed.begin(), words_removed.end()));
+  if (added.empty())
+    return;
+
+  DocumentMarkersRemover markersRemover(added);
+  content::RenderFrame::ForEach(&markersRemover);
+}
+
 // TODO(groby): Make sure we always have a spelling engine, even before
 // AddSpellcheckLanguage() is called.
 void SpellCheck::AddSpellcheckLanguage(base::File file,

components/spellcheck/renderer/spellcheck.h

@@ -15,11 +15,12 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "components/spellcheck/common/spellcheck.mojom.h"
 #include "components/spellcheck/renderer/custom_dictionary_engine.h"
 #include "components/spellcheck/spellcheck_build_features.h"
 #include "content/public/renderer/render_thread_observer.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
 
-struct SpellCheckBDictLanguage;
 class SpellcheckLanguage;
 struct SpellCheckResult;
 
@@ -29,8 +30,8 @@ struct WebTextCheckingResult;
 template <typename T> class WebVector;
 }
 
-namespace IPC {
-class Message;
+namespace service_manager {
+struct BindSourceInfo;
 }
 
 // TODO(morrita): Needs reorg with SpellCheckProvider.
@@ -38,7 +39,8 @@ class Message;
 // Shared spellchecking logic/data for a RenderProcess. All RenderViews use
 // this object to perform spellchecking tasks.
 class SpellCheck : public content::RenderThreadObserver,
-                   public base::SupportsWeakPtr<SpellCheck> {
+                   public base::SupportsWeakPtr<SpellCheck>,
+                   public spellcheck::mojom::SpellChecker {
  public:
   // TODO(groby): I wonder if this can be private, non-mac only.
   class SpellcheckRequest;
@@ -121,15 +123,18 @@ class SpellCheck : public content::RenderThreadObserver,
        const std::vector<std::vector<base::string16>>& suggestions_list,
        std::vector<base::string16>* optional_suggestions);
 
-  // RenderThreadObserver implementation:
-   bool OnControlMessageReceived(const IPC::Message& message) override;
+   // Binds requests for the SpellChecker interface.
+   void SpellCheckerRequest(const service_manager::BindSourceInfo& source_info,
+                            spellcheck::mojom::SpellCheckerRequest request);
 
-  // Message handlers.
-   void OnInit(const std::vector<SpellCheckBDictLanguage>& bdict_languages,
-               const std::set<std::string>& custom_words);
-   void OnCustomDictionaryChanged(const std::set<std::string>& words_added,
-                                  const std::set<std::string>& words_removed);
-  void OnEnableSpellCheck(bool enable);
+   // spellcheck::mojom::SpellChecker:
+   void Initialize(
+       std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
+       const std::vector<std::string>& custom_words,
+       bool enable) override;
+   void CustomDictionaryChanged(
+       const std::vector<std::string>& words_added,
+       const std::vector<std::string>& words_removed) override;
 
 #if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   // Posts delayed spellcheck task and clear it if any.
@@ -147,6 +152,9 @@ class SpellCheck : public content::RenderThreadObserver,
   std::unique_ptr<SpellcheckRequest> pending_request_param_;
 #endif
 
+  // Bindings for SpellChecker clients.
+  mojo::BindingSet<spellcheck::mojom::SpellChecker> spellchecker_bindings_;
+
   // A vector of objects used to actually check spelling, one for each enabled
   // language.
   std::vector<std::unique_ptr<SpellcheckLanguage>> languages_;