Refactoring inline_spelling_markers.html to use async path for spellcheck

Refactoring inline_spelling_markers.html to use async path for spellcheck

This change mostly adapts inline_spelling_markers.html to use asynchronous
path for spellchecking which will be enabled by default.
To achieve this, shouldBecomeEqual helper has been used.

Additionally, make cross platform text expectation by dumping
spelling/grammar markers instead of whole tree which in terms
of spellchecking says nothing.

Enabling unified text checker and adjusting MockGrammarCheck.cpp
allow us to restore grammar checking in inline_spelling_markers.html.

Remove unnecessary new lines in div elements so position of markers
can be given from 0.

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

[tony, 2013-10-29 17:16:47]

LGTM

[groby-ooo-7-16, 2013-10-29 17:19:05]

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
File LayoutTests/editing/spelling/inline_spelling_markers.html (right):

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
LayoutTests/editing/spelling/inline_spelling_markers.html:72: for (var i = 0; i
< div.innerHTML.length; i++ )
Moving over words should be enough - does the test fail when you do that
instead, or did you pick the character movement because you don't need to guess
at the upper bound?

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
LayoutTests/editing/spelling/inline_spelling_markers.html:90: div.blur();
Is the blur still needed?

https://codereview.chromium.org/49613006/diff/1/Source/testing/runner/MockGra...
File Source/testing/runner/MockGrammarCheck.cpp (right):

https://codereview.chromium.org/49613006/diff/1/Source/testing/runner/MockGra...
Source/testing/runner/MockGrammarCheck.cpp:68: {"the the adlj adaasj sdklj.
there there", 4, 3},
I'm not sure why we need a second test case. Both simply test that reporting a
grammar error at a specific location results in a green underline.

[grzegorz, 2013-10-30 15:47:47]

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
File LayoutTests/editing/spelling/inline_spelling_markers.html (right):

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
LayoutTests/editing/spelling/inline_spelling_markers.html:72: for (var i = 0; i
< div.innerHTML.length; i++ )
On 2013/10/29 17:19:05, groby wrote:
> Moving over words should be enough - does the test fail when you do that
> instead, or did you pick the character movement because you don't need to
guess
> at the upper bound?

You're right. Moving over the words is enough. I'll restore the origin code. The
purpose of this change was to avoid changing selection more times that we really
need.

https://codereview.chromium.org/49613006/diff/1/LayoutTests/editing/spelling/...
LayoutTests/editing/spelling/inline_spelling_markers.html:90: div.blur();
On 2013/10/29 17:19:05, groby wrote:
> Is the blur still needed?

Good point, hasGrammarMarker/hasSpellingMarker fail without blur event here
apart from first element (testLTR). It looks like selection change influences on
that and its markers.size() is 0. Without calling selection.modify(...)
everything seems fine and blur event is unnecessary.

https://codereview.chromium.org/49613006/diff/1/Source/testing/runner/MockGra...
File Source/testing/runner/MockGrammarCheck.cpp (right):

https://codereview.chromium.org/49613006/diff/1/Source/testing/runner/MockGra...
Source/testing/runner/MockGrammarCheck.cpp:68: {"the the adlj adaasj sdklj.
there there", 4, 3},
On 2013/10/29 17:19:05, groby wrote:
> I'm not sure why we need a second test case. Both simply test that reporting a
> grammar error at a specific location results in a green underline.

I think we should report grammar error where it exactly is, similarly to other
examples here. That looks better in pixel result. Otherwise the whole sentence
will be marked in green but in inline_spelling_markers.html we expect grammar
error at (4, 3) and (32, 5)

[grzegorz, 2013-10-31 13:55:22]

Two thing came to mind after patch#2
1) we could apply those changes to its brother
(inline-spelling-markers-hidpi.html) if they are ready to go
2) we should rename inline_spelling_markers.html using '-' instead of '_'

[groby-ooo-7-16, 2013-11-01 01:13:49]

lgtm

Since this doesn't seem to

[groby-ooo-7-16, 2013-11-01 01:14:32]

Gah. Ignore trailing text. This does indeed look fine to me, and if you can roll
this to the hidpi tests, you should.

[commit-bot: I haz the power, 2013-11-01 16:23:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/g.czajkowski@samsung.com/49613006/100001

[commit-bot: I haz the power, 2013-11-01 16:23:34]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 531.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
5f84a1b55a9b263d721f46ba20eba2bd9e3a18e3..c56cb54c462e5d58681325d1850a8d8916f4a7b0
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -531,3 +531,5 @@ crbug.com/312060 [ Win ]
virtual/threaded/animations/3d/matrix-transform-type-an
 crbug.com/312060 [ Win ]
virtual/threaded/animations/additive-transform-animations.html [
ImageOnlyFailure ]
 crbug.com/312060 [ Win ]
virtual/threaded/animations/keyframe-multiple-timing-functions-transform.html [
ImageOnlyFailure ]
 crbug.com/312060 [ Win ]
virtual/threaded/animations/missing-values-first-keyframe.html [
ImageOnlyFailure ]
+
+Bug(grzegorz) [ Linux Mac Win ] editing/spelling/inline_spelling_markers.html [
NeedsRebaseline ]

[grzegorz, 2013-11-04 08:52:12]

On 2013/11/01 01:14:32, groby wrote:
> Gah. Ignore trailing text. This does indeed look fine to me, and if you can
roll
> this to the hidpi tests, you should.

I will apply those changes to hidpi test in separate commit.

[commit-bot: I haz the power, 2013-11-04 17:55:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/g.czajkowski@samsung.com/49613006/190001

[tony, 2013-11-04 18:00:22]

https://codereview.chromium.org/49613006/diff/190001/LayoutTests/editing/spel...
File LayoutTests/editing/spelling/inline_spelling_markers.html (right):

https://codereview.chromium.org/49613006/diff/190001/LayoutTests/editing/spel...
LayoutTests/editing/spelling/inline_spelling_markers.html:109: <script
src="../../fast/js/resources/js-test-post.js"></script>
Oops, this is now gone.  Please remove and reupload.

[grzegorz, 2013-11-06 07:46:07]

https://codereview.chromium.org/49613006/diff/190001/LayoutTests/editing/spel...
File LayoutTests/editing/spelling/inline_spelling_markers.html (right):

https://codereview.chromium.org/49613006/diff/190001/LayoutTests/editing/spel...
LayoutTests/editing/spelling/inline_spelling_markers.html:109: <script
src="../../fast/js/resources/js-test-post.js"></script>
On 2013/11/04 18:00:22, tony wrote:
> Oops, this is now gone.  Please remove and reupload.

Done.

[commit-bot: I haz the power, 2013-11-06 07:57:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/g.czajkowski@samsung.com/49613006/340001

[commit-bot: I haz the power, 2013-11-06 09:10:15]

Change committed as 161436