Spellcheck cleanups

Introduce a build flag for the remote spelling service, and stop
compiling/instantiating SpellingServiceClient on platforms that
don't use it (Android). Also try to reduce the number of platform
ifdefs in spellcheck code, in favor of buildflag usage. Reduce
the amount of code that's compiled but never used by removing
several SpellCheckHostImpl methods on mac/win.

Also remove a couple of obsolete includes.

Change-Id: I5697a5d3f891f69159d5bba855ee72e11a3ca8be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1927412Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Auto-Submit: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718338}

android_webview/browser/aw_content_browser_client.cc

@@ -91,6 +91,7 @@
 #include "media/mojo/buildflags.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/android/network_library.h"
 #include "net/http/http_util.h"
 #include "net/net_buildflags.h"
@@ -745,8 +746,13 @@ void AwContentBrowserClient::ExposeInterfacesToRenderer(
       base::CreateSingleThreadTaskRunner({BrowserThread::IO}));
 
 #if BUILDFLAG(ENABLE_SPELLCHECK)
+  auto create_spellcheck_host =
+      [](mojo::PendingReceiver<spellcheck::mojom::SpellCheckHost> receiver) {
+        mojo::MakeSelfOwnedReceiver(std::make_unique<SpellCheckHostImpl>(),
+                                    std::move(receiver));
+      };
   registry->AddInterface(
-      base::BindRepeating(&SpellCheckHostImpl::Create),
+      base::BindRepeating(create_spellcheck_host),
       base::CreateSingleThreadTaskRunner({BrowserThread::UI}));
 #endif
 }

chrome/browser/BUILD.gn

@@ -5065,7 +5065,7 @@ jumbo_static_library("browser") {
       "spellchecker/spellcheck_service.h",
     ]
 
-    if (use_browser_spellchecker) {
+    if (use_browser_spellchecker && enable_spelling_service) {
       sources += [
         "spellchecker/spelling_request.cc",
         "spellchecker/spelling_request.h",

chrome/browser/spellchecker/spell_check_host_chrome_impl.cc

@@ -17,7 +17,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && BUILDFLAG(ENABLE_SPELLING_SERVICE)
 #include "chrome/browser/spellchecker/spelling_request.h"
 #endif
 
@@ -150,8 +150,7 @@ std::vector<SpellCheckResult> SpellCheckHostChromeImpl::FilterCustomWordResults(
 }
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if defined(OS_MACOSX) || defined(OS_WIN)
-
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && BUILDFLAG(ENABLE_SPELLING_SERVICE)
 void SpellCheckHostChromeImpl::CheckSpelling(const base::string16& word,
                                              int route_id,
                                              CheckSpellingCallback callback) {
@@ -199,7 +198,8 @@ void SpellCheckHostChromeImpl::CombineResultsForTesting(
     const std::vector<SpellCheckResult>& local_results) {
   SpellingRequest::CombineResults(remote_results, local_results);
 }
-#endif  // defined(OS_MACOSX) || defined(OS_WIN)
+#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
+        //  BUILDFLAG(ENABLE_SPELLING_SERVICE)
 
 #if defined(OS_MACOSX)
 int SpellCheckHostChromeImpl::ToDocumentTag(int route_id) {

chrome/browser/spellchecker/spell_check_host_chrome_impl.h

@@ -9,9 +9,12 @@
 #include "base/containers/unique_ptr_adapters.h"
 #include "build/build_config.h"
 #include "components/spellcheck/browser/spell_check_host_impl.h"
-#include "components/spellcheck/browser/spelling_service_client.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
+#if BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#include "components/spellcheck/browser/spelling_service_client.h"
+#endif
+
 class SpellcheckCustomDictionary;
 class SpellcheckService;
 class SpellingRequest;
@@ -63,9 +66,9 @@ class SpellCheckHostChromeImpl : public SpellCheckHostImpl {
       const std::vector<SpellCheckResult>& service_results);
 #endif
 
-#if defined(OS_MACOSX) || defined(OS_WIN)
-  // Non-Mac and non-Win(i.e., Android) implementations of the following APIs
-  // are in the base class SpellCheckHostImpl.
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && BUILDFLAG(ENABLE_SPELLING_SERVICE)
+  // Implementations of the following APIs for build configs that don't use the
+  // spelling service are in the base class SpellCheckHostImpl.
   void CheckSpelling(const base::string16& word,
                      int route_id,
                      CheckSpellingCallback callback) override;
@@ -86,7 +89,8 @@ class SpellCheckHostChromeImpl : public SpellCheckHostImpl {
   // All pending requests.
   std::set<std::unique_ptr<SpellingRequest>, base::UniquePtrComparator>
       requests_;
-#endif  // defined(OS_MACOSX) || defined(OS_WIN)
+#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
+        //  BUILDFLAG(ENABLE_SPELLING_SERVICE)
 
 #if defined(OS_MACOSX)
   int ToDocumentTag(int route_id);
@@ -101,10 +105,14 @@ class SpellCheckHostChromeImpl : public SpellCheckHostImpl {
   // The process ID of the renderer.
   const int render_process_id_;
 
+#if BUILDFLAG(ENABLE_SPELLING_SERVICE)
   // A JSON-RPC client that calls the remote Spelling service.
   SpellingServiceClient client_;
+#endif
 
+#if BUILDFLAG(USE_RENDERER_SPELLCHECKER)
   base::WeakPtrFactory<SpellCheckHostChromeImpl> weak_factory_{this};
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(SpellCheckHostChromeImpl);
 };

chrome/browser/spellchecker/spellcheck_service.cc

@@ -19,7 +19,6 @@
 #include "build/build_config.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
 #include "chrome/browser/spellchecker/spellcheck_hunspell_dictionary.h"
-#include "chrome/common/pref_names.h"
 #include "components/language/core/browser/pref_names.h"
 #include "components/prefs/pref_member.h"
 #include "components/prefs/pref_service.h"

chrome/test/BUILD.gn

@@ -4246,7 +4246,6 @@ test("unit_tests") {
       "../browser/spellchecker/spell_check_host_chrome_impl_mac_unittest.cc",
       "../browser/spellchecker/spellcheck_custom_dictionary_unittest.cc",
       "../browser/spellchecker/spellcheck_service_unittest.cc",
-      "../browser/spellchecker/spelling_service_client_unittest.cc",
       "../tools/convert_dict/convert_dict_unittest.cc",
     ]
 
@@ -4254,6 +4253,11 @@ test("unit_tests") {
       sources +=
           [ "../browser/spellchecker/spell_check_host_chrome_impl_unittest.cc" ]
     }
+
+    if (enable_spelling_service) {
+      sources +=
+          [ "../browser/spellchecker/spelling_service_client_unittest.cc" ]
+    }
   }
 
   if (enable_extensions) {

components/spellcheck/BUILD.gn

@@ -6,14 +6,12 @@ import("//build/buildflag_header.gni")
 import("//components/spellcheck/spellcheck_build_features.gni")
 
 buildflag_header("buildflags") {
-  # Name this "build" features to avoid confusion with
-  # components/spellcheck/common/spellcheck_features.h which are runtime
-  # features.
   header = "spellcheck_buildflags.h"
   flags = [
     "ENABLE_SPELLCHECK=$enable_spellcheck",
     "USE_BROWSER_SPELLCHECKER=$use_browser_spellchecker",
     "USE_RENDERER_SPELLCHECKER=$use_renderer_spellchecker",
+    "ENABLE_SPELLING_SERVICE=$enable_spelling_service",
     "HAS_SPELLCHECK_PANEL=$has_spellcheck_panel",
   ]
 }

components/spellcheck/browser/BUILD.gn

@@ -2,6 +2,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//components/spellcheck/spellcheck_build_features.gni")
+
 if (is_android) {
   import("//build/config/android/rules.gni")
 }
@@ -20,10 +22,15 @@ source_set("browser") {
     "spellcheck_platform_mac.mm",
     "spellchecker_session_bridge_android.cc",
     "spellchecker_session_bridge_android.h",
-    "spelling_service_client.cc",
-    "spelling_service_client.h",
   ]
 
+  if (enable_spelling_service) {
+    sources += [
+      "spelling_service_client.cc",
+      "spelling_service_client.h",
+    ]
+  }
+
   if (is_win) {
     sources += [ "spellcheck_platform_win.cc" ]
   }

components/spellcheck/browser/spell_check_host_impl.cc

@@ -5,18 +5,10 @@
 #include "components/spellcheck/browser/spell_check_host_impl.h"
 
 #include "content/public/browser/browser_thread.h"
-#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
 SpellCheckHostImpl::SpellCheckHostImpl() = default;
 SpellCheckHostImpl::~SpellCheckHostImpl() = default;
 
-// static
-void SpellCheckHostImpl::Create(
-    mojo::PendingReceiver<spellcheck::mojom::SpellCheckHost> receiver) {
-  mojo::MakeSelfOwnedReceiver(std::make_unique<SpellCheckHostImpl>(),
-                              std::move(receiver));
-}
-
 void SpellCheckHostImpl::RequestDictionary() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -46,7 +38,7 @@ void SpellCheckHostImpl::CallSpellingService(
 }
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE)
 void SpellCheckHostImpl::RequestTextCheck(const base::string16& text,
                                           int route_id,
                                           RequestTextCheckCallback callback) {
@@ -55,20 +47,14 @@ void SpellCheckHostImpl::RequestTextCheck(const base::string16& text,
   if (text.empty())
     mojo::ReportBadMessage(__FUNCTION__);
 
-#if defined(OS_ANDROID)
   session_bridge_.RequestTextCheck(text, std::move(callback));
-#else
-  // This API requires Chrome-only features on the platform.
-  std::move(callback).Run(std::vector<SpellCheckResult>());
-#endif
 }
 
 void SpellCheckHostImpl::CheckSpelling(const base::string16& word,
                                        int route_id,
                                        CheckSpellingCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  // This API requires Chrome-only features.
+  NOTREACHED();
   std::move(callback).Run(false);
 }
 
@@ -76,11 +62,11 @@ void SpellCheckHostImpl::FillSuggestionList(
     const base::string16& word,
     FillSuggestionListCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  // This API requires Chrome-only features.
-  std::move(callback).Run(std::vector<base::string16>());
+  NOTREACHED();
+  std::move(callback).Run({});
 }
-#endif  // BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
+        //  !BUILDFLAG(ENABLE_SPELLING_SERVICE)
 
 #if defined(OS_ANDROID)
 void SpellCheckHostImpl::DisconnectSessionBridge() {

components/spellcheck/browser/spell_check_host_impl.h

@@ -30,9 +30,6 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost {
   SpellCheckHostImpl();
   ~SpellCheckHostImpl() override;
 
-  static void Create(
-      mojo::PendingReceiver<spellcheck::mojom::SpellCheckHost> receiver);
-
  protected:
   // spellcheck::mojom::SpellCheckHost:
   void RequestDictionary() override;
@@ -43,7 +40,7 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost {
                            CallSpellingServiceCallback callback) override;
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE)
   void RequestTextCheck(const base::string16& text,
                         int route_id,
                         RequestTextCheckCallback callback) override;

components/spellcheck/renderer/spellcheck_provider.cc

@@ -83,7 +83,6 @@ SpellCheckProvider::SpellCheckProvider(
     SpellCheck* spellcheck,
     service_manager::LocalInterfaceProvider* embedder_provider)
     : content::RenderFrameObserver(render_frame),
-      content::RenderFrameObserverTracker<SpellCheckProvider>(render_frame),
       spellcheck_(spellcheck),
       embedder_provider_(embedder_provider) {
   DCHECK(spellcheck_);

components/spellcheck/renderer/spellcheck_provider.h

@@ -13,7 +13,6 @@
 #include "components/spellcheck/common/spellcheck.mojom.h"
 #include "components/spellcheck/spellcheck_buildflags.h"
 #include "content/public/renderer/render_frame_observer.h"
-#include "content/public/renderer/render_frame_observer_tracker.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/web/web_text_check_client.h"
@@ -32,10 +31,8 @@ class LocalInterfaceProvider;
 
 // This class deals with asynchronously invoking text spelling and grammar
 // checking services provided by the browser process (host).
-class SpellCheckProvider
-    : public content::RenderFrameObserver,
-      public content::RenderFrameObserverTracker<SpellCheckProvider>,
-      public blink::WebTextCheckClient {
+class SpellCheckProvider : public content::RenderFrameObserver,
+                           public blink::WebTextCheckClient {
  public:
   using WebTextCheckCompletions =
       base::IDMap<std::unique_ptr<blink::WebTextCheckingCompletion>>;

components/spellcheck/spellcheck_build_features.gni

@@ -14,5 +14,9 @@ use_browser_spellchecker = is_android || is_mac || is_win
 # Therefore, include Windows in both build flags.
 use_renderer_spellchecker = !use_browser_spellchecker || is_win
 
+# Whether the enhanced spellcheck service is available on the platform. This is
+# effectively equal to all desktop platforms.
+enable_spelling_service = use_renderer_spellchecker || is_mac
+
 # Only Mac has a spellcheck panel.
 has_spellcheck_panel = is_mac