Remove WebTouchEvent.targetTouches

Remove WebTouchEvent.targetTouches

Remove this field as it is no longer used by Chromium. Also prepare the
way for removing |WebTouchEvent.changedTouches| by populating
|WebTouchEvent.touches| with *all* touch points, even those that are
released or cancelled, in WebTouchEventBuilder. The plugin consumer
downstream can resynthesize the changed touches on-demand.

This is part 2 of a 4-sided patch simplifying WebTouchEvent:
1) https://codereview.chromium.org/557863002/
2) This change
3) https://codereview.chromium.org/559693003/
4) https://codereview.chromium.org/560493003/

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

[jdduke (slow), 2014-09-09 22:04:23]

jdduke@chromium.org changed reviewers:
+ aelias@chromium.org

[jdduke (slow), 2014-09-09 22:04:23]

aelias@: PTAL, thanks.

[aelias_OOO_until_Jul13, 2014-09-10 00:29:58]

I think I understand most of what's going on now and agree with the direction. 
One point: you're gradually changing the semantics of "touches" to include all
touches.  It seems complicated to transition with multi-sided patches while
keeping the browser functional at each step given that, and I think that
complexity is the main source of my confusion earlier.  It might be more
straightforward to add a new field "allTouches" with the new semantics you want.
 Then delete both "touches" and "changedTouches" after that.  That would obviate
the need for the kind of short-lived merging algorithm you're introducing here,
and will probably simplify the Blink-side transition as well.

[jdduke (slow), 2014-09-10 00:41:37]

On 2014/09/10 00:29:58, aelias wrote:
> I think I understand most of what's going on now and agree with the direction.

> One point: you're gradually changing the semantics of "touches" to include all
> touches.  It seems complicated to transition with multi-sided patches while
> keeping the browser functional at each step given that, and I think that
> complexity is the main source of my confusion earlier.  It might be more
> straightforward to add a new field "allTouches" with the new semantics you
want.
>  Then delete both "touches" and "changedTouches" after that.  That would
obviate
> the need for the kind of short-lived merging algorithm you're introducing
here,
> and will probably simplify the Blink-side transition as well.

Hmm, but |touches| *does* have the semantics I want, at least in practice. When
populated from the browser, it includes all touches, and when offered to the
plugin (the only consumer of the conversion from TouchEvent to WebTouchEvent),
it's immediately patched to include all touches
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/b...).


If you think |allTouches| is a better permanent name than |touches| even after
we remove |changedTouches| and |targetTouches|, that's fair (I disagree slightly
only on account of verbosity), but I'm not sure that adding a 4th intermediate
field will make the refactoring stages any cleaner. I plan on getting all 4
patches approved before I start landing any single patch, so we can land as fast
as Blink can roll and then be done with this mess entirely.

[jdduke (slow), 2014-09-10 00:42:41]

I'll include links to all patches in the descriptions, giving their ordering and
hopefully making the intent and end result a bit more clear.

[aelias_OOO_until_Jul13, 2014-09-10 01:14:05]

OK, I wasn't aware WebTouchEvent semantics were different from blink::TouchEvent
to that degree.  I understand the burning desire to clean this up now. 
Source/web lgtm with suggested simplification below.

https://codereview.chromium.org/558773002/diff/20001/Source/web/WebInputEvent...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/558773002/diff/20001/Source/web/WebInputEvent...
Source/web/WebInputEventConversion.cpp:775: pointIndex = j;
Would it have correct semantics to first add the changed touches, then add the
unchanged touches with "give up if already exists" policy?  I'm just wondering
if the "update state" part of this can be removed as the logic would end up a
lot simpler.  If that wouldn't work, then fine to leave this as is.

[jdduke (slow), 2014-09-10 01:53:36]

https://codereview.chromium.org/558773002/diff/20001/Source/web/WebInputEvent...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/558773002/diff/20001/Source/web/WebInputEvent...
Source/web/WebInputEventConversion.cpp:775: pointIndex = j;
On 2014/09/10 01:14:05, aelias wrote:
> Would it have correct semantics to first add the changed touches, then add the
> unchanged touches with "give up if already exists" policy?  I'm just wondering
> if the "update state" part of this can be removed as the logic would end up a
> lot simpler.  If that wouldn't work, then fine to leave this as is.

Yeah, that also works, will update in the morning.

[jdduke (slow), 2014-09-10 03:02:40]

jdduke@chromium.org changed reviewers:
+ pfeldman@chromium.org

[jdduke (slow), 2014-09-10 03:02:40]

pfeldman@: Owner review for public/web/WebInputEvent.h, thanks.

[pfeldman, 2014-09-10 18:38:40]

public/web lgtm

[jdduke (slow), 2014-09-10 23:32:31]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-09-10 23:33:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558773002/40001

[commit-bot: I haz the power, 2014-09-11 01:11:59]

Committed patchset #3 (id:40001) as 181775