Reland "Spellcheck cleanups"

This is a reland of e38327e1

FindIt automatically reverted this CL but it looks like a false positive

Original change's description:
> 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/+/1927412
> Reviewed-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}

TBR=boliu@chromium.org,rouslan@chromium.org

Change-Id: Iffed71cfbe5d512db1a054364598445b00672a94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946825Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720586}

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"
@@ -750,8 +751,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

@@ -5083,7 +5083,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

@@ -4264,7 +4264,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",
     ]
 
@@ -4272,6 +4271,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