Implement a request cache and disble retrieving a sub-region.

chrome/renderer/spellchecker/spellcheck_provider.cc

Index: chrome/renderer/spellchecker/spellcheck_provider.cc
===================================================================
--- chrome/renderer/spellchecker/spellcheck_provider.cc	(revision 141385)
+++ chrome/renderer/spellchecker/spellcheck_provider.cc	(working copy)
@@ -103,22 +103,62 @@
       document_tag,
       text));
 #else
-  // Send this text to a browser. A browser checks the user profile and send
-  // this text to the Spelling service only if a user enables this feature.
-  // TODO(hbono) Implement a cache to avoid sending IPC messages.
-  string16 line;
-  int offset = -1;
-  if (!GetRequestLine(text, &line, &offset)) {
+  if (text.isEmpty() || !HasWordCharacters(text, 0)) {
     completion->didCancelCheckingText();
     return;
   }
-
-  last_line_ = line;
+  // Cancel this spellcheck request if the cached text is a substring of the
+  // given text and the given text is the middle of a possible word.
+  string16 request(text);

[Hajime Morrita, 2012/06/11 06:53:21]

I hope this early-return-with-cache part to be a method.
Then it could be easily testable.

[Hironori Bono, 2012/06/11 07:24:14]

Unfortunately, this code is a temporary workaround and I need deeper
investigation about the root cause of Bug 131613. That is, I will re-write this
code totally when I identified the root cause. (I have added a TODO comment
here.)

+  size_t text_length = request.length();
+  size_t last_length = last_request_.length();
+  if (text_length >= last_length &&
+      !request.compare(0, last_length, last_request_)) {
+    if (text_length == last_length || !HasWordCharacters(text, last_length)) {
+      completion->didCancelCheckingText();

[Hajime Morrita, 2012/06/11 06:53:21]

This means that there should be no misspelling, right?
In that case, didFinishCheckingText() with empty result
looks making more sense even though there are no visible difference.

Same can be said to line 107.

[Hironori Bono, 2012/06/11 07:24:14]

This code means we should not change existing markers. For example, when
|last_request_| is "lasy dog" and WebKit spellchecks "lasy dog...", this code
prevents WebKit from removing the misspelled underline under "lasy". On the
other hand, if we call didFinishCheckingText with an empty result, WebKit
removes the misspelled underline under "lasy".

+      return;
+    }
+    int code = 0;
+    int length = static_cast<int>(text_length);
+    U16_PREV(text.data(), 0, length, code);
+    UErrorCode error = U_ZERO_ERROR;
+    if (uscript_getScript(code, &error) != USCRIPT_COMMON) {
+      completion->didCancelCheckingText();
+      return;
+    }
+  }
+  // Create a subset of the cached results and return it if the given text is a
+  // substring of the cached text.
+  if (text_length < last_length &&
+      !last_request_.compare(0, text_length, request)) {
+    size_t result_size = 0;
+    for (size_t i = 0; i < last_results_.size(); ++i) {
+      size_t start = last_results_[i].location;
+      size_t end = start + last_results_[i].length;
+      if (start <= text_length && end <= text_length)
+        ++result_size;
+    }
+    if (result_size > 0) {
+      WebKit::WebVector<WebKit::WebTextCheckingResult> results(result_size);
+      for (size_t i = 0; i < result_size; ++i) {
+        results[i].type = last_results_[i].type;
+        results[i].location = last_results_[i].location;
+        results[i].length = last_results_[i].length;
+        results[i].replacement = last_results_[i].replacement;
+      }
+      completion->didFinishCheckingText(results);
+      return;
+    }
+  }
+  // Send this text to a browser. A browser checks the user profile and send
+  // this text to the Spelling service only if a user enables this feature.
+  last_request_.clear();
+  last_results_.assign(WebKit::WebVector<WebKit::WebTextCheckingResult>());
   Send(new SpellCheckHostMsg_CallSpellingService(
       routing_id(),
       text_check_completions_.Add(completion),
-      offset,
-      line));
+      0,
+      request));
 #endif  // !OS_MACOSX
 }
 
@@ -278,59 +318,26 @@
     CreateTextCheckingResults(offset, results, &textcheck_results);
   }
   completion->didFinishCheckingText(textcheck_results);
+
+  // Cache the request and the converted results.
+  last_request_ = line;
+  last_results_.swap(textcheck_results);
 }
 
-bool SpellCheckProvider::HasWordCharacters(const string16& text,
-                                           int index) const {
-  const char16* data = text.c_str();
+bool SpellCheckProvider::HasWordCharacters(
+    const WebKit::WebString& text,
+    int index) const {
+  const char16* data = text.data();
   int length = text.length();
   while (index < length) {
     uint32 code = 0;
     U16_NEXT(data, index, length, code);
-    UErrorCode err = U_ZERO_ERROR;
-    if (uscript_getScript(code, &err) == USCRIPT_LATIN)
+    UErrorCode error = U_ZERO_ERROR;
+    if (uscript_getScript(code, &error) == USCRIPT_LATIN)
       return true;
   }
   return false;
 }
-
-bool SpellCheckProvider::GetRequestLine(const string16& text,
-                                        string16* line,
-                                        int* offset) const {
-  // WebKit sends the line being edited by a user to this class. (It also sends
-  // the previous line when the user is typing its first word.) We send the line
-  // being edited by a user when the input text satisfies all conditions listed
-  // below.
-  // * There is a non-word character at the end of of the input line so this
-  //   class can send a request only when a user finishes typing a word.
-  // * There are word characters in the input line.
-  // * There are word characters in the difference between the input line and
-  //   the previously-spellchecked line.
-  if (text.empty())
-    return false;
-  UErrorCode err = U_ZERO_ERROR;
-  if (uscript_getScript(*(text.rbegin()), &err) != USCRIPT_COMMON)
-    return false;
-  size_t input_offset = text.find('\n');
-  string16 input_line;
-  if (input_offset != string16::npos && HasWordCharacters(text, input_offset)) {
-    ++input_offset;
-    *offset = static_cast<int>(input_offset);
-    input_line = text.substr(input_offset);
-  } else {
-    if (!HasWordCharacters(text, 0))
-      return false;
-    *offset = 0;
-    input_line = text;
-  }
-  size_t length = last_line_.length();
-  if (length > 0 && !input_line.compare(0, length, last_line_)) {
-    if (!HasWordCharacters(input_line, static_cast<int>(length)))
-      return false;
-  }
-  line->assign(input_line);
-  return true;
-}
 #endif
 
 #if defined(OS_MACOSX)