Trigger spell check/remove markers if spell checker gets enabled/disabled.

Trigger spell check/remove markers if spell checker gets enabled/disabled.

With this change when spell checker is turned off the misspelling markers
disappear from the document (which is what a user expects and this currently
works in chromium because UI code takes care of that - with this change it may
stop). OTOH if spell checker is turned on again one would expect misspellings
will appear right away - in the currently focused editable at least. This is how
Gecko and Presto behave and this is what this patch does. Note it's not only
about restoring old markers because content might have changed or language might
have been switched. This is why new check is requested. This change may also
come in handy if spell checking language is changed. It would require restarting
spell checker on UI side on language change, though.

Since there might be an awful lot of text in the focused editable when spell
checker gets enabled, spell checking is chunked up. If asynchronous spell
checker is enabled whole text is checked. Otherwise only one chunk is checked
(the default chunk's size is set to 16KB but it might also be slightly bigger
since all chunks are aligned to the sentence boundary).

BUG=264737
This is the second attempt as this patch was integrated but
reverted due to performance regression found by
DOM/textarea-edit.html (issue 274569). This patch optimizes
single chunk case to try to avoid the regression.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156378

[pstanek, 2013-08-19 19:29:04]

21130005 reload as it was reverted due to performance regression. Let's try
this, then

[tony, 2013-08-19 19:51:54]

The change description should make it clear what is different from the previous
patch.

[pstanek, 2013-08-19 19:54:09]

On 2013/08/19 19:51:54, tony wrote:
> The change description should make it clear what is different from the
previous
> patch.

Only chunking loop in Editor.cpp. I added conditions to avoid chunking up if
there only one chunk to check.

[pstanek, 2013-08-19 19:54:28]

On 2013/08/19 19:54:09, pstanek wrote:
> On 2013/08/19 19:51:54, tony wrote:
> > The change description should make it clear what is different from the
> previous
> > patch.
> 
> Only chunking loop in Editor.cpp. I added conditions to avoid chunking up if
> there only one chunk to check.

+ is.

[tony, 2013-08-19 20:01:15]

https://codereview.chromium.org/23332004/diff/1/Source/core/editing/Editor.cpp
File Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/23332004/diff/1/Source/core/editing/Editor.cp...
Source/core/editing/Editor.cpp:1494: if (kNumChunksToCheck > 1 || !asynchronous)
{
Having if (kNumChunksToCheck) conditions in the for loop is weird.  It would be
better if we only entered the for loop if kNumChunksToCheck > 1.  Maybe
restructure it to something like:

if (kNumChunksToCheck <= 1) {
    markAllMisspellingsAndBadGrammarInRange(range, asynchronous, ...)
    return;
}

for (int iter = 0; iter < kNumChunksToCheck; ++iter) {
    // compute range to check
    markAllMisspellingsAndBadGrammarInRange(range, asynchronous, ...)
    // update currentChunkStart
}

[pstanek, 2013-08-19 21:22:00]

On 2013/08/19 20:01:15, tony wrote:
> https://codereview.chromium.org/23332004/diff/1/Source/core/editing/Editor.cpp
> File Source/core/editing/Editor.cpp (right):
> 
>
https://codereview.chromium.org/23332004/diff/1/Source/core/editing/Editor.cp...
> Source/core/editing/Editor.cpp:1494: if (kNumChunksToCheck > 1 ||
!asynchronous)
> {
> Having if (kNumChunksToCheck) conditions in the for loop is weird.  It would
be
> better if we only entered the for loop if kNumChunksToCheck > 1.  Maybe
> restructure it to something like:
> 
> if (kNumChunksToCheck <= 1) {
>     markAllMisspellingsAndBadGrammarInRange(range, asynchronous, ...)
>     return;
> }
> 
> for (int iter = 0; iter < kNumChunksToCheck; ++iter) {
>     // compute range to check
>     markAllMisspellingsAndBadGrammarInRange(range, asynchronous, ...)
>     // update currentChunkStart
> }

PTAL

[tony, 2013-08-19 21:50:01]

LGTM

https://codereview.chromium.org/23332004/diff/9001/Source/core/editing/Editor...
File Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/23332004/diff/9001/Source/core/editing/Editor...
Source/core/editing/Editor.cpp:1494:
markAllMisspellingsAndBadGrammarInRanges(textCheckingOptions, checkRange.get(),
paragraphRange.get(), true, 0);
Nit: true -> asynchronous.

[tony, 2013-08-19 21:51:58]

Please edit the change description to say what is different from the previous
patch.  E.g., some text like, "Optimize single chunk case to try to avoid a
performance regression found by DOM/textarea-edit.html."

[commit-bot: I haz the power, 2013-08-20 06:49:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pstanek@opera.com/23332004/15001

[commit-bot: I haz the power, 2013-08-20 07:02:27]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-08-20 07:15:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pstanek@opera.com/23332004/15001

[commit-bot: I haz the power, 2013-08-20 07:21:15]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-08-20 07:25:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pstanek@opera.com/23332004/15001

[commit-bot: I haz the power, 2013-08-20 07:35:54]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-08-20 07:39:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pstanek@opera.com/23332004/15001

[commit-bot: I haz the power, 2013-08-20 09:01:21]

Change committed as 156378