Add full touch support to mus.

Add full touch support to mus.

Everything necessary to use the existing display to touch input-device
association/transform code is accessible from mus now.
ScreenManagerOzone creates a TouchTransformController and tells it to
compute new transforms when the display configuration changes. That's
it.

Delete existing touch code that was added to the WS to associate a
single display and touch input-device.

BUG=679747
Review-Url: https://codereview.chromium.org/2627623003
Cr-Commit-Position: refs/heads/master@{#443162}
Committed: https://chromium.googlesource.com/chromium/src/+/f77fda1001064005346121f403bb80b3414cbc00

[kylechar, 2017-01-10 16:59:13]

kylechar@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2017-01-10 19:43:27]

I'd approve, but where is TouchTransformController?

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
File services/ui/display/screen_manager_ozone.cc (right):

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:67: if (touch_transform_controller_)
optional nit: check is not necessary

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:188: touch_transform_controller_ =
base::MakeUnique<TouchTransformController>(
I don't get results for TouchTransformController in codesearch or grepping my
local repo, and I don't know touch code well. Otherwise, your CL otherwise looks
fine; I just can't approve without some idea of what you're instantiating here
(and how it compares with the removed TouchController).

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:366: if
(touch_transform_controller_)
q: does this ever happen before init or after reset in the dtor? (is this check
necessary?)

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc
File services/ui/service.cc (left):

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc#oldc...
services/ui/service.cc:186: // This needs to happen after DeviceDataManager has
been constructed. That
optional nit: dcheck for DeviceDataManager::HasInstance here?

[kylechar, 2017-01-10 22:12:18]

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
File services/ui/display/screen_manager_ozone.cc (right):

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:67: if (touch_transform_controller_)
On 2017/01/10 19:43:27, msw wrote:
> optional nit: check is not necessary

Done.

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:188: touch_transform_controller_ =
base::MakeUnique<TouchTransformController>(
On 2017/01/10 19:43:27, msw wrote:
> I don't get results for TouchTransformController in codesearch or grepping my
> local repo, and I don't know touch code well. Otherwise, your CL otherwise
looks
> fine; I just can't approve without some idea of what you're instantiating here
> (and how it compares with the removed TouchController).

The current class is ash::TouchTransformerController, it's renamed in the parent
CL. I'm assuming there used to be some class TouchTransformer that no longer
exists because there are class/method names that reference it. The code now just
uses gfx::Transform instead, so I changed the manager class name too.

TouchController was a hacky version of TouchTransformerController I wrote that
to get pixel touchscreen working.

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:366: if
(touch_transform_controller_)
On 2017/01/10 19:43:27, msw wrote:
> q: does this ever happen before init or after reset in the dtor? (is this
check
> necessary?)

It shouldn't, only if someone changed the order in the destructor, so it's
probably not needed. Done.

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc
File services/ui/service.cc (left):

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc#oldc...
services/ui/service.cc:186: // This needs to happen after DeviceDataManager has
been constructed. That
On 2017/01/10 19:43:27, msw wrote:
> optional nit: dcheck for DeviceDataManager::HasInstance here?

There is an if DDM::HasInstance() in the method that gets called. Not a DCHECK
though, since Windows/Mac won't ever initialize it.

[msw, 2017-01-10 22:36:57]

lgtm

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
File services/ui/display/screen_manager_ozone.cc (right):

https://codereview.chromium.org/2627623003/diff/1/services/ui/display/screen_...
services/ui/display/screen_manager_ozone.cc:188: touch_transform_controller_ =
base::MakeUnique<TouchTransformController>(
On 2017/01/10 22:12:18, kylechar wrote:
> On 2017/01/10 19:43:27, msw wrote:
> > I don't get results for TouchTransformController in codesearch or grepping
my
> > local repo, and I don't know touch code well. Otherwise, your CL otherwise
> looks
> > fine; I just can't approve without some idea of what you're instantiating
here
> > (and how it compares with the removed TouchController).
> 
> The current class is ash::TouchTransformerController, it's renamed in the
parent
> CL. I'm assuming there used to be some class TouchTransformer that no longer
> exists because there are class/method names that reference it. The code now
just
> uses gfx::Transform instead, so I changed the manager class name too.
> 
> TouchController was a hacky version of TouchTransformerController I wrote that
> to get pixel touchscreen working.

Acknowledged; thanks for pointing out the change dependency that I missed.

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc
File services/ui/service.cc (left):

https://codereview.chromium.org/2627623003/diff/1/services/ui/service.cc#oldc...
services/ui/service.cc:186: // This needs to happen after DeviceDataManager has
been constructed. That
On 2017/01/10 22:12:18, kylechar wrote:
> On 2017/01/10 19:43:27, msw wrote:
> > optional nit: dcheck for DeviceDataManager::HasInstance here?
> 
> There is an if DDM::HasInstance() in the method that gets called. Not a DCHECK
> though, since Windows/Mac won't ever initialize it.

Acknowledged.

[kylechar, 2017-01-11 19:41:36]

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

[commit-bot: I haz the power, 2017-01-11 19:42:07]

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, 2017-01-11 19:59:20]

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

[commit-bot: I haz the power, 2017-01-11 19:59:21]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[kylechar, 2017-01-11 21:55:55]

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

[commit-bot: I haz the power, 2017-01-11 21:56:39]

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, 2017-01-11 23:40:19]

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

[commit-bot: I haz the power, 2017-01-11 23:40:20]

Dry run: This issue passed the CQ dry run.

[kylechar, 2017-01-12 05:19:41]

The CQ bit was checked by kylechar@chromium.org

[commit-bot: I haz the power, 2017-01-12 05:20:01]

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, 2017-01-12 05:28:27]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484198381359220,
"parent_rev": "fd39adcc3b11e8ebe81754de74fc3cd191e44c29", "commit_rev":
"f77fda1001064005346121f403bb80b3414cbc00"}

[commit-bot: I haz the power, 2017-01-12 05:29:09]

Description was changed from

==========
Add full touch support to mus.

Everything necessary to use the existing display to touch input-device
association/transform code is accessible from mus now.
ScreenManagerOzone creates a TouchTransformController and tells it to
compute new transforms when the display configuration changes. That's
it.

Delete existing touch code that was added to the WS to associate a
single display and touch input-device.

BUG=679747
==========

to

==========
Add full touch support to mus.

Everything necessary to use the existing display to touch input-device
association/transform code is accessible from mus now.
ScreenManagerOzone creates a TouchTransformController and tells it to
compute new transforms when the display configuration changes. That's
it.

Delete existing touch code that was added to the WS to associate a
single display and touch input-device.

BUG=679747

Review-Url: https://codereview.chromium.org/2627623003
Cr-Commit-Position: refs/heads/master@{#443162}
Committed:
https://chromium.googlesource.com/chromium/src/+/f77fda1001064005346121f403bb...
==========

[commit-bot: I haz the power, 2017-01-12 05:29:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f77fda1001064005346121f403bb...