Decouple EventWithLatencyInfo from WebInputEventTraits

Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)

Committed: https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84
Cr-Commit-Position: refs/heads/master@{#403402}

[tapted, 2016-06-30 05:24:01]

Description was changed from

==========
Decouple EventWithLatencyInfo and WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h (the
instantiations in the .cc also become a bit simpler).

To allow the guts to be hidden, use extern templates. This also takes
some burden off the compiler/linker since less code needs to be
generated on the fly and collapsed at link time.

BUG=615948
==========

to

==========
Decouple EventWithLatencyInfo and WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
==========

[tapted, 2016-06-30 06:47:05]

Patchset #5 (id:80001) has been deleted

[tapted, 2016-06-30 12:03:31]

Patchset #7 (id:140001) has been deleted

[tapted, 2016-06-30 12:11:40]

tapted@chromium.org changed reviewers:
+ tdresser@chromium.org

[tapted, 2016-06-30 12:13:25]

Description was changed from

==========
Decouple EventWithLatencyInfo and WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
==========

to

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
==========

[tapted, 2016-06-30 12:13:45]

Hi Tim, please take a look

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
File content/common/input/event_with_latency_info.h (right):

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
content/common/input/event_with_latency_info.h:35: }  // namespace internal
This wasn't my first choice.. In http://crrev.com/2091213002 I got this working
with extern templates which hid all this (and the bodies of the
[Can]CoalesceWith member functions) in the .cc file. But extern templates are
kinda confusing, and it ran afoul of compiler bugs - different ones both on MSVC
and clang/gcc - details in http://crbug.com/623844.

I found workarounds for the compiler bugs, but this is nicer in some ways too
since you get a compiler error rather than a linker error if you try call
CanCoalesceWith on an unsupported type (client code can even add new supported
types).

[tdresser, 2016-06-30 14:16:39]

LGTM with minor nits.

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
File content/common/input/event_with_latency_info.h (right):

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
content/common/input/event_with_latency_info.h:8: #include
"base/compiler_specific.h"
Are we using this?

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/w...
File content/common/input/web_input_event_traits_unittest.cc (right):

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/w...
content/common/input/web_input_event_traits_unittest.cc:26: // Merge this test
harness into EventWithLatencyInfoTest.
Can you create a bug and reference it in this TODO?

[tapted, 2016-07-01 01:09:57]

Patchset #7 (id:160001) has been deleted

[tapted, 2016-07-01 01:49:22]

Patchset #8 (id:200001) has been deleted

[tapted, 2016-07-01 02:50:56]

Description was changed from

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
==========

to

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org
==========

[tapted, 2016-07-01 02:50:56]

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

[tapted, 2016-07-01 02:51:27]

Description was changed from

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org
==========

to

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)
==========

[tapted, 2016-07-01 02:53:45]

Thanks!

+jochen TBR for web_contents_impl.cc (just adding a required header previously
included transitively)

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
File content/common/input/event_with_latency_info.h (right):

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/e...
content/common/input/event_with_latency_info.h:8: #include
"base/compiler_specific.h"
On 2016/06/30 14:16:39, tdresser wrote:
> Are we using this?

Yep - for WARN_UNUSED_RESULT in the original code

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/w...
File content/common/input/web_input_event_traits_unittest.cc (right):

https://codereview.chromium.org/2111593003/diff/120001/content/common/input/w...
content/common/input/web_input_event_traits_unittest.cc:26: // Merge this test
harness into EventWithLatencyInfoTest.
On 2016/06/30 14:16:39, tdresser wrote:
> Can you create a bug and reference it in this TODO?

Done. (file+referenced http://crbug.com/625000)

https://codereview.chromium.org/2111593003/diff/220001/content/browser/web_co...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2111593003/diff/220001/content/browser/web_co...
content/browser/web_contents/web_contents_impl.cc:80: #include
"content/common/input/web_input_event_traits.h"
^ this  (which I had in the .h earlier, but realised it's fine in the .cc
instead)

[tapted, 2016-07-01 02:53:52]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-07-01 02:53:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2111593003/#ps220001
(title: "Bug reference + web_contents_impl iwyu to .cc")

[commit-bot: I haz the power, 2016-07-01 02:54:03]

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-07-01 03:42:21]

Description was changed from

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)
==========

to

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)
==========

[commit-bot: I haz the power, 2016-07-01 03:42:22]

Committed patchset #8 (id:220001)

[commit-bot: I haz the power, 2016-07-01 03:45:19]

Description was changed from

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)
==========

to

==========
Decouple EventWithLatencyInfo from WebInputEventTraits

Only EventWithLatencyInfo uses [Can]Coalesce() from WebInputEventTraits.
WebInputEventTraits gets used for IPC stuff so is kinda sensitive, but
EventWithLatencyInfo isn't involved with IPC. So, coupling these
together also makes refactoring unnecessarily difficult.

E.g. the coupling means that if we want to move EventWithLatencyInfo to
a component, then WebInputEventTraits and a bunch of dependencies that
EventWithLatencyInfo doesn't care about need to come as well.

To decouple, the guts of [Can]Coalesce() moves to a new .cc file for
EventWithLatencyInfo. The types are known, so we get some added type
safety versus WebInputEventTraits which isn't templatized in the .h. The
instantiations in the .cc become a bit simpler and the runtime type
checks and static casts behind in web_input_event_trait.cc's Apply(..)
are not required.

BUG=615948
TBR=jochen@chromium.org (iwyu)

Committed: https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84
Cr-Commit-Position: refs/heads/master@{#403402}
==========

[commit-bot: I haz the power, 2016-07-01 03:45:21]

Patchset 8 (id:??) landed as
https://crrev.com/85282366fed27efe2e19a686b1376ad9eca37d84
Cr-Commit-Position: refs/heads/master@{#403402}