Bug 1396980 - Spending too much time making ranges for spellchecker on Facebook page, r=smaug
authorjj.evelyn@gmail.com
Sat, 06 Jan 2018 16:10:29 +0200
changeset 449896 6bf9c25d174c929577ea431248a5d770ef22ecd5
parent 449895 d97438b671ae346ce54ebfd3b990cc8a9171b2bf
child 449897 36aaa86e37bac0db15a499a7f48a83bf9dff5602
child 449921 30237d58eb352f8246585b2b2361b407d9915c49
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1396980
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1396980 - Spending too much time making ranges for spellchecker on Facebook page, r=smaug
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
@@ -1486,33 +1486,27 @@ 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);
+    nsINode* beginNode = wordNodeOffsetRange.Begin().mNode;
+    nsINode* endNode = wordNodeOffsetRange.End().mNode;
+    int32_t beginOffset = wordNodeOffsetRange.Begin().mOffset;
+    int32_t 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
@@ -1530,16 +1524,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;
@@ -1577,25 +1572,29 @@ nsresult mozInlineSpellChecker::DoSpellC
     bool isMisspelled;
     aWordUtil.NormalizeWord(wordText);
     nsresult rv = mSpellCheck->CheckCurrentWordNoSuggest(wordText,
                                                          &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,63 @@
 
 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 +86,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 +111,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 +205,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