Don't handle NaturalScroll in Chrome but pass it to CMT instead

Don't handle NaturalScroll in Chrome but pass it to CMT instead

In order to support mice via CMT (Project Cobra) we will have to do
the natural scroll handling in the CMT driver instead of Chrome.

BUG=chromium:285663
TEST=test with touchpads and traditional mice. Everything should work as before,
touchpad should have natural scroll applied, scroll wheels should not.
Test changing the natural scroll property and see if it's applied correctly.
Also test if touch mice have natural scroll applied.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260926

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261478

[achuithb, 2014-03-27 00:15:18]

https://chromiumcodereview.appspot.com/212603005/diff/1/ui/events/x/device_da...
File ui/events/x/device_data_manager.cc (right):

https://chromiumcodereview.appspot.com/212603005/diff/1/ui/events/x/device_da...
ui/events/x/device_data_manager.cc:179: float
DeviceDataManager::GetNaturalScrollFactor(int sourceid) const {
Could you please also get rid of this function?

[achuithb, 2014-03-27 00:54:26]

https://codereview.chromium.org/212603005/diff/1/ui/events/event_utils.h
File ui/events/event_utils.h (left):

https://codereview.chromium.org/212603005/diff/1/ui/events/event_utils.h#oldc...
ui/events/event_utils.h:133: EVENTS_EXPORT bool ShouldDefaultToNaturalScroll();
What about this? Shouldn't this be removed as well?

[achuithb, 2014-03-27 00:59:02]

https://codereview.chromium.org/212603005/diff/1/ui/events/event_utils.h
File ui/events/event_utils.h (left):

https://codereview.chromium.org/212603005/diff/1/ui/events/event_utils.h#oldc...
ui/events/event_utils.h:133: EVENTS_EXPORT bool ShouldDefaultToNaturalScroll();
On 2014/03/27 00:54:27, achuithb wrote:
> What about this? Shouldn't this be removed as well?

Actually, it looks like it shouldn't be. Never mind.

[denniskempin (chromium), 2014-03-27 20:29:07]

please re-review. after splitting the CL up some lines were missing.

[achuithb, 2014-03-28 06:10:15]

lgtm, thanks!

[denniskempin (chromium), 2014-03-28 16:22:43]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-03-28 16:26:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/80001

[commit-bot: I haz the power, 2014-03-28 17:17:06]

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

[commit-bot: I haz the power, 2014-03-28 17:17:07]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[denniskempin (chromium), 2014-03-28 17:43:43]

Hi sky! Can I ask you to review the files you own?

[tfarina, 2014-03-28 17:54:45]

+Sadrul, in case he wants to look at ui/events/!

[sadrul, 2014-03-28 17:59:11]

https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
File ui/events/x/device_data_manager.cc (right):

https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
ui/events/x/device_data_manager.cc:458: *x_offset = -data[DT_CMT_SCROLL_X];
This is fairly confusing. Why do we need to unconditionally - the values? Why
not send the -ed values in the valuators from the driver?

[denniskempin (chromium), 2014-03-28 22:22:55]

On 2014/03/28 17:59:11, sadrul wrote:
>
https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
> File ui/events/x/device_data_manager.cc (right):
> 
>
https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
> ui/events/x/device_data_manager.cc:458: *x_offset = -data[DT_CMT_SCROLL_X];
> This is fairly confusing. Why do we need to unconditionally - the values? Why
> not send the -ed values in the valuators from the driver?

That's true. It only has historic reasons as we have always inverted the values
when using traditional scrolling, not the other way round.

This will require another change to the driver, can I suggest to do this in a
separate CL?

[denniskempin (chromium), 2014-03-31 19:47:23]

On 2014/03/28 22:22:55, denniskempin wrote:
> On 2014/03/28 17:59:11, sadrul wrote:
> >
>
https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
> > File ui/events/x/device_data_manager.cc (right):
> > 
> >
>
https://codereview.chromium.org/212603005/diff/100001/ui/events/x/device_data...
> > ui/events/x/device_data_manager.cc:458: *x_offset = -data[DT_CMT_SCROLL_X];
> > This is fairly confusing. Why do we need to unconditionally - the values?
Why
> > not send the -ed values in the valuators from the driver?
> 
> That's true. It only has historic reasons as we have always inverted the
values
> when using traditional scrolling, not the other way round.
> 
> This will require another change to the driver, can I suggest to do this in a
> separate CL?

Re-review please. 

I will run some tests to check if all events still have the proper sign with
this CL and the driver side change applied.

[sadrul, 2014-03-31 19:50:25]

Thanks! ui/events/ lgtm

[achuithb, 2014-03-31 19:52:02]

lgtm still assuming the inverse works.

[denniskempin (chromium), 2014-03-31 20:14:06]

On 2014/03/31 19:52:02, achuithb wrote:
> lgtm still assuming the inverse works.

Tested and compared to a stable-channel chromebook. Everything works well. This
will go hand in hand with the following two CLs on the driver side.

https://chromium-review.googlesource.com/#/c/192401/
https://chromium-review.googlesource.com/#/c/192402/

As it takes a while for chrome changes to arrive in chrome os, I will submit
this first. then the CLs on the driver side. As soon as this CL is in I will
send out an email explaining that todays builds might have inverted scrolling
but should be fixed by end of today.

[denniskempin (chromium), 2014-03-31 20:15:00]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-03-31 20:16:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/16...

[sky, 2014-03-31 20:58:10]

LGTM

[commit-bot: I haz the power, 2014-03-31 21:30:17]

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

[commit-bot: I haz the power, 2014-03-31 21:30:18]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg
  tryserver.chromium on linux_chromium_chromeos_rel

[denniskempin (chromium), 2014-03-31 23:24:38]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-03-31 23:25:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/18...

[commit-bot: I haz the power, 2014-04-01 00:13:38]

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

[commit-bot: I haz the power, 2014-04-01 00:13:38]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[denniskempin (chromium), 2014-04-01 00:29:10]

On 2014/04/01 00:13:38, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on linux_chromium_chromeos_rel

Can someone help me figure out why the build is failing? The telemetry tests are
apparently failing, but I cannot find any test failures in the log:
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...

However the log abruptly ends with the test: 
[ RUN      ] ClickElementActionTest.testClickWithXPathWaitForRefChange

which apparently never finishes.

[sadrul, 2014-04-01 02:11:06]

On 2014/04/01 00:29:10, denniskempin wrote:
> On 2014/04/01 00:13:38, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   tryserver.chromium on linux_chromium_chromeos_rel
> 
> Can someone help me figure out why the build is failing? The telemetry tests
are
> apparently failing, but I cannot find any test failures in the log:
>
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...
> 
> However the log abruptly ends with the test: 
> [ RUN      ] ClickElementActionTest.testClickWithXPathWaitForRefChange
> 
> which apparently never finishes.

Probably flake/unrelated: I see some other tries failing here (e.g.
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...
)

[denniskempin (chromium), 2014-04-01 17:24:41]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-04-01 17:25:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/18...

[commit-bot: I haz the power, 2014-04-01 19:37:19]

Change committed as 260926

[rlarocque, 2014-04-01 20:08:04]

A revert of this CL has been created in
https://codereview.chromium.org/221363002/ by rlarocque@chromium.org.

The reason for reverting is: Has caused many compile failures.  We think the
trybots didn't catch it because they didn't build ash_unittests.

Example:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...

../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual void
ash::StickyKeysTest_ScrollEventOneshot_Test::TestBody()':
../../ash/sticky_keys/sticky_keys_unittest.cc:622:41:error: 'class
ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual void
ash::StickyKeysTest_ScrollDirectionChanged_Test::TestBody()':
../../ash/sticky_keys/sticky_keys_unittest.cc:674:41:error: 'class
ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual void
ash::StickyKeysTest_ScrollEventLocked_Test::TestBody()':
../../ash/sticky_keys/sticky_keys_unittest.cc:711:41:error: 'class
ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
ninja: build stopped: subcommand failed.
.

[sadrul, 2014-04-01 20:12:47]

On 2014/04/01 20:08:04, rlarocque wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/221363002/ by mailto:rlarocque@chromium.org.
> 
> The reason for reverting is: Has caused many compile failures.  We think the
> trybots didn't catch it because they didn't build ash_unittests.
> 
> Example:
>
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
> 
> ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
void
> ash::StickyKeysTest_ScrollEventOneshot_Test::TestBody()':
> ../../ash/sticky_keys/sticky_keys_unittest.cc:622:41:error: 'class
> ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
void
> ash::StickyKeysTest_ScrollDirectionChanged_Test::TestBody()':
> ../../ash/sticky_keys/sticky_keys_unittest.cc:674:41:error: 'class
> ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
void
> ash::StickyKeysTest_ScrollEventLocked_Test::TestBody()':
> ../../ash/sticky_keys/sticky_keys_unittest.cc:711:41:error: 'class
> ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> ninja: build stopped: subcommand failed.
> .

Hm. How did the CQ land this CL? Does it not build/run ash_unittests anymore?

[denniskempin (chromium), 2014-04-01 20:23:51]

On 2014/04/01 20:12:47, sadrul wrote:
> On 2014/04/01 20:08:04, rlarocque wrote:
> > A revert of this CL has been created in
> > https://codereview.chromium.org/221363002/ by mailto:rlarocque@chromium.org.
> > 
> > The reason for reverting is: Has caused many compile failures.  We think the
> > trybots didn't catch it because they didn't build ash_unittests.
> > 
> > Example:
> >
>
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
> > 
> > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> void
> > ash::StickyKeysTest_ScrollEventOneshot_Test::TestBody()':
> > ../../ash/sticky_keys/sticky_keys_unittest.cc:622:41:error: 'class
> > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> void
> > ash::StickyKeysTest_ScrollDirectionChanged_Test::TestBody()':
> > ../../ash/sticky_keys/sticky_keys_unittest.cc:674:41:error: 'class
> > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> void
> > ash::StickyKeysTest_ScrollEventLocked_Test::TestBody()':
> > ../../ash/sticky_keys/sticky_keys_unittest.cc:711:41:error: 'class
> > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > ninja: build stopped: subcommand failed.
> > .
> 
> Hm. How did the CQ land this CL? Does it not build/run ash_unittests anymore?

I'll build ash_unittests and fix the errors. Then resubmit this CL.

[denniskempin (chromium), 2014-04-02 17:24:47]

On 2014/04/01 20:23:51, denniskempin wrote:
> On 2014/04/01 20:12:47, sadrul wrote:
> > On 2014/04/01 20:08:04, rlarocque wrote:
> > > A revert of this CL has been created in
> > > https://codereview.chromium.org/221363002/ by
mailto:rlarocque@chromium.org.
> > > 
> > > The reason for reverting is: Has caused many compile failures.  We think
the
> > > trybots didn't catch it because they didn't build ash_unittests.
> > > 
> > > Example:
> > >
> >
>
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
> > > 
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> > void
> > > ash::StickyKeysTest_ScrollEventOneshot_Test::TestBody()':
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc:622:41:error: 'class
> > > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> > void
> > > ash::StickyKeysTest_ScrollDirectionChanged_Test::TestBody()':
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc:674:41:error: 'class
> > > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc: In member function 'virtual
> > void
> > > ash::StickyKeysTest_ScrollEventLocked_Test::TestBody()':
> > > ../../ash/sticky_keys/sticky_keys_unittest.cc:711:41:error: 'class
> > > ui::DeviceDataManager' has no member named 'set_natural_scroll_enabled'
> > > ninja: build stopped: subcommand failed.
> > > .
> > 
> > Hm. How did the CQ land this CL? Does it not build/run ash_unittests
anymore?
> 
> I'll build ash_unittests and fix the errors. Then resubmit this CL.

I fixed the ash unit tests. However I cannot run them due do missing libraries.
They compile and "should" also pass.

[denniskempin (chromium), 2014-04-02 17:24:54]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-04-02 17:25:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/20...

[commit-bot: I haz the power, 2014-04-03 00:17:45]

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

[commit-bot: I haz the power, 2014-04-03 00:17:46]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[tfarina, 2014-04-03 00:26:08]

ash_unittests are not running for you? Which libraries are missing?

Could you post the errors you are seeing here or file a bug in crbug.com?

Which config are you building it on? Aura, ChromeOS?

[denniskempin (chromium), 2014-04-03 17:24:56]

On 2014/04/03 00:26:08, tfarina wrote:
> ash_unittests are not running for you? Which libraries are missing?
> 
> Could you post the errors you are seeing here or file a bug in crbug.com?
> 
> Which config are you building it on? Aura, ChromeOS?

I am building for chrome os for the link platform. I've filed a bug at:
https://code.google.com/p/chromium/issues/detail?id=359646

[denniskempin (chromium), 2014-04-03 17:24:57]

On 2014/04/03 00:26:08, tfarina wrote:
> ash_unittests are not running for you? Which libraries are missing?
> 
> Could you post the errors you are seeing here or file a bug in crbug.com?
> 
> Which config are you building it on? Aura, ChromeOS?

I am building for chrome os for the link platform. I've filed a bug at:
https://code.google.com/p/chromium/issues/detail?id=359646

[denniskempin (chromium), 2014-04-03 17:26:10]

The CQ bit was checked by denniskempin@chromium.org

[commit-bot: I haz the power, 2014-04-03 17:26:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/denniskempin@chromium.org/212603005/20...

[commit-bot: I haz the power, 2014-04-03 18:16:03]

Change committed as 261478