Commit 3f3888d0 authored by Evelyn Hung's avatar Evelyn Hung
Browse files

Bug 1354641 - adjust the value of spell check heuristics. r=masayuki 

This patch is mainly for adjusting the value of INLINESPELL_CHECK_TIMEOUT
from 50ms to 1ms. The value means how long the main thread is blocked
for spelling check, and 50ms is too long. It causes significant delays
when a rich content document is loading, and the user tries to
type immediately before spell checking is done.

With the INLINESPELL_CHECK_TIMEOUT setting to 1ms, it's possible to be
too short to less powerful machines. Therefore we add
INLINESPELL_MINIMUM_WORDS_BEFORE_TIMEOUT to ensure at least N words were
checked.

MozReview-Commit-ID: 1KYeFpggZGn

--HG--
extra : rebase_source : d7733dd37a923e6d6afb7f88174f352d28d4a3c4
parent 47f3443d

extensions/spellcheck/src/mozInlineSpellChecker.cpp

+89 −86
Original line number Diff line number Diff line
@@ -78,20 +78,13 @@ using namespace mozilla::dom; 
//#define DEBUG_INLINESPELL



// the number of milliseconds that we will take at once to do spellchecking

#define INLINESPELL_CHECK_TIMEOUT 50

#define INLINESPELL_CHECK_TIMEOUT 1



// The number of words to check before we look at the time to see if

// INLINESPELL_CHECK_TIMEOUT ms have elapsed. This prevents us from spending

// too much time checking the clock. Note that misspelled words count for

// more than one word in this calculation.

#define INLINESPELL_TIMEOUT_CHECK_FREQUENCY 50



// This number is the number of checked words a misspelled word counts for

// when we're checking the time to see if the alloted time is up for

// spellchecking. Misspelled words take longer to process since we have to

// create a range, so they count more. The exact number isn't very important

// since this just controls how often we check the current time.

#define MISSPELLED_WORD_COUNT_PENALTY 4

// INLINESPELL_CHECK_TIMEOUT ms have elapsed. This prevents us from getting

// stuck and not moving forward because the INLINESPELL_CHECK_TIMEOUT might

// be too short to a low-end machine.

#define INLINESPELL_MINIMUM_WORDS_BEFORE_TIMEOUT 5



// These notifications are broadcast when spell check starts and ends.  STARTED

// must always be followed by ENDED.
@@ -101,7 +94,10 @@ using namespace mozilla::dom; 
static bool ContentIsDescendantOf(nsINode* aPossibleDescendant,

                                    nsINode* aPossibleAncestor);



static const char kMaxSpellCheckSelectionSize[] = "extensions.spellcheck.inline.max-misspellings";

static const char kMaxSpellCheckSelectionSize[] =

  "extensions.spellcheck.inline.max-misspellings";

static const PRTime kMaxSpellCheckTimeInUsec =

  INLINESPELL_CHECK_TIMEOUT * PR_USEC_PER_MSEC;



mozInlineSpellStatus::mozInlineSpellStatus(mozInlineSpellChecker* aSpellChecker)

    : mSpellChecker(aSpellChecker), mWordCount(0)
@@ -475,9 +471,12 @@ mozInlineSpellStatus::PositionToCollapsedRange(nsIDOMDocument* aDocument, 
class mozInlineSpellResume : public Runnable

{

public:

  mozInlineSpellResume(const mozInlineSpellStatus& aStatus,

  mozInlineSpellResume(UniquePtr<mozInlineSpellStatus>&& aStatus,

                       uint32_t aDisabledAsyncToken)

    : mDisabledAsyncToken(aDisabledAsyncToken), mStatus(aStatus) {}

    : mDisabledAsyncToken(aDisabledAsyncToken)

    , mStatus(Move(aStatus))

  {

  }



  nsresult Post()

  {
@@ -488,15 +487,15 @@ public: 
  {

    // Discard the resumption if the spell checker was disabled after the

    // resumption was scheduled.

    if (mDisabledAsyncToken == mStatus.mSpellChecker->mDisabledAsyncToken) {

      mStatus.mSpellChecker->ResumeCheck(&mStatus);

    if (mDisabledAsyncToken == mStatus->mSpellChecker->mDisabledAsyncToken) {

      mStatus->mSpellChecker->ResumeCheck(Move(mStatus));

    }

    return NS_OK;

  }



private:

  uint32_t mDisabledAsyncToken;

  mozInlineSpellStatus mStatus;

  UniquePtr<mozInlineSpellStatus> mStatus;

};



// Used as the nsIEditorSpellCheck::InitSpellChecker callback.
@@ -687,7 +686,8 @@ mozInlineSpellChecker::UpdateCanEnableInlineSpellChecking() 


// mozInlineSpellChecker::RegisterEventListeners

//

//    The inline spell checker listens to mouse events and keyboard navigation+ //    events.

//    The inline spell checker listens to mouse events and keyboard navigation

//    events.



nsresult

mozInlineSpellChecker::RegisterEventListeners()
@@ -888,14 +888,15 @@ mozInlineSpellChecker::SpellCheckAfterEditorChange( 
  rv = aSelection->GetAnchorOffset(&anchorOffset);

  NS_ENSURE_SUCCESS(rv, rv);



  mozInlineSpellStatus status(this);

  rv = status.InitForEditorChange((EditAction)aAction,

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForEditorChange((EditAction)aAction,

                                   anchorNode, anchorOffset,

                                  aPreviousSelectedNode, aPreviousSelectedOffset,

                                   aPreviousSelectedNode,

                                   aPreviousSelectedOffset,

                                   aStartNode, aStartOffset,

                                   aEndNode, aEndOffset);

  NS_ENSURE_SUCCESS(rv, rv);

  rv = ScheduleSpellCheck(status);

  rv = ScheduleSpellCheck(Move(status));

  NS_ENSURE_SUCCESS(rv, rv);



  // remember the current caret position after every change
@@ -918,11 +919,11 @@ mozInlineSpellChecker::SpellCheckRange(nsIDOMRange* aRange) 
    return NS_ERROR_NOT_INITIALIZED;

  }



  mozInlineSpellStatus status(this);

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  nsRange* range = static_cast<nsRange*>(aRange);

  nsresult rv = status.InitForRange(range);

  nsresult rv = status->InitForRange(range);

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



// mozInlineSpellChecker::GetMisspelledWord
@@ -989,10 +990,10 @@ mozInlineSpellChecker::AddWordToDictionary(const nsAString &word) 
  nsresult rv = mSpellCheck->AddWordToDictionary(wordstr.get());

  NS_ENSURE_SUCCESS(rv, rv); 



  mozInlineSpellStatus status(this);

  rv = status.InitForSelection();

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForSelection();

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



//  mozInlineSpellChecker::RemoveWordFromDictionary
@@ -1006,10 +1007,10 @@ mozInlineSpellChecker::RemoveWordFromDictionary(const nsAString &word) 
  nsresult rv = mSpellCheck->RemoveWordFromDictionary(wordstr.get());

  NS_ENSURE_SUCCESS(rv, rv); 

  

  mozInlineSpellStatus status(this);

  rv = status.InitForRange(nullptr);

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForRange(nullptr);

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



// mozInlineSpellChecker::IgnoreWord
@@ -1023,10 +1024,10 @@ mozInlineSpellChecker::IgnoreWord(const nsAString &word) 
  nsresult rv = mSpellCheck->IgnoreWordAllOccurrences(wordstr.get());

  NS_ENSURE_SUCCESS(rv, rv); 



  mozInlineSpellStatus status(this);

  rv = status.InitForSelection();

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForSelection();

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



// mozInlineSpellChecker::IgnoreWords
@@ -1041,10 +1042,10 @@ mozInlineSpellChecker::IgnoreWords(const char16_t **aWordsToIgnore, 
  for (uint32_t index = 0; index < aCount; index++)

    mSpellCheck->IgnoreWordAllOccurrences(aWordsToIgnore[index]);



  mozInlineSpellStatus status(this);

  nsresult rv = status.InitForSelection();

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  nsresult rv = status->InitForSelection();

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



NS_IMETHODIMP mozInlineSpellChecker::WillCreateNode(const nsAString & aTag, nsIDOMNode *aParent, int32_t aPosition)
@@ -1224,10 +1225,10 @@ mozInlineSpellChecker::SpellCheckBetweenNodes(nsIDOMNode *aStartNode, 
  if (! range)

    return NS_OK; // range is empty: nothing to do



  mozInlineSpellStatus status(this);

  rv = status.InitForRange(range);

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForRange(range);

  NS_ENSURE_SUCCESS(rv, rv);

  return ScheduleSpellCheck(status);

  return ScheduleSpellCheck(Move(status));

}



// mozInlineSpellChecker::ShouldSpellCheckNode
@@ -1312,20 +1313,23 @@ mozInlineSpellChecker::ShouldSpellCheckNode(nsIEditor* aEditor, 
//    the proper structures for calls to DoSpellCheck.



nsresult

mozInlineSpellChecker::ScheduleSpellCheck(const mozInlineSpellStatus& aStatus)

mozInlineSpellChecker::ScheduleSpellCheck(UniquePtr<mozInlineSpellStatus>&& aStatus)

{

  if (mFullSpellCheckScheduled) {

    // Just ignore this; we're going to spell-check everything anyway

    return NS_OK;

  }

  // Cache the value because we are going to move aStatus's ownership to

  // the new created mozInlineSpellResume instance.

  bool isFullSpellCheck = aStatus->IsFullSpellCheck();



  RefPtr<mozInlineSpellResume> resume =

    new mozInlineSpellResume(aStatus, mDisabledAsyncToken);

    new mozInlineSpellResume(Move(aStatus), mDisabledAsyncToken);

  NS_ENSURE_TRUE(resume, NS_ERROR_OUT_OF_MEMORY);



  nsresult rv = resume->Post();

  if (NS_SUCCEEDED(rv)) {

    if (aStatus.IsFullSpellCheck()) {

    if (isFullSpellCheck) {

      // We're going to check everything.  Suppress further spell-check attempts

      // until that happens.

      mFullSpellCheckScheduled = true;
@@ -1348,8 +1352,7 @@ mozInlineSpellChecker::ScheduleSpellCheck(const mozInlineSpellStatus& aStatus) 


nsresult

mozInlineSpellChecker::DoSpellCheckSelection(mozInlineSpellWordUtil& aWordUtil,

                                             Selection* aSpellCheckSelection,

                                             mozInlineSpellStatus* aStatus)

                                             Selection* aSpellCheckSelection)

{

  nsresult rv;
@@ -1380,8 +1383,8 @@ mozInlineSpellChecker::DoSpellCheckSelection(mozInlineSpellWordUtil& aWordUtil, 
  // We use this state object for all calls, and just update its range. Note

  // that we don't need to call FinishInit since we will be filling in the

  // necessary information.

  mozInlineSpellStatus status(this);

  rv = status.InitForRange(nullptr);

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForRange(nullptr);

  NS_ENSURE_SUCCESS(rv, rv);



  bool doneChecking;
@@ -1390,14 +1393,14 @@ mozInlineSpellChecker::DoSpellCheckSelection(mozInlineSpellWordUtil& aWordUtil, 
    // check range over it that needs to be deleted. All the old ranges

    // were cleared above. We also need to clear the word count so that we

    // check all words instead of stopping early.

    status.mRange = ranges[idx];

    rv = DoSpellCheck(aWordUtil, aSpellCheckSelection, &status,

    status->mRange = ranges[idx];

    rv = DoSpellCheck(aWordUtil, aSpellCheckSelection, status,

                      &doneChecking);

    NS_ENSURE_SUCCESS(rv, rv);

    MOZ_ASSERT(doneChecking,

               "We gave the spellchecker one word, but it didn't finish checking?!?!");



    status.mWordCount = 0;

    status->mWordCount = 0;

  }



  return NS_OK;
@@ -1436,7 +1439,7 @@ mozInlineSpellChecker::DoSpellCheckSelection(mozInlineSpellWordUtil& aWordUtil, 


nsresult mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil& aWordUtil,

                                             Selection *aSpellCheckSelection,

                                             mozInlineSpellStatus* aStatus,

                                             const UniquePtr<mozInlineSpellStatus>& aStatus,

                                             bool* aDoneChecking)

{

  *aDoneChecking = true;
@@ -1485,7 +1488,7 @@ nsresult mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil& aWordUtil, 
  if (! editor)

    return NS_ERROR_FAILURE;



  int32_t wordsSinceTimeCheck = 0;

  int32_t wordsChecked = 0;

  PRTime beginTime = PR_Now();



  nsAutoString wordText;
@@ -1495,7 +1498,6 @@ nsresult mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil& aWordUtil, 
                                            getter_AddRefs(wordRange),

                                            &dontCheckWord)) &&

         wordRange) {

    wordsSinceTimeCheck++;



    // get the range for the current word.

    nsINode *beginNode;
@@ -1508,6 +1510,25 @@ nsresult mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil& aWordUtil, 
    beginOffset = wordRange->GetStartOffset(erv);

    endOffset = wordRange->GetEndOffset(erv);



    // 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.



      // move the range to encompass the stuff that needs checking.

      nsresult rv = aStatus->mRange->SetStart(endNode, endOffset);

      if (NS_FAILED(rv)) {

        // The range might be unhappy because the beginning is after the

        // end. This is possible when the requested end was in the middle

        // of a word, just ignore this situation and assume we're done.

        return NS_OK;

      }

      *aDoneChecking = false;

      return NS_OK;

    }



    wordsChecked++;



#ifdef DEBUG_INLINESPELL

    printf("->Got word \"%s\"", NS_ConvertUTF16toUTF8(wordText).get());

    if (dontCheckWord)
@@ -1561,32 +1582,13 @@ nsresult mozInlineSpellChecker::DoSpellCheck(mozInlineSpellWordUtil& aWordUtil, 


    if (isMisspelled) {

      // misspelled words count extra toward the max

      wordsSinceTimeCheck += MISSPELLED_WORD_COUNT_PENALTY;

      AddRange(aSpellCheckSelection, wordRange);



      aStatus->mWordCount ++;

      if (aStatus->mWordCount >= mMaxMisspellingsPerCheck ||

          SpellCheckSelectionIsFull())

          SpellCheckSelectionIsFull()) {

        break;

      }



    // see if we've run out of time, only check every N words for perf

    if (wordsSinceTimeCheck >= INLINESPELL_TIMEOUT_CHECK_FREQUENCY) {

      wordsSinceTimeCheck = 0;

      if (PR_Now() > PRTime(beginTime + INLINESPELL_CHECK_TIMEOUT * PR_USEC_PER_MSEC)) {

        // stop checking, our time limit has been exceeded



        // move the range to encompass the stuff that needs checking

        rv = aStatus->mRange->SetStart(endNode, endOffset);

        if (NS_FAILED(rv)) {

          // The range might be unhappy because the beginning is after the

          // end. This is possible when the requested end was in the middle

          // of a word, just ignore this situation and assume we're done.

          return NS_OK;

        }

        *aDoneChecking = false;

        return NS_OK;

      }

    }

  }
@@ -1617,7 +1619,7 @@ private: 
//    the last resume left off.



nsresult

mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus* aStatus)

mozInlineSpellChecker::ResumeCheck(UniquePtr<mozInlineSpellStatus>&& aStatus)

{

  // Observers should be notified that spell check has ended only after spell

  // check is done below, but since there are many early returns in this method
@@ -1676,13 +1678,13 @@ mozInlineSpellChecker::ResumeCheck(mozInlineSpellStatus* aStatus) 


  bool doneChecking = true;

  if (aStatus->mOp == mozInlineSpellStatus::eOpSelection)

    rv = DoSpellCheckSelection(wordUtil, spellCheckSelection, aStatus);

    rv = DoSpellCheckSelection(wordUtil, spellCheckSelection);

  else

    rv = DoSpellCheck(wordUtil, spellCheckSelection, aStatus, &doneChecking);

  NS_ENSURE_SUCCESS(rv, rv);



  if (! doneChecking)

    rv = ScheduleSpellCheck(*aStatus);

    rv = ScheduleSpellCheck(Move(aStatus));

  return rv;

}
@@ -1877,14 +1879,15 @@ mozInlineSpellChecker::HandleNavigationEvent(bool aForceWordSpellCheck, 
  NS_ENSURE_SUCCESS(rv, rv);



  bool shouldPost;

  mozInlineSpellStatus status(this);

  rv = status.InitForNavigation(aForceWordSpellCheck, aNewPositionOffset,

  auto status = MakeUnique<mozInlineSpellStatus>(this);

  rv = status->InitForNavigation(aForceWordSpellCheck, aNewPositionOffset,

                                 currentAnchorNode, currentAnchorOffset,

                                mCurrentSelectionAnchorNode, mCurrentSelectionOffset,

                                 mCurrentSelectionAnchorNode,

                                 mCurrentSelectionOffset,

                                 &shouldPost);

  NS_ENSURE_SUCCESS(rv, rv);

  if (shouldPost) {

    rv = ScheduleSpellCheck(status);

    rv = ScheduleSpellCheck(Move(status));

    NS_ENSURE_SUCCESS(rv, rv);

  }

extensions/spellcheck/src/mozInlineSpellChecker.h

+4 −5
Original line number Diff line number Diff line
@@ -218,14 +218,13 @@ public: 


  // spell check the text contained within aRange, potentially scheduling

  // another check in the future if the time threshold is reached

  nsresult ScheduleSpellCheck(const mozInlineSpellStatus& aStatus);

  nsresult ScheduleSpellCheck(mozilla::UniquePtr<mozInlineSpellStatus>&& aStatus);



  nsresult DoSpellCheckSelection(mozInlineSpellWordUtil& aWordUtil,

                                 mozilla::dom::Selection* aSpellCheckSelection,

                                 mozInlineSpellStatus* aStatus);

                                 mozilla::dom::Selection* aSpellCheckSelection);

  nsresult DoSpellCheck(mozInlineSpellWordUtil& aWordUtil,

                        mozilla::dom::Selection *aSpellCheckSelection,

                        mozInlineSpellStatus* aStatus,

                        const mozilla::UniquePtr<mozInlineSpellStatus>& aStatus,

                        bool* aDoneChecking);



  // helper routine to determine if a point is inside of the passed in selection.
@@ -253,7 +252,7 @@ public: 
  nsresult GetSpellCheckSelection(nsISelection ** aSpellCheckSelection);

  nsresult SaveCurrentSelectionPosition();



  nsresult ResumeCheck(mozInlineSpellStatus* aStatus);

  nsresult ResumeCheck(mozilla::UniquePtr<mozInlineSpellStatus>&& aStatus);



protected:

  virtual ~mozInlineSpellChecker();