Re-send spelling results after document markers were removed

SpellCheckProvider can cancel spelling requests when WebKit
has discarded the spelling markers and spellcheck results in
the document. This behavior results in some misspellings missing
underlines. The fix is to always send the last spelling results
instead of canceling spellcheck requests.

TEST=Follow these steps:
1. Type "Icland is an icland ".
2. Erase all text.
3. Type "Icland is an icland " again.
The second word should have spelling markers.

BUG=215184


Review URL: https://chromiumcodereview.appspot.com/12716025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@189531 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 50 insertions, 16 deletions

diff --git a/chrome/renderer/spellchecker/spellcheck_provider.cc b/chrome/renderer/spellchecker/spellcheck_provider.cc
index 706b4b9..9ac25e7 100644
--- a/chrome/renderer/spellchecker/spellcheck_provider.cc
+++ b/chrome/renderer/spellchecker/spellcheck_provider.cc
@@ -313,14 +313,17 @@ bool SpellCheckProvider::SatisfyRequestFromCache(
     WebTextCheckingCompletion* completion) {
   size_t last_length = last_request_.length();
 
-  // 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.
+  // Send back the |last_results_| if the |last_request_| is a substring of
+  // |text| and |text| does not have more words to check. Provider cannot cancel
+  // the spellcheck request here, because WebKit might have discarded the
+  // previous spellcheck results and erased the spelling markers in response to
+  // the user editing the text.
   string16 request(text);
   size_t text_length = 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();
+      completion->didFinishCheckingText(last_results_);
       return true;
     }
     int code = 0;
@@ -328,7 +331,7 @@ bool SpellCheckProvider::SatisfyRequestFromCache(
     U16_PREV(text.data(), 0, length, code);
     UErrorCode error = U_ZERO_ERROR;
     if (uscript_getScript(code, &error) != USCRIPT_COMMON) {
-      completion->didCancelCheckingText();
+      completion->didFinishCheckingText(last_results_);
       return true;
     }
   }
diff --git a/chrome/renderer/spellchecker/spellcheck_provider_hunspell_unittest.cc b/chrome/renderer/spellchecker/spellcheck_provider_hunspell_unittest.cc
index 4a26fca..2c27173 100644
--- a/chrome/renderer/spellchecker/spellcheck_provider_hunspell_unittest.cc
+++ b/chrome/renderer/spellchecker/spellcheck_provider_hunspell_unittest.cc
@@ -83,28 +83,31 @@ TEST_F(SpellCheckProviderTest, MultiLineText) {
   EXPECT_EQ(ASCIIToUTF16("First Second\nThird   Fourth."), provider_.text_);
 }
 
-// Tests that the SpellCheckProvider class cancels incoming spellcheck requests
-// when it does not need to handle them.
+// Tests that the SpellCheckProvider class does not send requests to the
+// spelling service when not necessary.
 TEST_F(SpellCheckProviderTest, CancelUnnecessaryRequests) {
   FakeTextCheckingCompletion completion;
   provider_.RequestTextChecking(WebKit::WebString("hello."),
                                 &completion);
   EXPECT_EQ(completion.completion_count_, 1U);
   EXPECT_EQ(completion.cancellation_count_, 0U);
+  EXPECT_EQ(provider_.spelling_service_call_count_, 1U);
 
-  // Test that the SpellCheckProvider class cancels an incoming request with the
-  // text same as above.
+  // Test that the SpellCheckProvider does not send a request with the same text
+  // as above.
   provider_.RequestTextChecking(WebKit::WebString("hello."),
                                 &completion);
   EXPECT_EQ(completion.completion_count_, 2U);
-  EXPECT_EQ(completion.cancellation_count_, 1U);
+  EXPECT_EQ(completion.cancellation_count_, 0U);
+  EXPECT_EQ(provider_.spelling_service_call_count_, 1U);
 
   // Test that the SpellCheckProvider class cancels an incoming request that
   // does not include any words.
   provider_.RequestTextChecking(WebKit::WebString(":-)"),
                                 &completion);
   EXPECT_EQ(completion.completion_count_, 3U);
-  EXPECT_EQ(completion.cancellation_count_, 2U);
+  EXPECT_EQ(completion.cancellation_count_, 1U);
+  EXPECT_EQ(provider_.spelling_service_call_count_, 1U);
 
   // Test that the SpellCheckProvider class sends a request when it receives a
   // Russian word.
@@ -112,7 +115,30 @@ TEST_F(SpellCheckProviderTest, CancelUnnecessaryRequests) {
   provider_.RequestTextChecking(WebKit::WebString(WideToUTF16(kRussianWord)),
                                 &completion);
   EXPECT_EQ(completion.completion_count_, 4U);
-  EXPECT_EQ(completion.cancellation_count_, 2U);
+  EXPECT_EQ(completion.cancellation_count_, 1U);
+  EXPECT_EQ(provider_.spelling_service_call_count_, 2U);
+}
+
+// Tests that the SpellCheckProvider calls didFinishCheckingText() when
+// necessary.
+TEST_F(SpellCheckProviderTest, CompleteNecessaryRequests) {
+  FakeTextCheckingCompletion completion;
+
+  string16 text = ASCIIToUTF16("Icland is an icland ");
+  provider_.RequestTextChecking(WebKit::WebString(text), &completion);
+  EXPECT_EQ(0U, completion.cancellation_count_) << "Should finish checking \""
+                                                << text << "\"";
+
+  const int kSubstringLength = 18;
+  string16 substring = text.substr(0, kSubstringLength);
+  provider_.RequestTextChecking(WebKit::WebString(substring),
+                                &completion);
+  EXPECT_EQ(0U, completion.cancellation_count_) << "Should finish checking \""
+                                                << substring << "\"";
+
+  provider_.RequestTextChecking(WebKit::WebString(text), &completion);
+  EXPECT_EQ(0U, completion.cancellation_count_) << "Should finish checking \""
+                                                << text << "\"";
 }
 
 }  // namespace
diff --git a/chrome/renderer/spellchecker/spellcheck_provider_test.cc b/chrome/renderer/spellchecker/spellcheck_provider_test.cc
index 22bd6a2..9deafc1 100644
--- a/chrome/renderer/spellchecker/spellcheck_provider_test.cc
+++ b/chrome/renderer/spellchecker/spellcheck_provider_test.cc
@@ -22,7 +22,6 @@ FakeTextCheckingCompletion::~FakeTextCheckingCompletion() {}
 void FakeTextCheckingCompletion::didFinishCheckingText(
     const WebKit::WebVector<WebKit::WebTextCheckingResult>& results) {
   ++completion_count_;
-  last_results_ = results;
 }
 
 void FakeTextCheckingCompletion::didCancelCheckingText() {
@@ -32,7 +31,8 @@ void FakeTextCheckingCompletion::didCancelCheckingText() {
 
 TestingSpellCheckProvider::TestingSpellCheckProvider()
       : SpellCheckProvider(NULL, new MockSpellcheck),
-        offset_(-1) {
+        offset_(-1),
+        spelling_service_call_count_(0) {
 }
 
 TestingSpellCheckProvider::~TestingSpellCheckProvider() {
@@ -67,6 +67,7 @@ void TestingSpellCheckProvider::OnCallSpellingService(int route_id,
 #if defined (OS_MACOSX)
   NOTREACHED();
 #else
+  ++spelling_service_call_count_;
   WebKit::WebTextCheckingCompletion* completion =
       text_check_completions_.Lookup(identifier);
   if (!completion) {
@@ -76,9 +77,13 @@ void TestingSpellCheckProvider::OnCallSpellingService(int route_id,
   offset_ = offset;
   text_.assign(text);
   text_check_completions_.Remove(identifier);
-  completion->didFinishCheckingText(
-      std::vector<WebKit::WebTextCheckingResult>());
+  std::vector<WebKit::WebTextCheckingResult> results;
+  results.push_back(WebKit::WebTextCheckingResult(
+      WebKit::WebTextCheckingTypeSpelling,
+      0, 5, WebKit::WebString("hello")));
+  completion->didFinishCheckingText(results);
   last_request_ = text;
+  last_results_ = results;
 #endif
 }
 
diff --git a/chrome/renderer/spellchecker/spellcheck_provider_test.h b/chrome/renderer/spellchecker/spellcheck_provider_test.h
index 3ceed80..ad018f5a 100644
--- a/chrome/renderer/spellchecker/spellcheck_provider_test.h
+++ b/chrome/renderer/spellchecker/spellcheck_provider_test.h
@@ -31,7 +31,6 @@ class FakeTextCheckingCompletion : public WebKit::WebTextCheckingCompletion {
 
   size_t completion_count_;
   size_t cancellation_count_;
-  WebKit::WebVector<WebKit::WebTextCheckingResult> last_results_;
 };
 
 // Faked test target, which stores sent message for verification.
@@ -50,6 +49,7 @@ class TestingSpellCheckProvider : public SpellCheckProvider {
   int offset_;
   string16 text_;
   std::vector<IPC::Message*> messages_;
+  size_t spelling_service_call_count_;
 };
 
 // SpellCheckProvider test fixture.