Remove stale spellcheck markers at idle time

Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=Existing layout tests under editing/spelling/

Review-Url: https://codereview.chromium.org/2785103005
Cr-Commit-Position: refs/heads/master@{#461968}
Committed: https://chromium.googlesource.com/chromium/src/+/fd7ce52f21f9dd6c741ee0545cc109ed6267b95e

[Xiaocheng, 2017-03-31 00:02:19]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 00:04:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-03-31 00:07:11]

Description was changed from

==========
Remove stale spellcheck markers at idle time

BUG=
==========

to

==========
Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=n/a; No easily visible change
==========

[Xiaocheng, 2017-03-31 00:08:26]

xiaochengh@chromium.org changed reviewers:
+ yosin@chromium.org

[Xiaocheng, 2017-03-31 00:08:27]

PTAL.

I manually tested the behavior, which looks totally natural. It also passes all
existing tests, which include verification of stale marker removal. So I think
new test is not needed.

[Xiaocheng, 2017-03-31 00:08:47]

Description was changed from

==========
Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=n/a; No easily visible change
==========

to

==========
Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=Existing layout tests under editing/spelling/
==========

[commit-bot: I haz the power, 2017-03-31 01:48:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 01:48:12]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-04-03 17:56:57]

Ping...

[yosin_UTC9, 2017-04-05 01:52:35]

lgtm w/ nit

Wonderful!

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
(right):

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp:32:
if (comparePositions(visiblePos, wordEnd) > 0)
nit: We can use |visiblePos > wordEnd|

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp (right):

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp:657: if
(RuntimeEnabledFeatures::idleTimeSpellCheckingEnabled())
Nice! (^_^)b

[Xiaocheng, 2017-04-05 02:08:32]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-04-05 02:08:33]

Thanks for the review!

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
(right):

https://codereview.chromium.org/2785103005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp:32:
if (comparePositions(visiblePos, wordEnd) > 0)
On 2017/04/05 at 01:52:35, yosin_UTC9 wrote:
> nit: We can use |visiblePos > wordEnd|

We can't...

error: invalid operands to binary expression ('const blink::VisiblePosition'
(aka 'const VisiblePositionTemplate<EditingAlgorithm<blink::NodeTraversal> >')
and 'const blink::VisiblePosition')

[commit-bot: I haz the power, 2017-04-05 02:09:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 03:34:17]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491358112885390, "parent_rev":
"c2d4ff6157a4d06bd06089b872e0bc8a81068853", "commit_rev":
"fd7ce52f21f9dd6c741ee0545cc109ed6267b95e"}

[commit-bot: I haz the power, 2017-04-05 03:35:18]

Description was changed from

==========
Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=Existing layout tests under editing/spelling/
==========

to

==========
Remove stale spellcheck markers at idle time

Blink's spellchecker used to remove stale/invalid markers
synchronously, which is time-costly. This patch greatly
simplifies the removal logic by integrate it with idle
time spell checker.

Detailed design and analysis: https://goo.gl/P7F710

BUG=705180
TEST=Existing layout tests under editing/spelling/

Review-Url: https://codereview.chromium.org/2785103005
Cr-Commit-Position: refs/heads/master@{#461968}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd7ce52f21f9dd6c741ee0545cc1...
==========

[commit-bot: I haz the power, 2017-04-05 03:35:19]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/fd7ce52f21f9dd6c741ee0545cc1...