Asynchronous spellchecking on Win and Linux

chrome/renderer/spellchecker/spellcheck.cc

Index: chrome/renderer/spellchecker/spellcheck.cc
diff --git a/chrome/renderer/spellchecker/spellcheck.cc b/chrome/renderer/spellchecker/spellcheck.cc
index a423dff25264a7bda8a2a7b1c511809673448efc..0647799af5166311ac5e9817b75f5dad8cd518c9 100644
--- a/chrome/renderer/spellchecker/spellcheck.cc
+++ b/chrome/renderer/spellchecker/spellcheck.cc
@@ -4,8 +4,10 @@
 
 #include "chrome/renderer/spellchecker/spellcheck.h"
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/metrics/histogram.h"
+#include "base/message_loop.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/render_messages.h"
@@ -13,16 +15,76 @@
 #include "chrome/common/spellcheck_messages.h"
 #include "content/public/renderer/render_thread.h"
 #include "third_party/hunspell/src/hunspell/hunspell.hxx"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebTextCheckingCompletion.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebTextCheckingResult.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebVector.h"
 
 using base::TimeTicks;
 using content::RenderThread;
 
+// A request that checks the spellings of the specified text.
+class SpellCheck::SpellCheckRequest
+    : public base::RefCountedThreadSafe<SpellCheck::SpellCheckRequest> {

[Hironori Bono, 2012/01/27 06:17:41]

I do not think we need a new class for this use-case now. (We can now use
base::Bind() to call a member method safely and it makes this change much
simpler.)

[shinyak, 2012/01/30 06:53:05]

Actually |text| is a temporary object, so the reference may be deleted after
escaping SpellCheck::RequestTextChecking. So I want to have some references to
it. I made a queue in SpellCheck class to achieve it.

+ public:
+  SpellCheckRequest(const string16& text,
+                    int tag,
+                    WebKit::WebTextCheckingCompletion* completion,
+                    SpellCheck* spellcheck)
+      : text_(text),
+        tag_(tag),
+        completion_(completion),
+        spellcheck_(spellcheck) {
+    DCHECK(completion);
+    DCHECK(spellcheck);
+  }
+
+  // Performs spellchecking and calls the callback function.
+  void PerformSpellCheck() {
+#if !defined(OS_MACOSX)
+    std::vector<WebKit::WebTextCheckingResult> results;
+    spellcheck_->SpellCheckParagraph(text_, tag_, &results);

[Hironori Bono, 2012/01/27 06:17:41]

This call surely creates confusing crashes when a renderer calls function after
this SpellCheck object has been deleted. We need to use WeakPtr to access an
object from a task. (As written above, I do not recommend to use a task,
though.)

[shinyak, 2012/01/30 06:53:05]

Done.

+    completion_->didFinishCheckingText(results);
+#else
+    // SpellCheck::SpellCheckParagraph is not implemented on Mac,
+    // so we return without spellchecking. Note that Mac uses its own
+    // spellchecker, this function won't be used.

[Hironori Bono, 2012/01/27 06:17:41]

Use NOTREACHED() and let a renderer crash when someone call this function by
mistake.

[shinyak, 2012/01/30 06:53:05]

Done.

+    completion_->didFinishCheckingText(
+        WebKit::WebVector<WebKit::WebTextCheckingResult>());
+#endif
+  }
+
+  // Calls the callback function without doing spellchecking.
+  // Text is considered as having no misspellings.
+  void CancelSpellCheckRequest() {
+    completion_->didFinishCheckingText(
+        WebKit::WebVector<WebKit::WebTextCheckingResult>());
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<SpellCheckRequest>;
+
+  ~SpellCheckRequest() {
+  }
+
+  // Text to be checked in this task.
+  string16 text_;
+
+  // The document tag provided by WebKit.
+  int tag_;
+
+  // The interface to send the misspelled ranges to WebKit.
+  WebKit::WebTextCheckingCompletion* completion_;
+
+  // The spellchecker shared in this process.
+  SpellCheck* spellcheck_;
+};
+
 SpellCheck::SpellCheck()
     : file_(base::kInvalidPlatformFileValue),
       auto_spell_correct_turned_on_(false),
       is_using_platform_spelling_engine_(false),
-      initialized_(false) {
+      initialized_(false),
+      dictionary_requested_(false) {
   // Wait till we check the first word before doing any initializing.
 }
 
@@ -49,6 +111,8 @@ void SpellCheck::OnInit(IPC::PlatformFileForTransit bdict_file,
   Init(IPC::PlatformFileForTransitToPlatformFile(bdict_file),
        custom_words, language);
   auto_spell_correct_turned_on_ = auto_spell_correct;
+
+  PostDelayedSpellCheckTask();
 }
 
 void SpellCheck::OnWordAdded(const std::string& word) {
@@ -238,6 +302,38 @@ string16 SpellCheck::GetAutoCorrectionWord(const string16& word, int tag) {
   return autocorrect_word;
 }
 
+void SpellCheck::RequestTextChecking(
+    const string16& text,
+    int tag,
+    WebKit::WebTextCheckingCompletion* completion) {
+#if !defined(OS_MACOSX)
+  // Commented out on Mac, because SpellCheckRequest::PerformSpellCheck is not
+  // implemented on Mac. Mac uses its own spellchecker, so this method
+  // will not be used.
+
+  DCHECK(!is_using_platform_spelling_engine_);
+
+  // Clean up the previous request before starting a new request.
+  if (pending_request_) {
+    pending_request_->CancelSpellCheckRequest();
+    pending_request_ = NULL;
+  }
+
+  if (InitializeIfNeeded()) {
+    // We will check this text after we finish loading the hunspell dictionary.
+    // Save parameters so that we can use them when we receive an init message
+    // from the browser process.
+    pending_request_ = new SpellCheckRequest(text, tag, completion, this);
+    return;
+  }
+
+  scoped_refptr<SpellCheckRequest> request(
+      new SpellCheckRequest(text, tag, completion, this));
+  MessageLoop::current()->PostTask(FROM_HERE,
+      base::Bind(&SpellCheckRequest::PerformSpellCheck, request));
+#endif
+}
+
 void SpellCheck::InitializeHunspell() {
   if (hunspell_.get())
     return;
@@ -272,9 +368,11 @@ bool SpellCheck::InitializeIfNeeded() {
   if (is_using_platform_spelling_engine_)
     return false;
 
-  if (!initialized_) {
-    RenderThread::Get()->Send(new SpellCheckHostMsg_RequestDictionary);
-    initialized_ = true;
+  if (!initialized_ && !dictionary_requested_) {
+    // RenderThread will not exist in test.
+    if (RenderThread::Get())
+      RenderThread::Get()->Send(new SpellCheckHostMsg_RequestDictionary);
+    dictionary_requested_ = true;
     return true;
   }
 
@@ -282,7 +380,7 @@ bool SpellCheck::InitializeIfNeeded() {
   if (file_ != base::kInvalidPlatformFileValue)
     InitializeHunspell();
 
-  return false;
+  return !initialized_;
 }
 
 // When called, relays the request to check the spelling to the proper
@@ -314,6 +412,20 @@ bool SpellCheck::CheckSpelling(const string16& word_to_check, int tag) {
   return word_correct;
 }
 
+void SpellCheck::PostDelayedSpellCheckTask() {
+  if (!pending_request_)
+    return;
+
+  if (file_ == base::kInvalidPlatformFileValue) {
+    pending_request_->CancelSpellCheckRequest();
+  } else {
+    MessageLoop::current()->PostTask(FROM_HERE,

[Hironori Bono, 2012/01/27 06:17:41]

This is an old way to post a task. (This code crashes when
MessageLoop::current() does not exist.) We can now use MessageLoopProxy instead
of using MessageLoop directly.

[shinyak, 2012/01/30 06:53:05]

Done.

+        base::Bind(&SpellCheckRequest::PerformSpellCheck, pending_request_));
+  }
+
+  pending_request_ = NULL;
+}
+
 void SpellCheck::FillSuggestionList(
     const string16& wrong_word,
     std::vector<string16>* optional_suggestions) {