Ignore TouchCancel ack dispositions

Ignore TouchCancel ack dispositions

TouchCancel ack dispositions have no bearing on touch or gesture input
behavior.  As such, ignore the disposition, eliminating unnecessary blocking
of touch event dispatch.

BUG=302849, 302852
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236849

[jdduke (slow), 2013-11-21 02:38:53]

PTAL. I might as well land this separately from
https://codereview.chromium.org/48973005

[tdresser, 2013-11-21 13:54:30]

LGTM, with a few nits.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/immediate_input_router_unittest.cc
(right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/immediate_input_router_unittest.cc:596: if
(WebInputEventTraits::IgnoresAckDisposition(type)) {
Can you reverse this condition as you have above? It would be nice to be
consistent, and what you've done above is superior.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/input_router_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/input_router_unittest.cc:132: default:
nit - add a NOTREACHED() here.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:172:
scoped_ptr<InputEventAckState> sync_ack_result_;
Why bother with a pointer here, when |InputEventAckState| is just an enum?

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:634: // Tests
that touch-event's can be synchronously ack'ed.
s/event's/events

[jdduke (slow), 2013-11-21 16:55:29]

tedchoc@: Review for ContentViewGestureHandler? Thanks.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/immediate_input_router_unittest.cc
(right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/immediate_input_router_unittest.cc:596: if
(WebInputEventTraits::IgnoresAckDisposition(type)) {
On 2013/11/21 13:54:31, tdresser wrote:
> Can you reverse this condition as you have above? It would be nice to be
> consistent, and what you've done above is superior.

Done.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/input_router_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/input_router_unittest.cc:132: default:
On 2013/11/21 13:54:31, tdresser wrote:
> nit - add a NOTREACHED() here.

Hmm, I have the assert above for type being a valid touch event, and we don't
want to do anything for TouchStart, but I guess I can be more explicit there.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:172:
scoped_ptr<InputEventAckState> sync_ack_result_;
On 2013/11/21 13:54:31, tdresser wrote:
> Why bother with a pointer here, when |InputEventAckState| is just an enum?

scoped_ptr is as close as we have to an "optional<T>" type.  I suppose I could
just use the UNKNOWN type, but I prefer to be explicit about whether or not it
has been specified (and I'm not really worried about a heap alloc here).

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:634: // Tests
that touch-event's can be synchronously ack'ed.
On 2013/11/21 13:54:31, tdresser wrote:
> s/event's/events

Done.

[tdresser, 2013-11-21 17:02:47]

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/input_router_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/input_router_unittest.cc:132: default:
On 2013/11/21 16:55:29, jdduke wrote:
> On 2013/11/21 13:54:31, tdresser wrote:
> > nit - add a NOTREACHED() here.
> 
> Hmm, I have the assert above for type being a valid touch event, and we don't
> want to do anything for TouchStart, but I guess I can be more explicit there.

Ah, good point. I do like this somewhat better though.

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right):

https://codereview.chromium.org/78263003/diff/40001/content/browser/renderer_...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:172:
scoped_ptr<InputEventAckState> sync_ack_result_;
On 2013/11/21 16:55:29, jdduke wrote:
> On 2013/11/21 13:54:31, tdresser wrote:
> > Why bother with a pointer here, when |InputEventAckState| is just an enum?
> 
> scoped_ptr is as close as we have to an "optional<T>" type.  I suppose I could
> just use the UNKNOWN type, but I prefer to be explicit about whether or not it
> has been specified (and I'm not really worried about a heap alloc here).

Right, sounds good.

[Ted C, 2013-11-21 17:23:32]

lgtm - ContentViewGestureHandler.java

[commit-bot: I haz the power, 2013-11-22 20:23:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/78263003/240001

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

Change committed as 236849