Make sure send one WebTouchEvent ack per ui::TouchEvent

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Index: content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index aba2b57322ef07d0fe0960cff38befe6ac9c07b5..8e5cfd643e7c890f797b1b681bafc796bee7dd29 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -303,6 +303,7 @@ class FakeRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
       touch_event_.reset(

[jdduke (slow), 2015/05/07 15:20:28]

Hmm, why do have separate bookkeeping here for touches? This increases the
maintenance burden on the tests and makes it more likely for tested behavior to
deviate from actual behavior.

[lanwei, 2015/05/08 02:12:32]

Tim, I saw that you added this touch_event_, can you please explain why you
added this and reset it here every time? Thanks.

[tdresser, 2015/05/08 11:59:23]

This was put here to avoid code churn (or out of laziness, depending on your
perspective). We used to keep track of state in RWHVAura using a WebTouchEvent,
so all the tests were written with expectations on a WebTouchEvent. When I
switched us over to storing the state using a MotionEventAura, I added this
WebTouchEvent so we could avoid modifying all the tests.

I think it did make sense to avoid conflating a bunch of test churn with a
functional change, but it's true that in the long term this would be simpler
without this additional logic.

I've filed http://crbug.com/485968 to address this cleanup. Lan, I've put it on
my plate for now, but if you want to take it go ahead.

           new blink::WebTouchEvent(ui::CreateWebTouchEventFromMotionEvent(
               pointer_state(), event->may_cause_scrolling())));
+      SetTouchPointStateStationary(touch_event_.get(), event->touch_id());
     } else {
       // Never create a WebTouchEvent with 0 touch points.
       touch_event_.reset();
@@ -1124,6 +1125,76 @@ TEST_F(RenderWidgetHostViewAuraTest, TouchEventState) {
   EXPECT_EQ(nullptr, view_->touch_event_);
 }
 
+// Checks that touch-event state is maintained correctly for multiple touch
+// points.
+TEST_F(RenderWidgetHostViewAuraTest, MultiTouchPointsStates) {
+  view_->InitAsChild(NULL);
+  view_->Show();
+  GetSentMessageCountAndResetSink();
+
+  ui::TouchEvent press(ui::ET_TOUCH_PRESSED, gfx::Point(30, 30), 0,
+                       ui::EventTimeForNow());
+
+  view_->OnTouchEvent(&press);
+  EXPECT_EQ(blink::WebInputEvent::TouchStart, view_->touch_event_->type);
+  EXPECT_EQ(1U, view_->touch_event_->touchesLength);
+  EXPECT_EQ(blink::WebTouchPoint::StatePressed,
+            view_->touch_event_->touches[0].state);
+
+  ui::TouchEvent move(ui::ET_TOUCH_MOVED, gfx::Point(20, 20), 0,
+                      ui::EventTimeForNow());
+
+  view_->OnTouchEvent(&move);
+  EXPECT_EQ(blink::WebInputEvent::TouchMove, view_->touch_event_->type);
+  EXPECT_EQ(1U, view_->touch_event_->touchesLength);
+  EXPECT_EQ(blink::WebTouchPoint::StateMoved,
+            view_->touch_event_->touches[0].state);
+
+  // For the second touchstart, only the state of the second touch point is
+  // StatePressed, the state of the first touch point is StateStationary.
+  ui::TouchEvent press1(ui::ET_TOUCH_PRESSED, gfx::Point(10, 10), 1,

[tdresser, 2015/05/07 12:13:06]

nit: Can we use press1/press2 (or press0/press1) instead of press/press1?

[lanwei, 2015/05/08 02:12:32]

Done.

+                        ui::EventTimeForNow());
+
+  view_->OnTouchEvent(&press1);
+  EXPECT_EQ(blink::WebInputEvent::TouchStart, view_->touch_event_->type);
+  EXPECT_EQ(2U, view_->touch_event_->touchesLength);
+  EXPECT_EQ(blink::WebTouchPoint::StateStationary,
+            view_->touch_event_->touches[0].state);
+  EXPECT_EQ(blink::WebTouchPoint::StatePressed,
+            view_->touch_event_->touches[1].state);
+
+  // For the second touchmove, only the state of the second touch point is
+  // StateMoved, the state of the first touch point is StateStationary.
+  ui::TouchEvent move1(ui::ET_TOUCH_MOVED, gfx::Point(30, 30), 1,
+                       ui::EventTimeForNow());
+
+  view_->OnTouchEvent(&move1);
+  EXPECT_EQ(blink::WebInputEvent::TouchMove, view_->touch_event_->type);
+  EXPECT_EQ(2U, view_->touch_event_->touchesLength);
+  EXPECT_EQ(blink::WebTouchPoint::StateStationary,
+            view_->touch_event_->touches[0].state);
+  EXPECT_EQ(blink::WebTouchPoint::StateMoved,
+            view_->touch_event_->touches[1].state);
+

[tdresser, 2015/05/07 12:13:06]

Can we add an additional move here, to test the case where both pointers move,
one after the other?

[lanwei, 2015/05/08 02:12:32]

Done.

+  ui::TouchEvent cancel(ui::ET_TOUCH_CANCELLED, gfx::Point(20, 20), 0,
+                        ui::EventTimeForNow());
+
+  // For the touchcancel, only the state of the current touch point is
+  // StateCancelled, the state of the other touch point is StateStationary.
+  view_->OnTouchEvent(&cancel);
+  EXPECT_EQ(blink::WebInputEvent::TouchCancel, view_->touch_event_->type);
+  EXPECT_EQ(1U, view_->touch_event_->touchesLength);
+  EXPECT_EQ(blink::WebTouchPoint::StateStationary,
+            view_->touch_event_->touches[0].state);
+  EXPECT_EQ(1, view_->touch_event_->touches[0].id);
+
+  ui::TouchEvent cancel1(ui::ET_TOUCH_CANCELLED, gfx::Point(30, 30), 1,
+                         ui::EventTimeForNow());
+
+  view_->OnTouchEvent(&cancel1);
+  EXPECT_EQ(nullptr, view_->touch_event_);
+}
+
 // Checks that touch-events are queued properly when there is a touch-event
 // handler on the page.
 TEST_F(RenderWidgetHostViewAuraTest, TouchEventSyncAsync) {