Add unique_event_id to the WebTouchEvent

Add unique_event_id to the WebTouchEvent.

In order for us to map and track between the touch events and their ACKs,
we need to add unique_event_id to WebTouchEvent.

This patch is part of a series:

patch #1: This CL
patch #2: https://codereview.chromium.org/999423003/

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

[lanwei, 2015-03-12 22:29:50]

lanwei@chromium.org changed reviewers:
+ jdduke@chromium.org, tdresser@chromium.org

[tdresser, 2015-03-13 13:59:37]

Are you sure that all of these modifications are necessary to plumb the id back
through the touch event ack? (They quite possibly are, I'm just not certain).
It'll be clearer once you've got the patch that passes the id to the touch event
ack object.

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cpp
File Source/web/WebInputEvent.cpp (right):

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cp...
Source/web/WebInputEvent.cpp:63: int touchData[4];
We can get away with using a uint32_t throughout. I probably should have used a
32 bit int originally, can you clean that up Chrome side at some point? It would
be nice to save the extra 32 bits here.

We should clean up
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/event.cc...
though, to use a sequential id generator
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/sequential_...
at some point too. Previously we only used the unique id for validation, whereas
now we're going to use it to actually drive logic.

https://codereview.chromium.org/1008573002/diff/1/Source/web/tests/WebInputEv...
File Source/web/tests/WebInputEventConversionTest.cpp (right):

https://codereview.chromium.org/1008573002/diff/1/Source/web/tests/WebInputEv...
Source/web/tests/WebInputEventConversionTest.cpp:214:
RefPtrWillBeRawPtr<TouchEvent> touchEvent = TouchEvent::create(touchList.get(),
touchList.get(), touchList.get(), EventTypeNames::touchstart, domWindow, false,
false, false, false, false, false, 0u);
Use larger and more random numbers than 0, 1, 2, 3.
It will rule out a small class of potential problems.

[tdresser, 2015-03-13 15:21:34]

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cpp
File Source/web/WebInputEvent.cpp (right):

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cp...
Source/web/WebInputEvent.cpp:63: int touchData[4];
On 2015/03/13 13:59:37, tdresser wrote:
> We can get away with using a uint32_t throughout. I probably should have used
a
> 32 bit int originally, can you clean that up Chrome side at some point? It
would
> be nice to save the extra 32 bits here.
> 
> We should clean up
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/event.cc...
> though, to use a sequential id generator
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/sequential_...
> at some point too. Previously we only used the unique id for validation,
whereas
> now we're going to use it to actually drive logic.

Lan pointed out that using the sequential id generator is a terrible idea, as we
could end up reusing ids too soon. Just incrementing a uint32_t should be fine.

[jdduke (slow), 2015-03-13 15:44:53]

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cpp
File Source/web/WebInputEvent.cpp (right):

https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cp...
Source/web/WebInputEvent.cpp:63: int touchData[4];
On 2015/03/13 15:21:34, tdresser wrote:
> On 2015/03/13 13:59:37, tdresser wrote:
> > We can get away with using a uint32_t throughout. I probably should have
used
> a
> > 32 bit int originally, can you clean that up Chrome side at some point? It
> would
> > be nice to save the extra 32 bits here.
> > 
> > We should clean up
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/event.cc...
> > though, to use a sequential id generator
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/sequential_...
> > at some point too. Previously we only used the unique id for validation,
> whereas
> > now we're going to use it to actually drive logic.
> 
> Lan pointed out that using the sequential id generator is a terrible idea, as
we
> could end up reusing ids too soon. Just incrementing a uint32_t should be
fine.

I'm not in love with the idea that we might wrap here, and an extra word isn't
going to break the bank if it ensures that our unique ids are actually unique.
Ideally, 0 would only be used for events that haven't been assigned an ID.

[tdresser, 2015-03-13 15:46:08]

On 2015/03/13 15:44:53, jdduke wrote:
> https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cpp
> File Source/web/WebInputEvent.cpp (right):
> 
>
https://codereview.chromium.org/1008573002/diff/1/Source/web/WebInputEvent.cp...
> Source/web/WebInputEvent.cpp:63: int touchData[4];
> On 2015/03/13 15:21:34, tdresser wrote:
> > On 2015/03/13 13:59:37, tdresser wrote:
> > > We can get away with using a uint32_t throughout. I probably should have
> used
> > a
> > > 32 bit int originally, can you clean that up Chrome side at some point? It
> > would
> > > be nice to save the extra 32 bits here.
> > > 
> > > We should clean up
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/event.cc...
> > > though, to use a sequential id generator
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/sequential_...
> > > at some point too. Previously we only used the unique id for validation,
> > whereas
> > > now we're going to use it to actually drive logic.
> > 
> > Lan pointed out that using the sequential id generator is a terrible idea,
as
> we
> > could end up reusing ids too soon. Just incrementing a uint32_t should be
> fine.
> 
> I'm not in love with the idea that we might wrap here, and an extra word isn't
> going to break the bank if it ensures that our unique ids are actually unique.
> Ideally, 0 would only be used for events that haven't been assigned an ID.

Starting the ids at 1 seems like a good plan.
What's wrong with wrapping?

[lanwei, 2015-03-16 17:18:36]

Patchset #3 (id:40001) has been deleted

[tdresser, 2015-03-16 19:09:55]

https://codereview.chromium.org/1008573002/diff/50001/public/web/WebInputEvent.h
File public/web/WebInputEvent.h (right):

https://codereview.chromium.org/1008573002/diff/50001/public/web/WebInputEven...
public/web/WebInputEvent.h:531: , uniqueTouchEventId(1)
We want to ensure that every WebTouchEvent has a uniqueTouchEventId assigned. A
good way to do this would be to default to 0, and assert that the value isn't 0
once we're sure a uniqueTouchEventId should have been assigned.

[lanwei, 2015-03-16 20:20:47]

lanwei@chromium.org changed reviewers:
+ jochen@chromium.org

[lanwei, 2015-03-16 20:20:47]

jochen@chromium.org: Please review changes in 

Source/web/WebInputEvent.cpp
public/web/WebInputEvent.h

I add a new member unique_id in to Webtouchevent.

Thanks.

[lanwei, 2015-03-16 20:49:37]

lanwei@chromium.org changed reviewers:
- jochen@chromium.org

[lanwei, 2015-03-16 20:51:16]

lanwei@chromium.org changed reviewers:
+ tkent@chromium.org

[lanwei, 2015-03-16 20:51:16]

tkent@chromium.org: Please review changes in 

Source/web/WebInputEvent.cpp
public/web/WebInputEvent.h

I add a new member unique_id in to Webtouchevent.

Thanks.

[tkent, 2015-03-17 00:04:54]

lgtm

[lanwei, 2015-03-17 01:57:10]

The CQ bit was checked by lanwei@chromium.org

[commit-bot: I haz the power, 2015-03-17 01:57:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1008573002/70001

[commit-bot: I haz the power, 2015-03-17 02:01:12]

Committed patchset #4 (id:70001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191965