Merge the mojo blink type converter into ui/events/blink

Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
Committed: https://crrev.com/27bc053e0d9d7bd1a9cbcfa73a38d318eeef09bf
Cr-Commit-Position: refs/heads/master@{#415956}

[jonross, 2016-08-23 20:17:31]

Patchset #1 (id:1) has been deleted

[jonross, 2016-08-23 20:17:52]

jonross@chromium.org changed reviewers:
+ sadrul@chromium.org

[jonross, 2016-08-23 20:17:53]

Sadrul, PTAL

[sadrul, 2016-08-24 18:05:37]

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:155:
std::unique_ptr<blink::WebInputEvent> web_event(Convert(event));
Can the WebInputEvent be created on the stack instead?  (if a valid event can't
be created, the event-type should be Undefined)

https://codereview.chromium.org/2270953002/diff/20001/ui/events/blink/web_inp...
File ui/events/blink/web_input_event.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/ui/events/blink/web_inp...
ui/events/blink/web_input_event.cc:461: screen_location_callback) {
Let's leave this out for now.

Explanation: WebTouchEvent contains information about multiple touch-points. A
PointerEvent tracks a single touch point. So there's no good mapping between the
two. What CompositorMusConnection should do instead, is to maintain a
ui::MotionEvent[Aura], feed the ui::TouchEvent to that, and use the existing
CreateWebTouchEventFromMotionEvent() to generate the WebTouchEvent.

[jonross, 2016-08-24 21:42:05]

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:155:
std::unique_ptr<blink::WebInputEvent> web_event(Convert(event));
On 2016/08/24 18:05:36, sadrul wrote:
> Can the WebInputEvent be created on the stack instead?  (if a valid event
can't
> be created, the event-type should be Undefined)

The unique_ptr is used here to preserve and pass the event around through
delayed tasks.
  - line 181
  - later on render_widget_mus_connection

If we are fine copying the WebInputEvent we could declare it on the stack.

Thoughts?

[sadrul, 2016-08-24 21:47:15]

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:155:
std::unique_ptr<blink::WebInputEvent> web_event(Convert(event));
On 2016/08/24 21:42:04, jonross wrote:
> On 2016/08/24 18:05:36, sadrul wrote:
> > Can the WebInputEvent be created on the stack instead?  (if a valid event
> can't
> > be created, the event-type should be Undefined)
> 
> The unique_ptr is used here to preserve and pass the event around through
> delayed tasks.
>   - line 181
>   - later on render_widget_mus_connection
> 
> If we are fine copying the WebInputEvent we could declare it on the stack.
> 
> Thoughts?

Copying should be OK, yeah.

[jonross, 2016-08-25 15:11:59]

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:155:
std::unique_ptr<blink::WebInputEvent> web_event(Convert(event));
On 2016/08/24 21:47:14, sadrul wrote:
> On 2016/08/24 21:42:04, jonross wrote:
> > On 2016/08/24 18:05:36, sadrul wrote:
> > > Can the WebInputEvent be created on the stack instead?  (if a valid event
> > can't
> > > be created, the event-type should be Undefined)
> > 
> > The unique_ptr is used here to preserve and pass the event around through
> > delayed tasks.
> >   - line 181
> >   - later on render_widget_mus_connection
> > 
> > If we are fine copying the WebInputEvent we could declare it on the stack.
> > 
> > Thoughts?
> 
> Copying should be OK, yeah.

As discussed offline, differing object sizes led to incorrect copying. Staying
with unique_ptr

https://codereview.chromium.org/2270953002/diff/20001/ui/events/blink/web_inp...
File ui/events/blink/web_input_event.cc (right):

https://codereview.chromium.org/2270953002/diff/20001/ui/events/blink/web_inp...
ui/events/blink/web_input_event.cc:461: screen_location_callback) {
On 2016/08/24 18:05:36, sadrul wrote:
> Let's leave this out for now.
> 
> Explanation: WebTouchEvent contains information about multiple touch-points. A
> PointerEvent tracks a single touch point. So there's no good mapping between
the
> two. What CompositorMusConnection should do instead, is to maintain a
> ui::MotionEvent[Aura], feed the ui::TouchEvent to that, and use the existing
> CreateWebTouchEventFromMotionEvent() to generate the WebTouchEvent.

Done.

[sadrul, 2016-08-26 02:37:02]

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:168: return
base::WrapUnique(new blink::WebMouseEvent(blink_event));
Would "return base::MakeUnique<blink::WebMouseEvent>(blink_event);" work here?

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:179: return
base::WrapUnique(new blink::WebMouseWheelEvent(blink_event));
We will be getting PointerEvents for wheel-events soon
(https://codereview.chromium.org/2256343003/). Maybe leave this out, and update
the code above for mouse-events instead?

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:186: }
This entire block doesn't need any else, since each block ends with a return.

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.h (right):

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.h:86:
std::unique_ptr<blink::WebInputEvent> Convert(const ui::Event& event);
Non-overrides before override. Move this up.

[jonross, 2016-08-26 14:33:50]

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.cc (right):

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:168: return
base::WrapUnique(new blink::WebMouseEvent(blink_event));
On 2016/08/26 02:37:02, sadrul wrote:
> Would "return base::MakeUnique<blink::WebMouseEvent>(blink_event);" work here?

Done.

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:179: return
base::WrapUnique(new blink::WebMouseWheelEvent(blink_event));
On 2016/08/26 02:37:02, sadrul wrote:
> We will be getting PointerEvents for wheel-events soon
> (https://codereview.chromium.org/2256343003/). Maybe leave this out, and
update
> the code above for mouse-events instead?

Added a comment on that review. I'll redo this section accordingly (either using
the requested event helper, or doing a two stage check like in the current
converter)

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.cc:186: }
On 2016/08/26 02:37:02, sadrul wrote:
> This entire block doesn't need any else, since each block ends with a return.

Done.

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
File content/renderer/mus/compositor_mus_connection.h (right):

https://codereview.chromium.org/2270953002/diff/40001/content/renderer/mus/co...
content/renderer/mus/compositor_mus_connection.h:86:
std::unique_ptr<blink::WebInputEvent> Convert(const ui::Event& event);
On 2016/08/26 02:37:02, sadrul wrote:
> Non-overrides before override. Move this up.

Done.

[sadrul, 2016-08-26 16:08:16]

lgtm

[jonross, 2016-08-26 17:35:48]

jonross@chromium.org changed reviewers:
+ kenrb@chromium.org

[jonross, 2016-08-26 17:35:49]

Hey Ken,

Could you provide an owners review for the removal of
mojo/converters/blink/*type_converters*

We plan to have all ui::Event -> blink::WebInputEvent to use utils in
ui/events/blink, rather than maintaining duplicate logic in type converters.

Thanks,
Jon

[jonross, 2016-08-26 17:36:53]

jonross@chromium.org changed reviewers:
+ meacer@chromium.org
- kenrb@chromium.org

[jonross, 2016-08-26 17:36:54]

Hey meacer@

Could you provide an owners review for the removal of
mojo/converters/blink/*type_converters*

We plan to have all ui::Event -> blink::WebInputEvent to use utils in
ui/events/blink, rather than maintaining duplicate logic in type converters.

Thanks,
Jon

[jonross, 2016-08-26 17:39:41]

jonross@chromium.org changed reviewers:
+ cpu@chromium.org

[jonross, 2016-08-26 17:39:42]

Hey cpu@,

Could you provide an owner's review for content/renderer, and the non
type-converter files in mojo/?

We plan to have all ui::Event -> blink::WebInputEvent to use utils in
ui/events/blink, rather than maintaining duplicate logic in type converters.

Thanks,
Jon

[meacer, 2016-08-26 19:10:59]

mojo/converters/blink LGTM

[jonross, 2016-08-29 19:13:56]

Hey cpu@,

Could you provide an owner's review for content/renderer, and the non
type-converter files in mojo/?

We plan to have all ui::Event -> blink::WebInputEvent to use utils in
ui/events/blink, rather than maintaining duplicate logic in type converters.

Thanks,
Jon

[jonross, 2016-08-30 20:57:06]

Description was changed from

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
==========

to

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
==========

[jonross, 2016-08-30 20:57:06]

jonross@chromium.org changed reviewers:
- cpu@chromium.org

[jonross, 2016-08-30 20:58:32]

jonross@chromium.org changed reviewers:
+ avi@chromium.org

[jonross, 2016-08-30 20:58:33]

avi@chromium.org: Please review changes in 

content/renderer/

We are unifying the ui::Event to blink::WebInputEvent conversion into one
location. And I am updating the call sites in content/renderer/

Thanks,
Jon

[jonross, 2016-08-30 20:59:56]

jonross@chromium.org changed reviewers:
+ sky@chromium.org

[jonross, 2016-08-30 20:59:57]

sky@chromium.org: Please review changes in 

mojo/converters/

I'm deleting the ui::Event to blink::WebInputEvent type converters in favour of
the ui/events/blink/ utilities.

Thanks,
Jon

[sky, 2016-08-30 22:16:12]

LGTM

[Avi (use Gerrit), 2016-08-31 15:11:25]

content lgtm

[jonross, 2016-08-31 15:11:59]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2016-08-31 15:12:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 16:05:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 16:05:14]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jonross, 2016-08-31 21:31:13]

The CQ bit was checked by jonross@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-31 21:32:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 22:26:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 22:26:32]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jonross, 2016-08-31 22:32:17]

The CQ bit was checked by jonross@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-31 22:33:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 00:04:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 00:04:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jonross, 2016-09-01 14:01:54]

The CQ bit was checked by jonross@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 14:02:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 15:12:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 15:12:46]

Dry run: This issue passed the CQ dry run.

[jonross, 2016-09-01 15:14:18]

The CQ bit was checked by jonross@chromium.org

[jonross, 2016-09-01 15:14:19]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
sadrul@chromium.org, sky@chromium.org, meacer@chromium.org
Link to the patchset: https://codereview.chromium.org/2270953002/#ps120001
(title: "Rebase apparently restored deleted files")

[commit-bot: I haz the power, 2016-09-01 15:14:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 15:19:03]

Description was changed from

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
==========

to

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
==========

[commit-bot: I haz the power, 2016-09-01 15:19:05]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-09-01 15:23:18]

Description was changed from

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673
==========

to

==========
Merge the mojo blink type converter into ui/events/blink

The code in mojo/converters/blink/blink_input_event_type_converters was mostly
duplicate of ui/events/blink/web_input_event.

This change merges in the difference, blink::WebTouchEvent conversion. The tests
from mojo have been ported over.

Furthermore CompositorMusConnection has been updated to use ui/events/blink.

With no other callsites, the mojo type converter has been deleted

TEST=events_unittests, content_unittests
BUG=616673

Committed: https://crrev.com/27bc053e0d9d7bd1a9cbcfa73a38d318eeef09bf
Cr-Commit-Position: refs/heads/master@{#415956}
==========

[commit-bot: I haz the power, 2016-09-01 15:23:20]

Patchset 6 (id:??) landed as
https://crrev.com/27bc053e0d9d7bd1a9cbcfa73a38d318eeef09bf
Cr-Commit-Position: refs/heads/master@{#415956}