Improve scroll performance on Windows 10 with high precision touchpads.

Improve scroll performance on Windows 10 with high precision touchpads.

Chrome scrolling performance on Windows 10 with high precision touch pads, is very janky and jumpy.
Windows 10 introduces a new API called Direct Manipulation which allows consumers to support smooth
scrolling, panning etc. Mouse wheel messages are dispatched differently based on whether the window
is a Direct manipulation consumer. Microsoft recommends supporting Direct Manipulation on Windows 10
and above.

This is an intial patch to do the bare minimum to register Chrome as a Direct Manipulation consumer.
The associated functionality is provided by the newly added DirectManipulationHelper class which lives
as a member of the LegacyRenderWidgetHostHWND class which ensures that these changes are isolated cleanly.

This does smoothen scrolling on these devices. However there is still work to be done

BUG=517183, 517187
Committed: https://crrev.com/d53bb2ab620b6dc5e17da85dcf1b83b516c8c212
Cr-Commit-Position: refs/heads/master@{#342962}

[ananta, 2015-08-11 00:32:32]

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

[sky, 2015-08-11 15:43:12]

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/legacy_render_widget_host_win.cc (right):

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:50: class
DirectManipulationHelper {
Would it make more sense to do this in views so that views also gets this logic
too?

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:62:
CHECK(SUCCEEDED(hr));
Do these all need to be CHECKs, if any of these fail won't it all continue to
work?

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:96: return true;
Seems like you always return true.

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:148: };
DISALLOW...

[ananta, 2015-08-11 20:52:08]

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/legacy_render_widget_host_win.cc (right):

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:50: class
DirectManipulationHelper {
On 2015/08/11 15:43:12, sky wrote:
> Would it make more sense to do this in views so that views also gets this
logic
> too?

I guess so. I moved the DirectManipulationHelper class to ui\gfx\win. The class
is used from views and from the LegacyRenderWidgetHostHWND class. Reason for the
latter is to continue supporting drivers which assume the existence of the
window class and send messages to it.

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:62:
CHECK(SUCCEEDED(hr));
On 2015/08/11 15:43:12, sky wrote:
> Do these all need to be CHECKs, if any of these fail won't it all continue to
> work?

Added a TODO to remove the CHECKs once this stablizes.

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:96: return true;
On 2015/08/11 15:43:12, sky wrote:
> Seems like you always return true.

Changed the type to void

https://codereview.chromium.org/1283913002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/legacy_render_widget_host_win.cc:148: };
On 2015/08/11 15:43:12, sky wrote:
> DISALLOW...

Done.

[sky, 2015-08-11 23:45:02]

LGTM

https://codereview.chromium.org/1283913002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/legacy_render_widget_host_win.h (right):

https://codereview.chromium.org/1283913002/diff/40001/content/browser/rendere...
content/browser/renderer_host/legacy_render_widget_host_win.h:18: #include
"ui/gfx/win/direct_manipulation.h"
forward declare and include in .cc.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/gfx.gyp#newcode299
ui/gfx/gfx.gyp:299: 'win/direct_manipulation.cc',
spacing is off.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
File ui/gfx/win/direct_manipulation.cc (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.cc:17: return NULL;
nullptr

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
File ui/gfx/win/direct_manipulation.h (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.h:43: static DirectManipulationHelper*
CreateInstance();
Make this return a scoped_ptr.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.h:63: DirectManipulationHelper() {}
Don't inline this.

https://codereview.chromium.org/1283913002/diff/40001/ui/views/win/hwnd_messa...
File ui/views/win/hwnd_message_handler.h (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/views/win/hwnd_messa...
ui/views/win/hwnd_message_handler.h:26: #include
"ui/gfx/win/direct_manipulation.h"
forward declare the class and include in the .cc.

[ananta, 2015-08-12 00:41:58]

https://codereview.chromium.org/1283913002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/legacy_render_widget_host_win.h (right):

https://codereview.chromium.org/1283913002/diff/40001/content/browser/rendere...
content/browser/renderer_host/legacy_render_widget_host_win.h:18: #include
"ui/gfx/win/direct_manipulation.h"
On 2015/08/11 23:45:02, sky wrote:
> forward declare and include in .cc.

Done.

https://codereview.chromium.org/1283913002/diff/40001/content/browser/rendere...
content/browser/renderer_host/legacy_render_widget_host_win.h:18: #include
"ui/gfx/win/direct_manipulation.h"
On 2015/08/11 23:45:02, sky wrote:
> forward declare and include in .cc.

Done.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/gfx.gyp#newcode299
ui/gfx/gfx.gyp:299: 'win/direct_manipulation.cc',
On 2015/08/11 23:45:02, sky wrote:
> spacing is off.

Fixed.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
File ui/gfx/win/direct_manipulation.cc (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.cc:17: return NULL;
On 2015/08/11 23:45:02, sky wrote:
> nullptr

Not needed anymore with the function returning a scoped ptr

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
File ui/gfx/win/direct_manipulation.h (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.h:43: static DirectManipulationHelper*
CreateInstance();
On 2015/08/11 23:45:02, sky wrote:
> Make this return a scoped_ptr.

Done.

https://codereview.chromium.org/1283913002/diff/40001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.h:63: DirectManipulationHelper() {}
On 2015/08/11 23:45:02, sky wrote:
> Don't inline this.

Done.

https://codereview.chromium.org/1283913002/diff/40001/ui/views/win/hwnd_messa...
File ui/views/win/hwnd_message_handler.h (right):

https://codereview.chromium.org/1283913002/diff/40001/ui/views/win/hwnd_messa...
ui/views/win/hwnd_message_handler.h:26: #include
"ui/gfx/win/direct_manipulation.h"
On 2015/08/11 23:45:02, sky wrote:
> forward declare the class and include in the .cc.

Done.

[ananta, 2015-08-12 00:47:19]

The CQ bit was checked by ananta@chromium.org

[ananta, 2015-08-12 00:47:20]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1283913002/#ps80001
(title: "Fixed comment")

[commit-bot: I haz the power, 2015-08-12 00:47:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1283913002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1283913002/80001

[commit-bot: I haz the power, 2015-08-12 02:04:23]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-08-12 02:05:10]

Patchset 5 (id:??) landed as
https://crrev.com/d53bb2ab620b6dc5e17da85dcf1b83b516c8c212
Cr-Commit-Position: refs/heads/master@{#342962}

[Reid Kleckner, 2015-08-12 15:26:11]

rnk@chromium.org changed reviewers:
+ rnk@chromium.org

[Reid Kleckner, 2015-08-12 15:26:11]

I'll try to write a quick fix.

https://codereview.chromium.org/1283913002/diff/80001/ui/gfx/win/direct_manip...
File ui/gfx/win/direct_manipulation.h (right):

https://codereview.chromium.org/1283913002/diff/80001/ui/gfx/win/direct_manip...
ui/gfx/win/direct_manipulation.h:40: class GFX_EXPORT DirectManipulationHelper {
The Chromium style plugin wants this class to have an out of line destructor, so
this broke the clang bots.
http://build.chromium.org/p/chromium.fyi/builders/CrWinClang/builds/2497
error: [chromium-style] Complex class/struct needs an explicit out-of-line
destructor.

[hans, 2015-08-12 20:24:03]

hans@chromium.org changed reviewers:
+ hans@chromium.org

[hans, 2015-08-12 20:24:03]

https://codereview.chromium.org/1283913002/diff/80001/ui/views/win/hwnd_messa...
File ui/views/win/hwnd_message_handler.h (right):

https://codereview.chromium.org/1283913002/diff/80001/ui/views/win/hwnd_messa...
ui/views/win/hwnd_message_handler.h:594:
base::WeakPtrFactory<HWNDMessageHandler> autohide_factory_;
Another Win/Clang style plugin error:

In file included from ..\..\ui\views\win\hwnd_message_handler.cc:5:
..\..\ui/views/win/hwnd_message_handler.h(594,44) :  error: [chromium-style]
WeakPtrFactory members which refer to their outer class must be the last member
in the outer class definition.
  base::WeakPtrFactory<HWNDMessageHandler> autohide_factory_;
                                           ^

I'll prepare a patch.