From bf48e8ad9d0449d0ac90d5e89fde1c6bafd2c1ce Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Fri, 11 Jan 2019 15:46:05 +0900 Subject: [PATCH] Bug 1502661 - Part 3. Use async API for spell checking. r=masayuki Previous implementation is - Use idle runnable for spellchecker - If spellchecker runnable spends 1ms (with 5 words), spellchecker is suspended, then resume it after 1s. - If misspell is 3/4 of extensions.spellchecker.inline.max-misspellings per DoSpellCheck, spellchecker is stopped. After this change, IPC is async, so we cannot count misspell words without waiting the result of async call. So I would like to change to - Use idle runnable for spellchecker - If spellchecker runnable spends 1ms (with 5 words), spellchecker is suspended, then resume it after 1s. - If misspell reaches extensions.spellchecker.inline.max-misspellings, stop spellcheck. - spellchecker IPC is called per maximum 25 (INLINESPELL_MAXIMUM_CHUNKED_WORDS_PER_TASK) words. As long as my MacBook (Core m3), spellchecker can check 100-150 words per 1ms after this fix. Also, INLINESPELL_MAXIMUM_CHUNKED_WORDS_PER_TASK define is less than 5, a lot of a11y tests will be failed. Because it uses selection listener to count all misspell words without waiting completion. Differential Revision: https://phabricator.services.mozilla.com/D14837 --HG-- extra : rebase_source : 59185554236409d3320cc77714a776f2228214af --- .../spellcheck/src/mozInlineSpellChecker.cpp | 136 +++++++++++++----- .../spellcheck/src/mozInlineSpellChecker.h | 18 +-- .../spellcheck/src/mozInlineSpellWordUtil.cpp | 13 ++ .../spellcheck/src/mozInlineSpellWordUtil.h | 14 +- 4 files changed, 129 insertions(+), 52 deletions(-) diff --git a/extensions/spellcheck/src/mozInlineSpellChecker.cpp b/extensions/spellcheck/src/mozInlineSpellChecker.cpp index 1a51473b50ba..b7e1152d7442 100644 --- a/extensions/spellcheck/src/mozInlineSpellChecker.cpp +++ b/extensions/spellcheck/src/mozInlineSpellChecker.cpp @@ -68,6 +68,7 @@ using namespace mozilla; using namespace mozilla::dom; +using namespace mozilla::ipc; // Set to spew messages to the console about what is happening. //#define DEBUG_INLINESPELL @@ -81,6 +82,9 @@ using namespace mozilla::dom; // be too short to a low-end machine. #define INLINESPELL_MINIMUM_WORDS_BEFORE_TIMEOUT 5 +// The maximum number of words to check word via IPC. +#define INLINESPELL_MAXIMUM_CHUNKED_WORDS_PER_TASK 25 + // These notifications are broadcast when spell check starts and ends. STARTED // must always be followed by ENDED. #define INLINESPELL_STARTED_TOPIC "inlineSpellChecker-spellCheck-started" @@ -92,7 +96,7 @@ static const PRTime kMaxSpellCheckTimeInUsec = INLINESPELL_CHECK_TIMEOUT * PR_USEC_PER_MSEC; mozInlineSpellStatus::mozInlineSpellStatus(mozInlineSpellChecker* aSpellChecker) - : mSpellChecker(aSpellChecker), mWordCount(0) {} + : mSpellChecker(aSpellChecker) {} // mozInlineSpellStatus::InitForEditorChange // @@ -520,7 +524,6 @@ mozInlineSpellChecker::mozInlineSpellChecker() if (prefs) prefs->GetIntPref(kMaxSpellCheckSelectionSize, &mMaxNumWordsInSpellSelection); - mMaxMisspellingsPerCheck = mMaxNumWordsInSpellSelection * 3 / 4; } mozInlineSpellChecker::~mozInlineSpellChecker() {} @@ -1195,8 +1198,6 @@ nsresult mozInlineSpellChecker::DoSpellCheckSelection( MOZ_ASSERT( doneChecking, "We gave the spellchecker one word, but it didn't finish checking?!?!"); - - status->mWordCount = 0; } return NS_OK; @@ -1238,12 +1239,18 @@ nsresult mozInlineSpellChecker::DoSpellCheck( const UniquePtr& aStatus, bool* aDoneChecking) { *aDoneChecking = true; - NS_ENSURE_TRUE(mSpellCheck, NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!mSpellCheck)) { + return NS_ERROR_NOT_INITIALIZED; + } + + if (SpellCheckSelectionIsFull()) { + return NS_OK; + } // get the editor for ShouldSpellCheckNode, this may fail in reasonable // circumstances since the editor could have gone away RefPtr textEditor = mTextEditor; - if (!textEditor) { + if (!textEditor || textEditor->Destroyed()) { return NS_ERROR_FAILURE; } @@ -1291,25 +1298,34 @@ nsresult mozInlineSpellChecker::DoSpellCheck( int32_t wordsChecked = 0; PRTime beginTime = PR_Now(); + nsTArray words; + nsTArray checkRanges; nsAutoString wordText; NodeOffsetRange wordNodeOffsetRange; bool dontCheckWord; + // XXX Spellchecker API isn't async on chrome process. + static const size_t requestChunkSize = + XRE_IsContentProcess() ? INLINESPELL_MAXIMUM_CHUNKED_WORDS_PER_TASK : 1; while (NS_SUCCEEDED(aWordUtil.GetNextWord(wordText, &wordNodeOffsetRange, &dontCheckWord)) && !wordNodeOffsetRange.Empty()) { // get the range for the current word. - nsINode* beginNode = wordNodeOffsetRange.Begin().mNode; - nsINode* endNode = wordNodeOffsetRange.End().mNode; - int32_t beginOffset = wordNodeOffsetRange.Begin().mOffset; - int32_t endOffset = wordNodeOffsetRange.End().mOffset; + nsINode* beginNode = wordNodeOffsetRange.Begin().Node(); + nsINode* endNode = wordNodeOffsetRange.End().Node(); + int32_t beginOffset = wordNodeOffsetRange.Begin().Offset(); + int32_t endOffset = wordNodeOffsetRange.End().Offset(); // 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."); + printf("We have run out of the time, schedule next round.\n"); #endif + CheckCurrentWordsNoSuggest(aSpellCheckSelection, words, checkRanges); + words.Clear(); + checkRanges.Clear(); + // move the range to encompass the stuff that needs checking. nsresult rv = aStatus->mRange->SetStart(beginNode, beginOffset); if (NS_FAILED(rv)) { @@ -1366,39 +1382,31 @@ nsresult mozInlineSpellChecker::DoSpellCheck( } // check spelling and add to selection if misspelled - bool isMisspelled; mozInlineSpellWordUtil::NormalizeWord(wordText); - nsresult rv = - mSpellCheck->CheckCurrentWordNoSuggest(wordText, &isMisspelled); - if (NS_FAILED(rv)) continue; - + words.AppendElement(wordText); + checkRanges.AppendElement(wordNodeOffsetRange); wordsChecked++; - if (isMisspelled) { - // misspelled words count extra toward the max - RefPtr 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; - } - } + if (words.Length() >= requestChunkSize) { + CheckCurrentWordsNoSuggest(aSpellCheckSelection, words, checkRanges); + words.Clear(); + checkRanges.Clear(); } } + CheckCurrentWordsNoSuggest(aSpellCheckSelection, words, checkRanges); + return NS_OK; } // An RAII helper that calls ChangeNumPendingSpellChecks on destruction. -class AutoChangeNumPendingSpellChecks { +class MOZ_RAII AutoChangeNumPendingSpellChecks final { public: - AutoChangeNumPendingSpellChecks(mozInlineSpellChecker* aSpellChecker, - int32_t aDelta) - : mSpellChecker(aSpellChecker), mDelta(aDelta) {} + explicit AutoChangeNumPendingSpellChecks(mozInlineSpellChecker* aSpellChecker, + int32_t aDelta + MOZ_GUARD_OBJECT_NOTIFIER_PARAM) + : mSpellChecker(aSpellChecker), mDelta(aDelta) { + MOZ_GUARD_OBJECT_NOTIFIER_INIT; + } ~AutoChangeNumPendingSpellChecks() { mSpellChecker->ChangeNumPendingSpellChecks(mDelta); @@ -1407,8 +1415,68 @@ class AutoChangeNumPendingSpellChecks { private: RefPtr mSpellChecker; int32_t mDelta; + MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER }; +void mozInlineSpellChecker::CheckCurrentWordsNoSuggest( + Selection* aSpellCheckSelection, const nsTArray& aWords, + const nsTArray& aRanges) { + if (aWords.IsEmpty()) { + return; + } + + ChangeNumPendingSpellChecks(1); + + RefPtr self = this; + RefPtr spellCheckerSelection = aSpellCheckSelection; + uint32_t token = mDisabledAsyncToken; + mSpellCheck->CheckCurrentWordsNoSuggest(aWords)->Then( + GetMainThreadSerialEventTarget(), __func__, + [self, spellCheckerSelection, aRanges, + token](const nsTArray& aIsMisspelled) { + if (token != self->mDisabledAsyncToken) { + // This result is never used + return; + } + + if (!self->mTextEditor || self->mTextEditor->Destroyed()) { + return; + } + + AutoChangeNumPendingSpellChecks pendingChecks(self, -1); + + if (self->SpellCheckSelectionIsFull()) { + return; + } + + for (size_t i = 0; i < aIsMisspelled.Length(); i++) { + if (!aIsMisspelled[i]) { + continue; + } + + RefPtr wordRange = + mozInlineSpellWordUtil::MakeRange(aRanges[i]); + // If we somehow can't make a range for this word, just ignore + // it. + if (wordRange) { + self->AddRange(spellCheckerSelection, wordRange); + } + } + }, + [self, token](nsresult aRv) { + if (!self->mTextEditor || self->mTextEditor->Destroyed()) { + return; + } + + if (token != self->mDisabledAsyncToken) { + // This result is never used + return; + } + + self->ChangeNumPendingSpellChecks(-1); + }); +} + // mozInlineSpellChecker::ResumeCheck // // Called by the resume event when it fires. We will try to pick up where diff --git a/extensions/spellcheck/src/mozInlineSpellChecker.h b/extensions/spellcheck/src/mozInlineSpellChecker.h index cdf0f7becce4..f53233da7dff 100644 --- a/extensions/spellcheck/src/mozInlineSpellChecker.h +++ b/extensions/spellcheck/src/mozInlineSpellChecker.h @@ -10,11 +10,11 @@ #include "nsIDOMEventListener.h" #include "nsIEditorSpellCheck.h" #include "nsIInlineSpellChecker.h" +#include "mozInlineSpellWordUtil.h" #include "nsRange.h" #include "nsWeakReference.h" class InitEditorSpellCheckCallback; -class mozInlineSpellWordUtil; class mozInlineSpellChecker; class mozInlineSpellResume; class UpdateCurrentDictionaryCallback; @@ -52,11 +52,6 @@ class mozInlineSpellStatus { RefPtr mSpellChecker; - // The total number of words checked in this sequence, using this tally tells - // us when to stop. This count is preserved as we continue checking in new - // messages. - int32_t mWordCount; - // what happened? enum Operation { eOpChange, // for SpellCheckAfterEditorChange except @@ -139,13 +134,6 @@ class mozInlineSpellChecker final : public nsIInlineSpellChecker, int32_t mNumWordsInSpellSelection; int32_t mMaxNumWordsInSpellSelection; - // How many misspellings we can add at once. This is often less than the max - // total number of misspellings. When you have a large textarea prepopulated - // with text with many misspellings, we can hit this limit. By making it - // lower than the total number of misspelled words, new text typed by the - // user can also have spellchecking in it. - int32_t mMaxMisspellingsPerCheck; - // we need to keep track of the current text position in the document // so we can spell check the old word when the user clicks around the // document. @@ -282,6 +270,10 @@ class mozInlineSpellChecker final : public nsIInlineSpellChecker, void StartToListenToEditSubActions() { mIsListeningToEditSubActions = true; } void EndListeningToEditSubActions() { mIsListeningToEditSubActions = false; } + + void CheckCurrentWordsNoSuggest(mozilla::dom::Selection* aSpellCheckSelection, + const nsTArray& aWords, + const nsTArray& aRanges); }; #endif // #ifndef mozilla_mozInlineSpellChecker_h diff --git a/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp b/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp index bbe8304be916..525858ccb940 100644 --- a/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp +++ b/extensions/spellcheck/src/mozInlineSpellWordUtil.cpp @@ -330,6 +330,19 @@ nsresult mozInlineSpellWordUtil::MakeRange(NodeOffset aBegin, NodeOffset aEnd, return NS_OK; } +// static +already_AddRefed mozInlineSpellWordUtil::MakeRange( + const NodeOffsetRange& aRange) { + RefPtr range = new nsRange(aRange.Begin().Node()); + nsresult rv = + range->SetStartAndEnd(aRange.Begin().Node(), aRange.Begin().Offset(), + aRange.End().Node(), aRange.End().Offset()); + if (NS_WARN_IF(NS_FAILED(rv))) { + return nullptr; + } + return range.forget(); +} + /*********** Word Splitting ************/ // classifies a given character in the DOM word diff --git a/extensions/spellcheck/src/mozInlineSpellWordUtil.h b/extensions/spellcheck/src/mozInlineSpellWordUtil.h index 279b9507f669..8fb8bca59e23 100644 --- a/extensions/spellcheck/src/mozInlineSpellWordUtil.h +++ b/extensions/spellcheck/src/mozInlineSpellWordUtil.h @@ -22,10 +22,10 @@ class TextEditor; } // namespace mozilla struct NodeOffset { - nsINode* mNode; + nsCOMPtr mNode; int32_t mOffset; - NodeOffset() : mNode(nullptr), mOffset(0) {} + NodeOffset() : mOffset(0) {} NodeOffset(nsINode* aNode, int32_t aOffset) : mNode(aNode), mOffset(aOffset) {} @@ -34,6 +34,9 @@ struct NodeOffset { } bool operator!=(const NodeOffset& aOther) const { return !(*this == aOther); } + + nsINode* Node() const { return mNode.get(); } + int32_t Offset() const { return mOffset; } }; class NodeOffsetRange { @@ -47,11 +50,11 @@ class NodeOffsetRange { NodeOffsetRange(NodeOffset b, NodeOffset e) : mBegin(b), mEnd(e), mEmpty(false) {} - NodeOffset Begin() { return mBegin; } + NodeOffset Begin() const { return mBegin; } - NodeOffset End() { return mEnd; } + NodeOffset End() const { return mEnd; } - bool Empty() { return mEmpty; } + bool Empty() const { return mEmpty; } }; /** @@ -104,6 +107,7 @@ class MOZ_STACK_CLASS mozInlineSpellWordUtil { // Convenience functions, object must be initialized nsresult MakeRange(NodeOffset aBegin, NodeOffset aEnd, nsRange** aRange); + static already_AddRefed MakeRange(const NodeOffsetRange& 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.