Implemented typo recognition in Chrome for Android.

chrome/browser/spellchecker/spellchecker_session_bridge_android.cc

+// 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 "chrome/browser/spellchecker/spellchecker_session_bridge_android.h"
+
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "chrome/common/spellcheck_messages.h"
+#include "chrome/common/spellcheck_result.h"
+#include "content/public/browser/render_process_host.h"
+#include "jni/SpellCheckerSessionBridge_jni.h"
+
+SpellCheckerSessionBridge::SpellingRequest::SpellingRequest(

[please use gerrit instead, 2015/08/17 21:29:46]

Match the order of methods in the header and the source file. If I am reading
the header and the source at the same time, it's jarring to scroll one window up
and the second window down. :-\

This must come after "ProcessSpellCheckResults()".

[dylanking, 2015/08/18 01:21:31]

Good point, done.

+    int new_route_id,

[please use gerrit instead, 2015/08/17 21:29:46]

Name your parameters the same in both header and source file. Don't worry,
compilers and readers are smart enough to figure out what ": route_id(route_id)"
means.

[dylanking, 2015/08/18 01:21:31]

Ah, I didn't know the compiler would be ok with that (I've since looked into the
standard). I usually rely on the '_' at the end of member names, but IIRC struct
members don't receive those.

Anyway, changed!

+    int new_identifier,
+    const base::string16& new_text)
+    : route_id(new_route_id), identifier(new_identifier), text(new_text) {}
+
+SpellCheckerSessionBridge::SpellCheckerSessionBridge(int render_process_id)
+    : render_process_id_(render_process_id),
+      java_object_(Java_SpellCheckerSessionBridge_create(
+          base::android::AttachCurrentThread(),
+          reinterpret_cast<intptr_t>(this))) {}
+
+SpellCheckerSessionBridge::~SpellCheckerSessionBridge() {}
+
+// static
+bool SpellCheckerSessionBridge::RegisterJNI(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+void SpellCheckerSessionBridge::RequestTextCheck(int route_id,
+                                                 int identifier,
+                                                 const base::string16& text) {
+  if (java_object_.is_null()) {

[please use gerrit instead, 2015/08/17 21:29:46]

C++ style (unlike Java style) says do not put curly braces around a single line
if statement.

[dylanking, 2015/08/18 01:21:31]

Done.

+    return;
+  }
+
+  // Save incoming requests to run at the end of the currently active request.
+  // If multiple requests arrive during one active request, only the most
+  // recent request will run (the others get overwritten).
+  if (active_request_) {
+    pending_request_.reset(new SpellingRequest(route_id, identifier, text));
+    return;
+  }
+
+  active_request_.reset(new SpellingRequest(route_id, identifier, text));
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_SpellCheckerSessionBridge_requestTextCheck(
+      env, java_object_.obj(),
+      base::android::ConvertUTF16ToJavaString(env, text).obj());
+}
+
+void SpellCheckerSessionBridge::ProcessSpellCheckResults(
+    JNIEnv* env,
+    jobject jobj,
+    jintArray offset_array,
+    jintArray length_array) {
+  std::vector<int> offsets;
+  std::vector<int> lengths;
+
+  base::android::JavaIntArrayToIntVector(env, offset_array, &offsets);
+  base::android::JavaIntArrayToIntVector(env, length_array, &lengths);
+
+  // Convert specific offset/length pairs into SpellCheckResults, which can be
+  // sent to the renderer to be drawn as red underlines.

[please use gerrit instead, 2015/08/17 21:29:46]

It's obvious what this code is doing. Remove this comment.

As a side note, what the renderer will do with the markers is its own business.
:-) On a more serious note, the renderer might make the underlines gray or
change from underline to highlight, for example. Also, "red underline" is the
name of a web server that spellchecks for Chrome. In summary, let's not use "red
underline" phrase in comments.

[dylanking, 2015/08/18 01:21:31]

Removed.

+  std::vector<SpellCheckResult> results;
+  for (size_t i = 0; i < offsets.size(); i++) {
+    results.push_back(
+        SpellCheckResult(SpellCheckResult::SPELLING, offsets[i], lengths[i]));
+  }
+
+  content::RenderProcessHost* sender =
+      content::RenderProcessHost::FromID(render_process_id_);
+
+  if (sender != nullptr) {
+    sender->Send(new SpellCheckMsg_RespondTextCheck(
+        active_request_->route_id, active_request_->identifier,
+        active_request_->text, results));
+  }
+
+  // If any pending requests arrived during the execution of this active
+  // request, the most recently arrived pending request will become the active
+  // request and be run.

[please use gerrit instead, 2015/08/17 21:29:46]

It's clear what the code is doing. Remove this comment.

[dylanking, 2015/08/18 01:21:31]

Removed.

+  active_request_ = pending_request_.Pass();
+  if (active_request_) {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_SpellCheckerSessionBridge_requestTextCheck(
+        env, java_object_.obj(),
+        base::android::ConvertUTF16ToJavaString(env, active_request_->text)
+            .obj());
+  }
+}