Refactor touch hit-testing hover/active tests.

A few touch hit-testing hover/active tests use the same setup code. This patch
consolidates that into a separate file and also updates the generated gesture
sequences to be valid.

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

[Zeeshan Qureshi, 2014-03-13 22:43:42]

Tim: Can you go over the test output and see if it looks sane.

[tdresser, 2014-03-14 14:24:51]

Test output all looks good.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
File LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html
(right):

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html:39:
box = document.getElementById("frame").contentDocument.getElementById('box');
Why do you just pass the id to isBox* sometimes, but cache the object other
times? I think I'd prefer just always caching it, and getting rid of
|getElement|.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
File LayoutTests/fast/events/touch/gesture/gesture-tap-active-state.html
(right):

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/gesture/gesture-tap-active-state.html:109:
eventSender.gestureTapCancel(50, 50);
Assert on the box state after the tap cancel.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
File LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js
(right):

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:22: function
elementCenter(e)
Where is this used?

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:31: //
Generate methods
Could switch to "boxShouldBeOfColor," which would include the "shouldBeTrue"
call. Might make things a bit more readable in the tests.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:35:
window.isBoxHoverActive = isBoxOfColor("rgb(255, 255, 0)");
How about "isBoxHoveredAndActive"?

[Zeeshan Qureshi, 2014-03-14 14:52:20]

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
File LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html
(right):

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html:39:
box = document.getElementById("frame").contentDocument.getElementById('box');
On 2014/03/14 14:24:51, tdresser wrote:
> Why do you just pass the id to isBox* sometimes, but cache the object other
> times? I think I'd prefer just always caching it, and getting rid of
> |getElement|.

Yeah, makes sense. Better than modifying getElement to handle frames.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
File LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js
(right):

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:22: function
elementCenter(e)
On 2014/03/14 14:24:51, tdresser wrote:
> Where is this used?

Oh I was using this in another CL and decided to split this into a separate one.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:31: //
Generate methods
On 2014/03/14 14:24:51, tdresser wrote:
> Could switch to "boxShouldBeOfColor," which would include the "shouldBeTrue"
> call. Might make things a bit more readable in the tests.

I'm not quite following this, wouldn't the test output not include the
isBox/Hovered/Active/etc then? Don't we want to explicitly state in the test
output what state we're checking.

https://codereview.chromium.org/197923006/diff/1/LayoutTests/fast/events/touc...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:35:
window.isBoxHoverActive = isBoxOfColor("rgb(255, 255, 0)");
On 2014/03/14 14:24:51, tdresser wrote:
> How about "isBoxHoveredAndActive"?

That sounds better.

[Rick Byers, 2014-03-14 17:01:02]

Looks like a good clean-up to me, thanks!

https://codereview.chromium.org/197923006/diff/20001/LayoutTests/fast/events/...
File LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html
(right):

https://codereview.chromium.org/197923006/diff/20001/LayoutTests/fast/events/...
LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html:47:
debug("Verify tapdown,tap sets and clears active");
s/tapdown/showPress/

https://codereview.chromium.org/197923006/diff/20001/LayoutTests/fast/events/...
File LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js
(right):

https://codereview.chromium.org/197923006/diff/20001/LayoutTests/fast/events/...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:9:
testFailed('Box had backgroundColor: ' + color);
seems like it would be a lot more understandable if the test reported the
observed state instead of the color (which is just an implementation detail) in
case of a failure.  Maybe have a list mapping colors to state names (eg.
'active')?

https://codereview.chromium.org/197923006/diff/20001/LayoutTests/fast/events/...
LayoutTests/fast/events/touch/resources/touch-hover-active-tests.js:24:
window.isBoxHovered = isBoxOfColor("rgb(255, 0, 0)");
the name of this function and that below is a little misleading (i.e.
isBoxHovered returns false for something that's hovered and also active).  You
should at least have a comment making that clear, or update the name.

Combined with the above you could just have a single getHoverActiveState
function that returns a string that callers check.

[tdresser, 2014-03-20 14:01:04]

On 2014/03/20 13:40:44, Zeeshan Qureshi wrote:

LGTM.

[Rick Byers, 2014-03-24 15:03:49]

I like this a lot, thanks!
LGTM

[Zeeshan Qureshi, 2014-03-24 15:04:58]

The CQ bit was checked by zeeshanq@chromium.org

[commit-bot: I haz the power, 2014-03-24 15:05:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/197923006/40001

[Zeeshan Qureshi, 2014-03-24 15:06:40]

The CQ bit was unchecked by zeeshanq@chromium.org

[Zeeshan Qureshi, 2014-03-24 15:07:51]

The CQ bit was checked by zeeshanq@chromium.org

[commit-bot: I haz the power, 2014-03-24 15:07:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/197923006/40001

[commit-bot: I haz the power, 2014-03-24 15:36:48]

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

[commit-bot: I haz the power, 2014-03-24 15:36:48]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[Zeeshan Qureshi, 2014-03-24 15:37:44]

The CQ bit was checked by zeeshanq@chromium.org

[commit-bot: I haz the power, 2014-03-24 15:37:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/197923006/40001

[commit-bot: I haz the power, 2014-03-24 16:07:40]

Change committed as 169858