Use EnableIf in spellcheck mojom

The SpellCheckHost mojom has some platform-dependent APIs.

This patch uses the EnableIf mojo attribute so that we don't need
to leave dummy API implementations on unsupported platforms.

Bug: 813217
Change-Id: I407ebc6687c404b1583ceeac346a29cb775847ee
Reviewed-on: https://chromium-review.googlesource.com/905579
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537879}

chrome/browser/chrome_site_per_process_browsertest.cc

@@ -799,32 +799,33 @@ class MockSpellCheckHost : spellcheck::mojom::SpellCheckHost {
   // spellcheck::mojom::SpellCheckHost:
   void RequestDictionary() override {}
   void NotifyChecked(const base::string16& word, bool misspelled) override {}
-  void ToggleSpellCheck(bool, bool) override {}
-  void CheckSpelling(const base::string16& word,
-                     int,
-                     CheckSpellingCallback) override {}
-  void FillSuggestionList(const base::string16& word,
-                          FillSuggestionListCallback) override {}
 
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void CallSpellingService(const base::string16& text,
                            CallSpellingServiceCallback callback) override {
-#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     std::move(callback).Run(true, std::vector<SpellCheckResult>());
     TextReceived(text);
-#endif
   }
+#endif
 
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void RequestTextCheck(const base::string16& text,
                         int route_id,
                         RequestTextCheckCallback callback) override {
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     std::move(callback).Run(std::vector<SpellCheckResult>());
     TextReceived(text);
-#endif
   }
 
+  void ToggleSpellCheck(bool, bool) override {}
+  void CheckSpelling(const base::string16& word,
+                     int,
+                     CheckSpellingCallback) override {}
+  void FillSuggestionList(const base::string16& word,
+                          FillSuggestionListCallback) override {}
+#endif
+
   content::RenderProcessHost* process_host_;
   bool text_received_ = false;
   base::string16 text_;

components/spellcheck/browser/spell_check_host_impl.cc

@@ -21,8 +21,7 @@ void SpellCheckHostImpl::Create(
 void SpellCheckHostImpl::RequestDictionary() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // This API either requires Chrome-only features, or is not supported on the
-  // current platform.
+  // This API requires Chrome-only features.
   return;
 }
 
@@ -30,11 +29,11 @@ void SpellCheckHostImpl::NotifyChecked(const base::string16& word,
                                        bool misspelled) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // This API either requires Chrome-only features, or is not supported on the
-  // current platform.
+  // This API requires Chrome-only features.
   return;
 }
 
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 void SpellCheckHostImpl::CallSpellingService(
     const base::string16& text,
     CallSpellingServiceCallback callback) {
@@ -43,11 +42,12 @@ void SpellCheckHostImpl::CallSpellingService(
   if (text.empty())
     mojo::ReportBadMessage(__FUNCTION__);
 
-  // This API either requires Chrome-only features, or is not supported on the
-  // current platform.
+  // This API requires Chrome-only features.
   std::move(callback).Run(false, std::vector<SpellCheckResult>());
 }
+#endif  // !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 void SpellCheckHostImpl::RequestTextCheck(const base::string16& text,
                                           int route_id,
                                           RequestTextCheckCallback callback) {
@@ -59,8 +59,7 @@ void SpellCheckHostImpl::RequestTextCheck(const base::string16& text,
 #if defined(OS_ANDROID)
   session_bridge_.RequestTextCheck(text, std::move(callback));
 #else
-  // This API either requires Chrome-only features, or is not supported on the
-  // current non-Android platform.
+  // This API requires Chrome-only features on the platform.
   std::move(callback).Run(std::vector<SpellCheckResult>());
 #endif
 }
@@ -78,8 +77,7 @@ void SpellCheckHostImpl::CheckSpelling(const base::string16& word,
                                        CheckSpellingCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // This API either requires Chrome-only features, or is not supported on the
-  // current platform.
+  // This API requires Chrome-only features.
   std::move(callback).Run(false);
 }
 
@@ -88,7 +86,7 @@ void SpellCheckHostImpl::FillSuggestionList(
     FillSuggestionListCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // This API either requires Chrome-only features, or is not supported on the
-  // current platform.
+  // This API requires Chrome-only features.
   std::move(callback).Run(std::vector<base::string16>());
 }
+#endif  // BUILDFLAG(USE_BROWSER_SPELLCHECKER)

components/spellcheck/browser/spell_check_host_impl.h

@@ -35,8 +35,13 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost {
   // spellcheck::mojom::SpellCheckHost:
   void RequestDictionary() override;
   void NotifyChecked(const base::string16& word, bool misspelled) override;
+
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void CallSpellingService(const base::string16& text,
                            CallSpellingServiceCallback callback) override;
+#endif  // !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void RequestTextCheck(const base::string16& text,
                         int route_id,
                         RequestTextCheckCallback callback) override;
@@ -46,6 +51,7 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost {
                      CheckSpellingCallback callback) override;
   void FillSuggestionList(const base::string16& word,
                           FillSuggestionListCallback callback) override;
+#endif  // BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 
  private:
 #if defined(OS_ANDROID)

components/spellcheck/common/BUILD.gn

@@ -43,4 +43,11 @@ mojom("interfaces") {
   public_deps = [
     "//mojo/common:common_custom_types",
   ]
+
+  enabled_features = []
+  if (use_browser_spellchecker) {
+    enabled_features += [ "USE_BROWSER_SPELLCHECKER" ]
+  } else {
+    enabled_features += [ "USE_RENDERER_SPELLCHECKER" ]
+  }
 }

components/spellcheck/common/spellcheck.mojom

@@ -44,34 +44,35 @@ interface SpellCheckHost {
 
   // Asks the host to spellcheck the |text| using a remote Spelling server
   // to do the spellchecking. If the remote Spelling server is available,
-  // returns |success| true, and the spellchecked |results|. Note this API
-  // requires a !BUILDFLAG(USE_BROWSER_SPELLCHECKER) build.
+  // returns |success| true, and the spellchecked |results|.
+  [EnableIf=USE_RENDERER_SPELLCHECKER]
   CallSpellingService(mojo_base.mojom.String16 text) =>
       (bool success, array<SpellCheckResult> results);
 
   // Asks the host to spellcheck the |text| using a platform-specific spell
-  // checker, and returns the spellchecked |results|. Note this API requires a
-  // BUILDFLAG(USE_BROWSER_SPELLCHECKER) build.
+  // checker, and returns the spellchecked |results|.
   // TODO(crbug.com/812959): Try not to pass |route_id|. What SpellCheckHost
   // needs is the render frame id, which should be passed in a cleaner way.
+  [EnableIf=USE_BROWSER_SPELLCHECKER]
   RequestTextCheck(mojo_base.mojom.String16 text, int32 route_id) =>
       (array<SpellCheckResult> results);
 
-  // Toggles the enabled state of the platform-specific spell checker. Note this
-  // API is Android-only.
+  // Toggles the enabled state of the platform-specific spell checker.
+  [EnableIf=USE_BROWSER_SPELLCHECKER]
   ToggleSpellCheck(bool enabled, bool checked);
 
   // Checks the correctness of a word with a platform-specific spell checker.
-  // Note this API is Mac-only.
   // TODO(groby): This needs to originate from SpellcheckProvider.
   // TODO(crbug.com/812959): Try not to pass |route_id|. What SpellCheckHost
   // needs is the render frame id, which should be passed in a cleaner way.
-  [Sync] CheckSpelling(mojo_base.mojom.String16 word, int32 route_id)
+  [EnableIf=USE_BROWSER_SPELLCHECKER, Sync]
+  CheckSpelling(mojo_base.mojom.String16 word, int32 route_id)
       => (bool correct);
 
   // Returns a list of suggestions for a given word with a platform-specific
-  // spell checker. Note this API is Mac-only.
-  [Sync] FillSuggestionList(mojo_base.mojom.String16 word) =>
+  // spell checker.
+  [EnableIf=USE_BROWSER_SPELLCHECKER, Sync]
+  FillSuggestionList(mojo_base.mojom.String16 word) =>
       (array<mojo_base.mojom.String16> suggestions);
 };
 

components/spellcheck/renderer/spellcheck_provider_test.cc

@@ -34,14 +34,12 @@ TestingSpellCheckProvider::TestingSpellCheckProvider(
     : SpellCheckProvider(nullptr,
                          new SpellCheck(nullptr, embedder_provider),
                          embedder_provider),
-      spelling_service_call_count_(0),
       binding_(this) {}
 
 TestingSpellCheckProvider::TestingSpellCheckProvider(
     SpellCheck* spellcheck,
     service_manager::LocalInterfaceProvider* embedder_provider)
     : SpellCheckProvider(nullptr, spellcheck, embedder_provider),
-      spelling_service_call_count_(0),
       binding_(this) {}
 
 TestingSpellCheckProvider::~TestingSpellCheckProvider() {
@@ -68,20 +66,16 @@ void TestingSpellCheckProvider::RequestDictionary() {}
 void TestingSpellCheckProvider::NotifyChecked(const base::string16& word,
                                               bool misspelled) {}
 
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 void TestingSpellCheckProvider::CallSpellingService(
     const base::string16& text,
     CallSpellingServiceCallback callback) {
-#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   OnCallSpellingService(text);
   std::move(callback).Run(true, std::vector<SpellCheckResult>());
-#else
-  NOTREACHED();
-#endif
 }
 
 void TestingSpellCheckProvider::OnCallSpellingService(
     const base::string16& text) {
-#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   ++spelling_service_call_count_;
   blink::WebTextCheckingCompletion* completion =
       text_check_completions_.Lookup(last_identifier_);
@@ -98,20 +92,19 @@ void TestingSpellCheckProvider::OnCallSpellingService(
   completion->DidFinishCheckingText(results);
   last_request_ = text;
   last_results_ = results;
-#else
-  NOTREACHED();
-#endif
 }
 
+void TestingSpellCheckProvider::ResetResult() {
+  text_.clear();
+}
+#endif  // !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 void TestingSpellCheckProvider::RequestTextCheck(
     const base::string16& text,
     int,
     RequestTextCheckCallback callback) {
-#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   text_check_requests_.push_back(std::make_pair(text, std::move(callback)));
-#else
-  NOTREACHED();
-#endif
 }
 
 void TestingSpellCheckProvider::ToggleSpellCheck(bool, bool) {
@@ -128,10 +121,7 @@ void TestingSpellCheckProvider::FillSuggestionList(const base::string16&,
                                                    FillSuggestionListCallback) {
   NOTREACHED();
 }
-
-void TestingSpellCheckProvider::ResetResult() {
-  text_.clear();
-}
+#endif  // BUILDFLAG(USE_BROWSER_SPELLCHECKER)
 
 void TestingSpellCheckProvider::SetLastResults(
     const base::string16 last_request,

components/spellcheck/renderer/spellcheck_provider_test.h

@@ -49,30 +49,39 @@ class TestingSpellCheckProvider : public SpellCheckProvider,
   void RequestTextChecking(const base::string16& text,
                            blink::WebTextCheckingCompletion* completion);
 
-  void OnCallSpellingService(const base::string16& text);
-  void ResetResult();
-
   void SetLastResults(
       const base::string16 last_request,
       blink::WebVector<blink::WebTextCheckingResult>& last_results);
   bool SatisfyRequestFromCache(const base::string16& text,
                                blink::WebTextCheckingCompletion* completion);
 
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+  void ResetResult();
+
   // Variables logging CallSpellingService() mojo calls.
   base::string16 text_;
-  size_t spelling_service_call_count_;
+  size_t spelling_service_call_count_ = 0;
+#endif
 
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   // Variables logging RequestTextCheck() mojo calls.
   using RequestTextCheckParams =
       std::pair<base::string16, RequestTextCheckCallback>;
   std::vector<RequestTextCheckParams> text_check_requests_;
+#endif
 
  private:
   // spellcheck::mojom::SpellCheckHost:
   void RequestDictionary() override;
   void NotifyChecked(const base::string16& word, bool misspelled) override;
+
+#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void CallSpellingService(const base::string16& text,
                            CallSpellingServiceCallback callback) override;
+  void OnCallSpellingService(const base::string16& text);
+#endif
+
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   void RequestTextCheck(const base::string16&,
                         int,
                         RequestTextCheckCallback) override;
@@ -83,6 +92,7 @@ class TestingSpellCheckProvider : public SpellCheckProvider,
                      CheckSpellingCallback) override;
   void FillSuggestionList(const base::string16&,
                           FillSuggestionListCallback) override;
+#endif
 
   // Message loop (if needed) to deliver the SpellCheckHost request flow.
   std::unique_ptr<base::MessageLoop> loop_;