Implement cold mode invocation for idle time spell checker

Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

It also removes editing/spelling/focusing-other-frame.html as it's
not testing anything special (with idle time spell checker), but
unnecessarily complicates the test logic.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html

Review-Url: https://codereview.chromium.org/2734013002
Cr-Commit-Position: refs/heads/master@{#455344}
Committed: https://chromium.googlesource.com/chromium/src/+/2483390c3c56d62a8d65bb3bb3672102a4956505

[Xiaocheng, 2017-03-07 00:22:47]

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

[commit-bot: I haz the power, 2017-03-07 00:23:34]

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-07 00:27:57]

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

[Xiaocheng, 2017-03-07 00:27:58]

PTAL.

[yosin_UTC9, 2017-03-07 01:47:03]

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

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:163:
int fullLength = TextIterator::rangeLength(fullRange.startPosition(),
nit: s/int/const int/

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:176:
int chunkLimit = fullLength / kColdModeChunkSize + 1;
nit: s/int/const int/

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:177:
for (int requestNum = 0; requestNum <= chunkLimit; ++requestNum) {
nit: s/requestNum/chunkNumber/
or
nit: s/requestNum/chunkIndex/

I feel comparing |requestNum| and |chunkLimit| strange since relation between
"request" and "chunk" isn't clear.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:178:
Position chunkEnd = calculateCharacterSubrange(
nit: s/Position/const Position/

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:184:
EphemeralRange chunkRange(chunkStart, chunkEnd);
nit: s/EphemeralRange/const EphmeralRange/

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:185:
EphemeralRange checkRange = requestNum
nit: s/EphmeralRange/const EphemeralRange&/ to avoid copy
nit: s/requstnum/requestNum >= 1/ to say non-zero explicitly

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:203:
Node* body = frame().document()->body();
|body| can be null, e.g. no BODY element.
We may want to check this at entry of this function before update layout.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:217:
} else {
Prefer early-continue style to shallow indentation.

[commit-bot: I haz the power, 2017-03-07 02:07:04]

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

[commit-bot: I haz the power, 2017-03-07 02:07:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Xiaocheng, 2017-03-07 04:19:41]

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

[Xiaocheng, 2017-03-07 04:20:30]

Updated.

The unit test failure is a little bit tricky to fix... PTAL at everything but
that.

I'll find a fix to the unit test failure.

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

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:163:
int fullLength = TextIterator::rangeLength(fullRange.startPosition(),
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/int/const int/

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:176:
int chunkLimit = fullLength / kColdModeChunkSize + 1;
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/int/const int/

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:177:
for (int requestNum = 0; requestNum <= chunkLimit; ++requestNum) {
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/requestNum/chunkNumber/
> or
> nit: s/requestNum/chunkIndex/
> 
> I feel comparing |requestNum| and |chunkLimit| strange since relation between
"request" and "chunk" isn't clear.

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:178:
Position chunkEnd = calculateCharacterSubrange(
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/Position/const Position/

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:184:
EphemeralRange chunkRange(chunkStart, chunkEnd);
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/EphemeralRange/const EphmeralRange/

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:185:
EphemeralRange checkRange = requestNum
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> nit: s/EphmeralRange/const EphemeralRange&/ to avoid copy
> nit: s/requstnum/requestNum >= 1/ to say non-zero explicitly

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:203:
Node* body = frame().document()->body();
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> |body| can be null, e.g. no BODY element.
> We may want to check this at entry of this function before update layout.

Done.

https://codereview.chromium.org/2734013002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:217:
} else {
On 2017/03/07 at 01:47:03, yosin_UTC9 wrote:
> Prefer early-continue style to shallow indentation.

Done by introducing |shouldCheckNodeInColdMode()|

[commit-bot: I haz the power, 2017-03-07 04:20:35]

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

[yosin_UTC9, 2017-03-07 04:28:50]

lgtm

https://codereview.chromium.org/2734013002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
(right):

https://codereview.chromium.org/2734013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:201:
spellCheckRequester().requestCheckingFor(chunkRequest);
In following patch, we may want to make
SpellCheckerRequester::requestCheckingFor() to take EphmeralRange and
requestNumber instead of SpellCheckRequest.

All call sites, including this one, don't use |SpellCheckRequest*| after
requested.

[commit-bot: I haz the power, 2017-03-07 05:51:19]

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

[commit-bot: I haz the power, 2017-03-07 05:51:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Xiaocheng, 2017-03-07 06:46:44]

Description was changed from

==========
Implement cold mode invocation for idle time spell checker

BUG=517298
==========

to

==========
Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html
==========

[Xiaocheng, 2017-03-07 06:46:54]

https://codereview.chromium.org/2734013002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
(right):

https://codereview.chromium.org/2734013002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp:201:
spellCheckRequester().requestCheckingFor(chunkRequest);
On 2017/03/07 at 04:28:50, yosin_UTC9 wrote:
> In following patch, we may want to make
> SpellCheckerRequester::requestCheckingFor() to take EphmeralRange and
requestNumber instead of SpellCheckRequest.
> 
> All call sites, including this one, don't use |SpellCheckRequest*| after
requested.

Good catch. Will do.

[Xiaocheng, 2017-03-07 06:47:00]

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

[Xiaocheng, 2017-03-07 06:47:55]

Description was changed from

==========
Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html
==========

to

==========
Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

It also removes editing/spelling/focusing-other-frame.html as it's
not testing anything special (with idle time spell checker), but
unnecessarily complicates the test logic.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html
==========

[commit-bot: I haz the power, 2017-03-07 06:48:07]

Dry run: 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-03-07 09:26:34]

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

[commit-bot: I haz the power, 2017-03-07 09:26:35]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Xiaocheng, 2017-03-07 21:28:01]

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

[commit-bot: I haz the power, 2017-03-07 21:28:53]

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-08 01:53:54]

The CQ bit was unchecked by xiaochengh@chromium.org

[Xiaocheng, 2017-03-08 01:53:56]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-03-08 01:53:57]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2734013002/#ps60001
(title: "rebased Mar 7")

[commit-bot: I haz the power, 2017-03-08 01:54:28]

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-03-08 02:11:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488938036984380,
"parent_rev": "c87999ea945e86d70cc5f98df81d65efc567cdd5", "commit_rev":
"2483390c3c56d62a8d65bb3bb3672102a4956505"}

[commit-bot: I haz the power, 2017-03-08 02:12:07]

Description was changed from

==========
Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

It also removes editing/spelling/focusing-other-frame.html as it's
not testing anything special (with idle time spell checker), but
unnecessarily complicates the test logic.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html
==========

to

==========
Implement cold mode invocation for idle time spell checker

This patch implements IdleSpellCheckCallback::coldModeInvocation and
its helper functions.

It also removes editing/spelling/focusing-other-frame.html as it's
not testing anything special (with idle time spell checker), but
unnecessarily complicates the test logic.

BUG=517298
TEST=editing/spelling/cold_mode_static_page.html
editing/spelling/code_mode_type_idle.html

Review-Url: https://codereview.chromium.org/2734013002
Cr-Commit-Position: refs/heads/master@{#455344}
Committed:
https://chromium.googlesource.com/chromium/src/+/2483390c3c56d62a8d65bb3bb367...
==========

[commit-bot: I haz the power, 2017-03-08 02:12:08]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/2483390c3c56d62a8d65bb3bb367...