Refactor code to avoid direct dependency upon ICU: spellcheck_worditerator

chrome/renderer/spellchecker/spellcheck_worditerator.cc

 // Copyright (c) 2012 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.
 
 // Implements a custom word iterator used for our spellchecker.
 
 #include "chrome/renderer/spellchecker/spellcheck_worditerator.h"
 
 #include <map>
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/i18n/break_iterator.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"

[groby-ooo-7-16, 2014/05/08 17:52:00]

No need to include scoped_ptr.h - transitive dependency

[Andrew Hayden (chromium.org), 2014/05/09 15:16:16]

Done.

 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/renderer/spellchecker/spellcheck.h"
 #include "third_party/icu/source/common/unicode/normlzr.h"
 #include "third_party/icu/source/common/unicode/schriter.h"
 #include "third_party/icu/source/common/unicode/uscript.h"
 #include "third_party/icu/source/i18n/unicode/ulocdata.h"
 
 // SpellcheckCharAttribute implementation:
 
@@ skipping 268 matching lines @@
   UScriptCode script_code = uscript_getScript(c, &status);
   if (script_code == script_code_ || script_code == USCRIPT_COMMON)
     output->push_back(c);
   return true;
 }
 
 // SpellcheckWordIterator implementation:
 
 SpellcheckWordIterator::SpellcheckWordIterator()
     : text_(NULL),
-      length_(0),
-      position_(UBRK_DONE),
       attribute_(NULL),
-      iterator_(NULL) {
+      iterator_() {
 }
 
 SpellcheckWordIterator::~SpellcheckWordIterator() {
   Reset();
 }
 
 bool SpellcheckWordIterator::Initialize(
     const SpellcheckCharAttribute* attribute,
     bool allow_contraction) {
   // Create a custom ICU break iterator with empty text used in this object. (We
   // allow setting text later so we can re-use this iterator.)
   DCHECK(attribute);
-  UErrorCode open_status = U_ZERO_ERROR;
-  UParseError parse_status;
-  base::string16 rule(attribute->GetRuleSet(allow_contraction));
+  const base::string16 rule(attribute->GetRuleSet(allow_contraction));
 
   // If there is no rule set, the attributes were invalid.
   if (rule.empty())
     return false;
 
-  iterator_ = ubrk_openRules(rule.c_str(), rule.length(), NULL, 0,
-                             &parse_status, &open_status);
-  if (U_FAILURE(open_status))
+  scoped_ptr<base::i18n::BreakIterator> iterator(
+      new base::i18n::BreakIterator(base::string16(), rule));
+  if (!iterator->Init()) {
+    NOTREACHED() << "failed to open iterator";

[groby-ooo-7-16, 2014/05/08 17:52:00]

Why NOTREACHED? If BreakIterator::Init truly can't fail, just have it there. Or
better, have it not return an error. If it can fail, this is not unreachable.

[Andrew Hayden (chromium.org), 2014/05/09 15:16:16]

I'll add a comment here. It can't fail *in this case* unless the rules fail to
parse, which would mean (since they're hardcoded) that there was a serious code
error in this class.

     return false;
+  }
+  iterator_ = iterator.Pass();
 
   // Set the character attributes so we can normalize the words extracted by
   // this iterator.
   attribute_ = attribute;
   return true;
 }
 
 bool SpellcheckWordIterator::IsInitialized() const {
-  // Return true if we have an ICU custom iterator.
+  // Return true iff we have an iterator.
   return !!iterator_;
 }
 
 bool SpellcheckWordIterator::SetText(const base::char16* text, size_t length) {
   DCHECK(!!iterator_);
 
   // Set the text to be split by this iterator.
-  UErrorCode status = U_ZERO_ERROR;
-  ubrk_setText(iterator_, text, length, &status);
-  if (U_FAILURE(status))
+  if (!iterator_->SetText(text, length)) {
+    NOTREACHED() << "failed to set text";

[groby-ooo-7-16, 2014/05/08 17:52:00]

See above.

[Andrew Hayden (chromium.org), 2014/05/09 15:16:16]

Done.

     return false;
+  }
 
-  // Retrieve the position to the first word in this text. We return false if
-  // this text does not have any words. (For example, The input text consists
-  // only of Chinese characters while the spellchecker language is English.)
-  position_ = ubrk_first(iterator_);
-  if (position_ == UBRK_DONE)
+  // Return false if this text does not have any words. (For example, the
+  // input text consists only of Chinese characters while the spellchecker
+  // language is English.)
+  if (!iterator_->IsValid())
     return false;
 
   text_ = text;
-  length_ = static_cast<int>(length);
   return true;
 }
 
 bool SpellcheckWordIterator::GetNextWord(base::string16* word_string,
                                          int* word_start,
                                          int* word_length) {
-  DCHECK(!!text_ && length_ > 0);
+  DCHECK(!!text_);
 
   word_string->clear();
   *word_start = 0;
   *word_length = 0;
 
-  if (!text_ || position_ == UBRK_DONE)
+  if (!text_ || !iterator_->IsValid())
     return false;
 
   // Find a word that can be checked for spelling. Our rule sets filter out
   // invalid words (e.g. numbers and characters not supported by the
   // spellchecker language) so this ubrk_getRuleStatus() call returns
   // UBRK_WORD_NONE when this iterator finds an invalid word. So, we skip such
   // words until we can find a valid word or reach the end of the input string.
-  int next = ubrk_next(iterator_);
-  while (next != UBRK_DONE) {
-    if (ubrk_getRuleStatus(iterator_) != UBRK_WORD_NONE) {
-      if (Normalize(position_, next - position_, word_string)) {
-        *word_start = position_;
-        *word_length = next - position_;
-        position_ = next;
+  while (iterator_->Advance()) {
+    const size_t start = iterator_->prev();
+    const size_t length = iterator_->pos() - start;
+    if (iterator_->IsWord()) {
+      if (Normalize(start, length, word_string)) {
+        *word_start = start;
+        *word_length = length;
         return true;
       }
     }
-    position_ = next;
-    next = ubrk_next(iterator_);
   }
 
-  // There aren't any more words in the given text. Set the position to
-  // UBRK_DONE to prevent from calling ubrk_next() next time when this function
-  // is called.
-  position_ = UBRK_DONE;
+  // There aren't any more words in the given text.
   return false;
 }
 
 void SpellcheckWordIterator::Reset() {
-  if (iterator_) {
-    ubrk_close(iterator_);
-    iterator_ = NULL;
-  }
+  iterator_.reset(0);

[groby-ooo-7-16, 2014/05/08 17:52:00]

Just reset() - more idiomatic.

[Andrew Hayden (chromium.org), 2014/05/09 15:16:16]

Done.

 }
 
 bool SpellcheckWordIterator::Normalize(int input_start,
                                        int input_length,
                                        base::string16* output_string) const {
   // We use NFKC (Normalization Form, Compatible decomposition, followed by
   // canonical Composition) defined in Unicode Standard Annex #15 to normalize
   // this token because it it the most suitable normalization algorithm for our
   // spellchecker. Nevertheless, it is not a perfect algorithm for our
   // spellchecker and we need manual normalization as well. The normalized
   // text does not have to be NUL-terminated since its characters are copied to
   // string16, which adds a NUL character when we need.
   icu::UnicodeString input(FALSE, &text_[input_start], input_length);
   UErrorCode status = U_ZERO_ERROR;
   icu::UnicodeString output;
   icu::Normalizer::normalize(input, UNORM_NFKC, 0, output, status);
   if (status != U_ZERO_ERROR && status != U_STRING_NOT_TERMINATED_WARNING)
     return false;
 
   // Copy the normalized text to the output.
   icu::StringCharacterIterator it(output);
   for (UChar c = it.first(); c != icu::CharacterIterator::DONE; c = it.next())
     attribute_->OutputChar(c, output_string);
 
   return !output_string->empty();
 }