Bug 1396980 - Lazily creating nsRange object when misspell word is found. r=kanru draft
authorEvelyn Hung <jj.evelyn@gmail.com>
Thu, 19 Oct 2017 12:18:42 +0800
changeset 685277 1355c1a7f93f969b530938dc29f11336073ac096
parent 680782 c6a2643362a67cdf7a87ac165454fce4b383debb
child 737110 89d1e5a5eb3e6d8685c076bb30ef057d5e103e66
push id85889
push userbmo:ehung@mozilla.com
push dateTue, 24 Oct 2017 10:07:02 +0000
reviewerskanru
bugs1396980
milestone58.0a1
Bug 1396980 - Lazily creating nsRange object when misspell word is found. r=kanru We used to create a nsRange for each word when doing spell check. It's time consuming and waste heap memory. This patch introducing a NodeOffsetRange type to represent a range with begin and end NodeOffset on call stack. We'll then create the nsRange object only for a misspell word. MozReview-Commit-ID: FsU67hfIaSK
extensions/spellcheck/src/mozInlineSpellChecker.cpp
extensions/spellcheck/src/mozInlineSpellWordUtil.cpp
extensions/spellcheck/src/mozInlineSpellWordUtil.h
--- a/extensions/spellcheck/src/mozInlineSpellChecker.cpp
+++ b/extensions/spellcheck/src/mozInlineSpellChecker.cpp
@@ -1497,33 +1497,31 @@ nsresult mozInlineSpellChecker::DoSpellC
   if (!mTextEditor) {
     return NS_ERROR_FAILURE;
   }
 
   int32_t wordsChecked = 0;
   PRTime beginTime = PR_Now();
 
   nsAutoString wordText;
-  RefPtr<nsRange> wordRange;
+  NodeOffsetRange wordNodeOffsetRange;
   bool dontCheckWord;
-  while (NS_SUCCEEDED(aWordUtil.GetNextWord(wordText,
-                                            getter_AddRefs(wordRange),
+  while (NS_SUCCEEDED(aWordUtil.GetNextWord(wordText, &wordNodeOffsetRange,
                                             &dontCheckWord)) &&
-         wordRange) {
+         !wordNodeOffsetRange.Empty()) {
 
     // get the range for the current word.
     nsINode *beginNode;
     nsINode *endNode;
     int32_t beginOffset, endOffset;
 
-    ErrorResult erv;
-    beginNode = wordRange->GetStartContainer(erv);
-    endNode = wordRange->GetEndContainer(erv);
-    beginOffset = wordRange->GetStartOffset(erv);
-    endOffset = wordRange->GetEndOffset(erv);
+    beginNode = wordNodeOffsetRange.Begin().mNode;
+    endNode = wordNodeOffsetRange.End().mNode;
+    beginOffset = wordNodeOffsetRange.Begin().mOffset;
+    endOffset = wordNodeOffsetRange.End().mOffset;
 
     // see if we've done enough words in this round and run out of time.
     if (wordsChecked >= INLINESPELL_MINIMUM_WORDS_BEFORE_TIMEOUT &&
         PR_Now() > PRTime(beginTime + kMaxSpellCheckTimeInUsec)) {
       // stop checking, our time limit has been exceeded.
       #ifdef DEBUG_INLINESPELL
         printf("We have run out of the time, schedule next round.");
       #endif
@@ -1541,16 +1539,17 @@ nsresult mozInlineSpellChecker::DoSpellC
 
 #ifdef DEBUG_INLINESPELL
     printf("->Got word \"%s\"", NS_ConvertUTF16toUTF8(wordText).get());
     if (dontCheckWord)
       printf(" (not checking)");
     printf("\n");
 #endif
 
+    ErrorResult erv;
     // see if there is a spellcheck range that already intersects the word
     // and remove it. We only need to remove old ranges, so don't bother if
     // there were no ranges when we started out.
     if (originalRangeCount > 0) {
       // likewise, if this word is inside new text, we won't bother testing
       if (!aStatus->mCreatedRange ||
           !aStatus->mCreatedRange->IsPointInRange(*beginNode, beginOffset, erv)) {
         nsTArray<RefPtr<nsRange>> ranges;
@@ -1587,25 +1586,29 @@ nsresult mozInlineSpellChecker::DoSpellC
     // check spelling and add to selection if misspelled
     bool isMisspelled;
     aWordUtil.NormalizeWord(wordText);
     nsresult rv = mSpellCheck->CheckCurrentWordNoSuggest(wordText.get(), &isMisspelled);
     if (NS_FAILED(rv))
       continue;
 
     wordsChecked++;
-
     if (isMisspelled) {
       // misspelled words count extra toward the max
-      AddRange(aSpellCheckSelection, wordRange);
-
-      aStatus->mWordCount ++;
-      if (aStatus->mWordCount >= mMaxMisspellingsPerCheck ||
-          SpellCheckSelectionIsFull()) {
-        break;
+      RefPtr<nsRange> wordRange;
+      // If we somehow can't make a range for this word, just ignore it.
+      if(NS_SUCCEEDED(aWordUtil.MakeRange(wordNodeOffsetRange.Begin(),
+                                          wordNodeOffsetRange.End(),
+                                          getter_AddRefs(wordRange)))) {
+        AddRange(aSpellCheckSelection, wordRange);
+        aStatus->mWordCount++;
+        if (aStatus->mWordCount >= mMaxMisspellingsPerCheck ||
+            SpellCheckSelectionIsFull()) {
+          break;
+        }
       }
     }
   }
 
   return NS_OK;
 }
 
 // An RAII helper that calls ChangeNumPendingSpellChecks on destruction.
--- a/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp
+++ b/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp
@@ -226,16 +226,24 @@ mozInlineSpellWordUtil::EnsureWords()
 
 nsresult
 mozInlineSpellWordUtil::MakeRangeForWord(const RealWord& aWord, nsRange** aRange)
 {
   NodeOffset begin = MapSoftTextOffsetToDOMPosition(aWord.mSoftTextOffset, HINT_BEGIN);
   NodeOffset end = MapSoftTextOffsetToDOMPosition(aWord.EndOffset(), HINT_END);
   return MakeRange(begin, end, aRange);
 }
+void
+mozInlineSpellWordUtil::MakeNodeOffsetRangeForWord(const RealWord& aWord,
+                                         NodeOffsetRange* aNodeOffsetRange)
+{
+  NodeOffset begin = MapSoftTextOffsetToDOMPosition(aWord.mSoftTextOffset, HINT_BEGIN);
+  NodeOffset end = MapSoftTextOffsetToDOMPosition(aWord.EndOffset(), HINT_END);
+  *aNodeOffsetRange = NodeOffsetRange(begin, end);
+}
 
 // mozInlineSpellWordUtil::GetRangeForWord
 
 nsresult
 mozInlineSpellWordUtil::GetRangeForWord(nsIDOMNode* aWordNode,
                                         int32_t aWordOffset,
                                         nsRange** aRange)
 {
@@ -284,34 +292,34 @@ NormalizeWord(const nsAString& aInput, i
 
 // mozInlineSpellWordUtil::GetNextWord
 //
 //    FIXME-optimization: we shouldn't have to generate a range every single
 //    time. It would be better if the inline spellchecker didn't require a
 //    range unless the word was misspelled. This may or may not be possible.
 
 nsresult
-mozInlineSpellWordUtil::GetNextWord(nsAString& aText, nsRange** aRange,
+mozInlineSpellWordUtil::GetNextWord(nsAString& aText,
+                                    NodeOffsetRange* aNodeOffsetRange,
                                     bool* aSkipChecking)
 {
 #ifdef DEBUG_SPELLCHECK
   printf("GetNextWord called; mNextWordIndex=%d\n", mNextWordIndex);
 #endif
 
   if (mNextWordIndex < 0 ||
       mNextWordIndex >= int32_t(mRealWords.Length())) {
     mNextWordIndex = -1;
-    *aRange = nullptr;
+    *aNodeOffsetRange = NodeOffsetRange();
     *aSkipChecking = true;
     return NS_OK;
   }
 
   const RealWord& word = mRealWords[mNextWordIndex];
-  nsresult rv = MakeRangeForWord(word, aRange);
-  NS_ENSURE_SUCCESS(rv, rv);
+  MakeNodeOffsetRangeForWord(word, aNodeOffsetRange);
   ++mNextWordIndex;
   *aSkipChecking = !word.mCheckableWord;
   ::NormalizeWord(mSoftText, word.mSoftTextOffset, word.mLength, aText);
 
 #ifdef DEBUG_SPELLCHECK
   printf("GetNextWord returning: %s (skip=%d)\n",
          NS_ConvertUTF16toUTF8(aText).get(), *aSkipChecking);
 #endif
@@ -960,17 +968,17 @@ FindLastNongreaterOffset(const nsTArray<
     // Every mapping had offset greater than aSoftTextOffset.
     MOZ_ASSERT(aContainer[*aIndex].mSoftTextOffset > aSoftTextOffset);
   }
   return true;
 }
 
 } // namespace
 
-mozInlineSpellWordUtil::NodeOffset
+NodeOffset
 mozInlineSpellWordUtil::MapSoftTextOffsetToDOMPosition(int32_t aSoftTextOffset,
                                                        DOMMapHint aHint)
 {
   NS_ASSERTION(mSoftTextValid, "Soft text must be valid if we're to map out of it");
   if (!mSoftTextValid)
     return NodeOffset(nullptr, -1);
 
   // Find the last mapping, if any, such that mSoftTextOffset <= aSoftTextOffset
--- a/extensions/spellcheck/src/mozInlineSpellWordUtil.h
+++ b/extensions/spellcheck/src/mozInlineSpellWordUtil.h
@@ -16,16 +16,54 @@
 
 class nsRange;
 class nsINode;
 
 namespace mozilla {
 class TextEditor;
 } // namespace mozilla
 
+struct NodeOffset {
+  nsINode* mNode;
+  int32_t  mOffset;
+
+  NodeOffset(): mNode(nullptr), mOffset(0) {}
+  NodeOffset(nsINode* aNode, int32_t aOffset) :
+    mNode(aNode), mOffset(aOffset) {}
+
+  bool operator==(const NodeOffset& aOther) const {
+    return mNode == aOther.mNode && mOffset == aOther.mOffset;
+  }
+
+  bool operator!=(const NodeOffset& aOther) const {
+    return !(*this == aOther);
+  }
+};
+
+class NodeOffsetRange {
+private:
+  NodeOffset mBegin;
+  NodeOffset mEnd;
+  bool mEmpty;
+public:
+  NodeOffsetRange() : mEmpty(true) {}
+  NodeOffsetRange(NodeOffset b, NodeOffset e) :
+    mBegin(b), mEnd(e), mEmpty(false) {}
+
+  NodeOffset Begin() {
+    return mBegin;
+  }
+  NodeOffset End() {
+    return mEnd;
+  }
+  bool Empty() {
+    return mEmpty;
+  }
+};
+
 /**
  *    This class extracts text from the DOM and builds it into a single string.
  *    The string includes whitespace breaks whereever non-inline elements begin
  *    and end. This string is broken into "real words", following somewhat
  *    complex rules; for example substrings that look like URLs or
  *    email addresses are treated as single words, but otherwise many kinds of
  *    punctuation are treated as word separators. GetNextWord provides a way
  *    to iterate over these "real words".
@@ -39,32 +77,16 @@ class TextEditor;
  *    3. Call SetPosition to initialize the current position inside the
  *       previously given range.
  *    4. Call GetNextWord over and over until it returns false.
  */
 
 class mozInlineSpellWordUtil
 {
 public:
-  struct NodeOffset {
-    nsINode* mNode;
-    int32_t  mOffset;
-
-    NodeOffset(nsINode* aNode, int32_t aOffset) :
-      mNode(aNode), mOffset(aOffset) {}
-
-    bool operator==(const NodeOffset& aOther) const {
-      return mNode == aOther.mNode && mOffset == aOther.mOffset;
-    }
-
-    bool operator!=(const NodeOffset& aOther) const {
-      return !(*this == aOther);
-    }
-  };
-
   mozInlineSpellWordUtil()
     : mRootNode(nullptr),
       mSoftBegin(nullptr, 0), mSoftEnd(nullptr, 0),
       mNextWordIndex(-1), mSoftTextValid(false) {}
 
   nsresult Init(mozilla::TextEditor* aTextEditor);
 
   nsresult SetEnd(nsINode* aEndNode, int32_t aEndOffset);
@@ -80,21 +102,25 @@ public:
   // inside the word, you should check the range.
   //
   // THIS CHANGES THE CURRENT POSITION AND RANGE. It is designed to be called
   // before you actually generate the range you are interested in and iterate
   // the words in it.
   nsresult GetRangeForWord(nsIDOMNode* aWordNode, int32_t aWordOffset,
                            nsRange** aRange);
 
+  // Convenience functions, object must be initialized
+  nsresult MakeRange(NodeOffset aBegin, NodeOffset aEnd, nsRange** aRange);
+
   // Moves to the the next word in the range, and retrieves it's text and range.
   // An empty word and a nullptr range are returned when we are done checking.
   // aSkipChecking will be set if the word is "special" and shouldn't be
   // checked (e.g., an email address).
-  nsresult GetNextWord(nsAString& aText, nsRange** aRange,
+  nsresult GetNextWord(nsAString& aText,
+                       NodeOffsetRange* aNodeOffsetRange,
                        bool* aSkipChecking);
 
   // Call to normalize some punctuation. This function takes an autostring
   // so we can access characters directly.
   static void NormalizeWord(nsAString& aWord);
 
   nsIDOMDocument* GetDOMDocument() const { return mDOMDocument; }
   nsIDocument* GetDocument() const { return mDocument; }
@@ -170,14 +196,14 @@ private:
 
   // build mSoftText and mSoftTextDOMMapping
   void BuildSoftText();
   // Build mRealWords array
   nsresult BuildRealWords();
 
   nsresult SplitDOMWord(int32_t aStart, int32_t aEnd);
 
-  // Convenience functions, object must be initialized
-  nsresult MakeRange(NodeOffset aBegin, NodeOffset aEnd, nsRange** aRange);
   nsresult MakeRangeForWord(const RealWord& aWord, nsRange** aRange);
+  void MakeNodeOffsetRangeForWord(const RealWord& aWord,
+                                  NodeOffsetRange* aNodeOffsetRange);
 };
 
 #endif