Patch 3.1: Refactored spellcheck_message_filter to be generic

chrome/browser/spellchecker/spellcheck_message_filter_platform_android.cc

Index: chrome/browser/spellchecker/spellcheck_message_filter_platform_android.cc
diff --git a/chrome/browser/spellchecker/spellcheck_message_filter_platform_android.cc b/chrome/browser/spellchecker/spellcheck_message_filter_platform_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4365982e481f907c0b88856e0141aa806e7eb8d7
--- /dev/null
+++ b/chrome/browser/spellchecker/spellcheck_message_filter_platform_android.cc
@@ -0,0 +1,172 @@
+// 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.
+
+#include <algorithm>
+#include <functional>
+
+#include "base/barrier_closure.h"
+#include "base/bind.h"
+#include "chrome/browser/spellchecker/feedback_sender.h"
+#include "chrome/browser/spellchecker/spellcheck_factory.h"
+#include "chrome/browser/spellchecker/spellcheck_message_filter_platform.h"

[please use gerrit instead, 2015/07/09 22:21:30]

Please put this #include first in file with an extra newline after it.

[dylanking, 2015/07/09 23:49:58]

Done.

+#include "chrome/browser/spellchecker/spellcheck_platform_mac.h"
+#include "chrome/browser/spellchecker/spellcheck_service.h"
+#include "chrome/browser/spellchecker/spelling_service_client.h"
+#include "chrome/common/spellcheck_messages.h"
+#include "chrome/common/spellcheck_result.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/render_process_host.h"
+
+using content::BrowserThread;
+using content::BrowserContext;
+
+namespace {
+
+bool CompareLocation(const SpellCheckResult& r1,
+                     const SpellCheckResult& r2) {
+}
+
+}  // namespace
+
+class SpellingRequest {

[please use gerrit instead, 2015/07/09 22:21:30]

Please remove this mac implementation specific class.

[dylanking, 2015/07/09 23:49:58]

Done.

+ public:
+  SpellingRequest(SpellingServiceClient* client,
+                  content::BrowserMessageFilter* destination,
+                  int render_process_id);
+
+  void RequestCheck(const base::string16& text,
+                    int route_id,
+                    int identifier,
+                    int document_tag,
+                    const std::vector<SpellCheckMarker>& markers);
+ private:
+  // Request server-side checking.
+  void RequestRemoteCheck(const base::string16& text);
+
+  // Request a check from local spell checker.
+  void RequestLocalCheck(const base::string16& text, int document_tag);
+
+  // Check if all pending requests are done, send reply to render process if so.
+  void OnCheckCompleted();
+
+  // Called when server-side checking is complete.
+  void OnRemoteCheckCompleted(bool success,
+                              const base::string16& text,
+                              const std::vector<SpellCheckResult>& results);
+
+  // Called when local checking is complete.
+  void OnLocalCheckCompleted(const std::vector<SpellCheckResult>& results);
+
+  std::vector<SpellCheckResult> local_results_;
+  std::vector<SpellCheckResult> remote_results_;
+
+  // Barrier closure for completion of both remote and local check.
+  base::Closure completion_barrier_;
+  bool remote_success_;
+
+  SpellingServiceClient* client_;  // Owned by |destination|.
+  content::BrowserMessageFilter* destination_;  // ref-counted.
+  int render_process_id_;
+
+  int route_id_;
+  int identifier_;
+  int document_tag_;
+  std::vector<SpellCheckMarker> markers_;
+};
+
+SpellingRequest::SpellingRequest(SpellingServiceClient* client,
+                                 content::BrowserMessageFilter* destination,
+                                 int render_process_id)
+    : remote_success_(false),
+      client_(client),
+      destination_(destination),
+      render_process_id_(render_process_id),
+      route_id_(-1),
+      identifier_(-1),
+      document_tag_(-1) {
+}
+
+void SpellingRequest::RequestCheck(
+    const base::string16& text,
+    int route_id,
+    int identifier,
+    int document_tag,
+    const std::vector<SpellCheckMarker>& markers) {
+}
+
+void SpellingRequest::RequestRemoteCheck(const base::string16& text) {
+}
+
+void SpellingRequest::RequestLocalCheck(const base::string16& text,
+                                        int document_tag) {
+}
+
+void SpellingRequest::OnCheckCompleted() {
+}
+
+void SpellingRequest::OnRemoteCheckCompleted(
+    bool success,
+    const base::string16& text,
+    const std::vector<SpellCheckResult>& results) {
+}
+
+void SpellingRequest::OnLocalCheckCompleted(
+    const std::vector<SpellCheckResult>& results) {
+}
+
+
+SpellCheckMessageFilterMac::SpellCheckMessageFilterMac(int render_process_id)

[please use gerrit instead, 2015/07/09 22:21:30]

Here and everywhere:

s/SpellCheckMessageFilterMac/SpellCheckMessageFilterPlatform

[dylanking, 2015/07/09 23:49:58]

Done, quite an oversight by me. Realized this right before reading these
comments. Thanks!

+    : BrowserMessageFilter(SpellCheckMsgStart),
+      render_process_id_(render_process_id),
+      client_(new SpellingServiceClient) {
+}
+
+void SpellCheckMessageFilterMac::OverrideThreadForMessage(
+    const IPC::Message& message, BrowserThread::ID* thread) {
+}
+
+bool SpellCheckMessageFilterMac::OnMessageReceived(
+    const IPC::Message& message) {
+}
+
+// static
+void SpellCheckMessageFilterMac::CombineResults(
+    std::vector<SpellCheckResult>* remote_results,
+    const std::vector<SpellCheckResult>& local_results) {
+}
+
+SpellCheckMessageFilterMac::~SpellCheckMessageFilterMac() {}
+
+void SpellCheckMessageFilterMac::OnCheckSpelling(const base::string16& word,
+                                                 int route_id,
+                                                 bool* correct) {
+}
+
+void SpellCheckMessageFilterMac::OnFillSuggestionList(
+    const base::string16& word,
+    std::vector<base::string16>* suggestions) {
+}
+
+void SpellCheckMessageFilterMac::OnShowSpellingPanel(bool show) {
+}
+
+void SpellCheckMessageFilterMac::OnUpdateSpellingPanelWithMisspelledWord(
+    const base::string16& word) {
+}
+
+void SpellCheckMessageFilterMac::OnRequestTextCheck(
+    int route_id,
+    int identifier,
+    const base::string16& text,
+    std::vector<SpellCheckMarker> markers) {
+}
+
+int SpellCheckMessageFilterMac::ToDocumentTag(int route_id) {
+}
+
+// TODO(groby): We are currently not notified of retired tags. We need

[please use gerrit instead, 2015/07/09 22:21:30]

Please remove this TODO.

[dylanking, 2015/07/09 23:49:58]

Done.

+// to track destruction of RenderViewHosts on the browser process side
+// to update our mappings when a document goes away.
+void SpellCheckMessageFilterMac::RetireDocumentTag(int route_id) {
+}

[please use gerrit instead, 2015/07/09 22:21:30]

Please put NOTREACHED() in all of the methods that you think will not make sense
on Android. For example, I am pretty sure that document tags do not make sense
on Android. Therefore, ToDocumentTag() and RetireDocumentTag() should contain
NOTREACHED().

[dylanking, 2015/07/09 23:49:58]

Done for these two functions.  I'm not sure if any others are eligible for this,
but I have my suspicions about some such as the SpellingPanel methods. 

As I proceed with implementation of the spellchecker, I'll be sure to mark
relevant methods as NOTREACHED().