Revert "Spellcheck cleanups"

This reverts commit e38327e1.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 718338 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2UzODMyN2UxZDQ5MDE5N2M5ZDdhMDk3Mjg5ZmVlN2FmMzhiMWZhN2UM

Sample Failed Build: https://ci.chromium.org/b/8896048495893501824

Sample Failed Step: compile

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}


Change-Id: Ia990162d2d72055c99af51fc165c52c4aa3fe1c9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929466
Cr-Commit-Position: refs/heads/master@{#718348}

android_webview/browser/aw_content_browser_client.cc

@@ -91,7 +91,6 @@
 #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"
@@ -746,13 +745,8 @@ 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(create_spellcheck_host),
+      base::BindRepeating(&SpellCheckHostImpl::Create),
       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 && enable_spelling_service) {
+    if (use_browser_spellchecker) {
       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) && BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 #include "chrome/browser/spellchecker/spelling_request.h"
 #endif
 
@@ -150,7 +150,8 @@ std::vector<SpellCheckResult> SpellCheckHostChromeImpl::FilterCustomWordResults(
 }
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#if defined(OS_MACOSX) || defined(OS_WIN)
+
 void SpellCheckHostChromeImpl::CheckSpelling(const base::string16& word,
                                              int route_id,
                                              CheckSpellingCallback callback) {
@@ -198,8 +199,7 @@ void SpellCheckHostChromeImpl::CombineResultsForTesting(
     const std::vector<SpellCheckResult>& local_results) {
   SpellingRequest::CombineResults(remote_results, local_results);
 }
-#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
-        //  BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#endif  // defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_MACOSX)
 int SpellCheckHostChromeImpl::ToDocumentTag(int route_id) {

chrome/browser/spellchecker/spell_check_host_chrome_impl.h

@@ -9,11 +9,8 @@
 #include "base/containers/unique_ptr_adapters.h"
 #include "build/build_config.h"
 #include "components/spellcheck/browser/spell_check_host_impl.h"
-#include "mojo/public/cpp/bindings/pending_receiver.h"
-
-#if BUILDFLAG(ENABLE_SPELLING_SERVICE)
 #include "components/spellcheck/browser/spelling_service_client.h"
-#endif
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 
 class SpellcheckCustomDictionary;
 class SpellcheckService;
@@ -66,9 +63,9 @@ class SpellCheckHostChromeImpl : public SpellCheckHostImpl {
       const std::vector<SpellCheckResult>& service_results);
 #endif
 
-#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.
+#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.
   void CheckSpelling(const base::string16& word,
                      int route_id,
                      CheckSpellingCallback callback) override;
@@ -89,8 +86,7 @@ class SpellCheckHostChromeImpl : public SpellCheckHostImpl {
   // All pending requests.
   std::set<std::unique_ptr<SpellingRequest>, base::UniquePtrComparator>
       requests_;
-#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
-        //  BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#endif  // defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_MACOSX)
   int ToDocumentTag(int route_id);
@@ -105,14 +101,10 @@ 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,6 +19,7 @@
 #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,6 +4246,7 @@ 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",
     ]
 
@@ -4253,11 +4254,6 @@ 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,12 +6,14 @@ 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,8 +2,6 @@
 # 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")
 }
@@ -22,15 +20,10 @@ 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,10 +5,18 @@
 #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);
 
@@ -38,7 +46,7 @@ void SpellCheckHostImpl::CallSpellingService(
 }
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 void SpellCheckHostImpl::RequestTextCheck(const base::string16& text,
                                           int route_id,
                                           RequestTextCheckCallback callback) {
@@ -47,14 +55,20 @@ 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);
-  NOTREACHED();
+
+  // This API requires Chrome-only features.
   std::move(callback).Run(false);
 }
 
@@ -62,11 +76,11 @@ void SpellCheckHostImpl::FillSuggestionList(
     const base::string16& word,
     FillSuggestionListCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  NOTREACHED();
-  std::move(callback).Run({});
+
+  // This API requires Chrome-only features.
+  std::move(callback).Run(std::vector<base::string16>());
 }
-#endif  //  BUILDFLAG(USE_BROWSER_SPELLCHECKER) &&
-        //  !BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#endif  // BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 
 #if defined(OS_ANDROID)
 void SpellCheckHostImpl::DisconnectSessionBridge() {

components/spellcheck/browser/spell_check_host_impl.h

@@ -30,6 +30,9 @@ 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;
@@ -40,7 +43,7 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost {
                            CallSpellingServiceCallback callback) override;
 #endif  // BUILDFLAG(USE_RENDERER_SPELLCHECKER)
 
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE)
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void RequestTextCheck(const base::string16& text,
                         int route_id,
                         RequestTextCheckCallback callback) override;

components/spellcheck/renderer/spellcheck_provider.cc

@@ -83,6 +83,7 @@ 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,6 +13,7 @@
 #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"
@@ -31,8 +32,10 @@ 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 blink::WebTextCheckClient {
+class SpellCheckProvider
+    : public content::RenderFrameObserver,
+      public content::RenderFrameObserverTracker<SpellCheckProvider>,
+      public blink::WebTextCheckClient {
  public:
   using WebTextCheckCompletions =
       base::IDMap<std::unique_ptr<blink::WebTextCheckingCompletion>>;

components/spellcheck/spellcheck_build_features.gni

@@ -14,9 +14,5 @@ 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