Get rid of verify-with-timeout in spellcheck_test

Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819
Committed: https://crrev.com/d54cef0fcd7ce30563f7abcd07a80b0958d7e504
Cr-Commit-Position: refs/heads/master@{#440058}

[Xiaocheng, 2016-12-21 06:02:45]

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

[commit-bot: I haz the power, 2016-12-21 06:03:01]

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

[Xiaocheng, 2016-12-21 06:15:43]

Description was changed from

==========
Rewrite spellcheck_test.js for better spell-checking layout tests

BUG=674819
==========

to

==========
Rewrite spellcheck_test.js for better spell-checking layout tests

This patch rewrites spellcheck_test.js that:

1. Marker verification used to be setting a timeout and wait. Now it's done
with testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks

2. The |Test| object for each spellcheck_test is advanced as much as possible,
right inside spellcheckTest(), to prevent testharness from terminating early.

3. Even if a test fails, its sample is still removed as long as we are running
tests (i.e., with testRunner). This is for minimizing interference between test
cases since unremoved sample affects visibility, while idle time spell checker
relies on visibility.

BUG=674819
==========

[Xiaocheng, 2016-12-21 06:54:19]

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

[commit-bot: I haz the power, 2016-12-21 06:54:31]

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

[Xiaocheng, 2016-12-21 06:56:39]

Description was changed from

==========
Rewrite spellcheck_test.js for better spell-checking layout tests

This patch rewrites spellcheck_test.js that:

1. Marker verification used to be setting a timeout and wait. Now it's done
with testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks

2. The |Test| object for each spellcheck_test is advanced as much as possible,
right inside spellcheckTest(), to prevent testharness from terminating early.

3. Even if a test fails, its sample is still removed as long as we are running
tests (i.e., with testRunner). This is for minimizing interference between test
cases since unremoved sample affects visibility, while idle time spell checker
relies on visibility.

BUG=674819
==========

to

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking unit tests, ,marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spellcheck tests run faster after this patch.

BUG=674819
==========

[Xiaocheng, 2016-12-21 06:57:10]

Description was changed from

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking unit tests, ,marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spellcheck tests run faster after this patch.

BUG=674819
==========

to

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, ,marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819
==========

[Xiaocheng, 2016-12-21 07:35:23]

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

[Xiaocheng, 2016-12-21 07:35:23]

PTAL.

[tkent, 2016-12-21 07:42:55]

lgtm

> For spell-checking layout tests, ,marker verification used to be setting

Remove ',' before 'marker'

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js (right):

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js:316: /**
@type {!Array<!Function>} Verification functions of tests cases whose idle
nit: For a multiline annotation, please fold after "/**". e.g.

/**
 * @type {!Array<!Function>} ...
 * ....
 */

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js:320: /**
@type {!Array<!Function>} Verification functions of tests cases whose idle
Ditto.

[Xiaocheng, 2016-12-21 07:45:55]

Description was changed from

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, ,marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819
==========

to

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819
==========

[Xiaocheng, 2016-12-21 07:51:43]

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

[Xiaocheng, 2016-12-21 07:52:00]

Thanks for the review!

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js (right):

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js:316: /**
@type {!Array<!Function>} Verification functions of tests cases whose idle
On 2016/12/21 at 07:42:55, slow_or_unavailable wrote:
> nit: For a multiline annotation, please fold after "/**". e.g.
> 
> /**
>  * @type {!Array<!Function>} ...
>  * ....
>  */

Done.

https://codereview.chromium.org/2590823006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js:320: /**
@type {!Array<!Function>} Verification functions of tests cases whose idle
On 2016/12/21 at 07:42:55, slow_or_unavailable wrote:
> Ditto.

Done.

[commit-bot: I haz the power, 2016-12-21 07:52:01]

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

[Xiaocheng, 2016-12-21 08:10:13]

The CQ bit was unchecked by xiaochengh@chromium.org

[Xiaocheng, 2016-12-21 08:10:15]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2016-12-21 08:10:15]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2590823006/#ps40001
(title: "Wed Dec 21 16:51:05 JST 2016")

[commit-bot: I haz the power, 2016-12-21 08:10:24]

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, 2016-12-21 08:59:24]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482307815543260,
"parent_rev": "76545ab44ca836a4338a28d879d08a1a3ab9d84e", "commit_rev":
"e84e80872c2c5fe9429f7ec1ed6aa095e9a73cc0"}

[commit-bot: I haz the power, 2016-12-21 08:59:52]

Description was changed from

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819
==========

to

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819

Review-Url: https://codereview.chromium.org/2590823006
==========

[commit-bot: I haz the power, 2016-12-21 08:59:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-21 09:02:24]

Description was changed from

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819

Review-Url: https://codereview.chromium.org/2590823006
==========

to

==========
Get rid of verify-with-timeout in spellcheck_test

For spell-checking layout tests, marker verification used to be setting
a timeout and wait. This patch utilizes
testRunner.setSpellCheckResolvedCallback, and if no new marker is expected
(in which case there might be no request), testRunner.runIdleTasks, to
reduce futile waiting.

We expect to see spell-checking tests run faster after this patch.

BUG=674819

Committed: https://crrev.com/d54cef0fcd7ce30563f7abcd07a80b0958d7e504
Cr-Commit-Position: refs/heads/master@{#440058}
==========

[commit-bot: I haz the power, 2016-12-21 09:02:24]

Patchset 3 (id:??) landed as
https://crrev.com/d54cef0fcd7ce30563f7abcd07a80b0958d7e504
Cr-Commit-Position: refs/heads/master@{#440058}

[Xiaocheng, 2016-12-21 10:22:59]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2593933003/ by xiaochengh@chromium.org.

The reason for reverting is: Introducing flakiness to WebKit Linux Trusty Leak:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=....