Avoid sending touchmove events where all pointers are stationary

content/browser/renderer_host/input/touch_event_queue.cc

Index: content/browser/renderer_host/input/touch_event_queue.cc
diff --git a/content/browser/renderer_host/input/touch_event_queue.cc b/content/browser/renderer_host/input/touch_event_queue.cc
index 6adc661be18c5394f4571ac2d037c11b74133cda..83a6af6baf85377689d4dd705732e2ce303c9074 100644
--- a/content/browser/renderer_host/input/touch_event_queue.cc
+++ b/content/browser/renderer_host/input/touch_event_queue.cc
@@ -46,14 +46,14 @@ bool ShouldTouchTriggerTimeout(const WebTouchEvent& event) {
 }
 
 // Compare all properties of touch points to determine the state.
-bool HasPointChanged(const WebTouchPoint& last_point,
-    const WebTouchPoint& current_point) {
-  if (last_point.screenPosition != current_point.screenPosition ||
-      last_point.position != current_point.position ||
-      last_point.radiusX != current_point.radiusX ||
-      last_point.radiusY != current_point.radiusY ||
-      last_point.rotationAngle != current_point.rotationAngle ||
-      last_point.force != current_point.force) {
+bool HasPointChanged(const WebTouchPoint& point_1,
+    const WebTouchPoint& point_2) {

[jdduke (slow), 2015/02/11 17:17:12]

Nit: Will this now fit on the same line?

Also, maybe we should add a DCHECK_EQ(point_1.id, point_2.id).

[USE s.singapati at gmail.com, 2015/02/12 11:15:02]

line exceeds 80.
yes, DCHECK_EQ is fine.

[USE s.singapati at gmail.com, 2015/02/13 12:16:48]

Done.

+  if (point_1.screenPosition != point_2.screenPosition ||
+      point_1.position != point_2.position ||
+      point_1.radiusX != point_2.radiusX ||
+      point_1.radiusY != point_2.radiusY ||
+      point_1.rotationAngle != point_2.rotationAngle ||
+      point_1.force != point_2.force) {
     return true;
   }
   return false;
@@ -729,9 +729,22 @@ TouchEventQueue::FilterBeforeForwarding(const WebTouchEvent& event) {
         continue;
 
       for (size_t j = 0; j < last_sent_touchevent_->touchesLength; ++j) {
-        if (point.id == last_sent_touchevent_->touches[j].id)
+        if (point.id != last_sent_touchevent_->touches[j].id)
+          continue;
+
+        // It is possible that all the pointers in a given TouchMove may have
+        // state as StateMoved, even though none of the pointers have not
+        // changed in real. Avoid sending these events to the renderer.
+        if (event.type == WebInputEvent::TouchMove) {
+          if (HasPointChanged(last_sent_touchevent_->touches[j], point))

[jdduke (slow), 2015/02/11 17:17:12]

Can we invert this condition?

if (event.type == WebInputEvent::TouchMove) {
  if (!HasPointChanged(...))
    return ACK_WITH_NOT_CONSUMED
}
return FORWARD_TO_RENDERER;

Simple to follow, and no need for the break statement.

[USE s.singapati at gmail.com, 2015/02/12 11:15:02]

Hmm..I think not. ACK_WITH_NOT_CONSUMED should not be sent YET here, because
next pointers in the TouchMove may have been changed in real.
Probably need to update comment for break; statement like
"Pointer with same id found. If this is a TouchMove, then event may not have
been forwarded yet. So, continue handling next pointers in the |event|."

And if the requirement is really to send ACK_WITH_NOT_CONSUMED, then this should
be done at the end of method.

if (event.type == WebInputEvent::TouchMove)
 return ACK_WITH_NOT_CONSUMED;

return ACK_WITH_NO_CONSUMER_EXISTS;

[jdduke (slow), 2015/02/12 16:55:56]

Hmm, yeah I had that completely wrong. It's still a little hard to follow,
maybe:

if (event.type != WebInputEvent::TouchMove)
  return FORWARD_TO_RENDERER;

if (HasPointChanged(...))
  return FORWARD_TO_RENDERER;

break;

with similar comments to what you had.

[USE s.singapati at gmail.com, 2015/02/13 12:16:47]

Done.

+            return FORWARD_TO_RENDERER;
+        } else {
           return FORWARD_TO_RENDERER;
+        }
+        // Pointer with same id found and handled.
+        break;
       }
+
     }
   }