Touch emulator: enables double tap support for pages similar to Chrome on Android.

Touch emulator: enable double tap support for pages similarly to Chrome on Android.

This patch also moves the logic which decides on enabling/disabling double tap
to a common place to be reused both in touch emulation and android gesture detection.

BUG=425586, 325506
Committed: https://crrev.com/8a282af93f01dd53540eb1cd3531598ce687f668
Cr-Commit-Position: refs/heads/master@{#313123}

[dgozman, 2015-01-20 13:30:12]

dgozman@chromium.org changed reviewers:
+ aelias@chromium.org, jdduke@chromium.org

[dgozman, 2015-01-20 13:30:13]

Hi,

Could you please take a look? This is mostly code moving.

I've decided to extract the "disable double tap for mobile-optimized sites"
logic to a common place to reuse, since I expect it may be updated in the
future, and this patch ensures that touch emulator will be updated as well.

The most suitable place I've found is "ui_events_helper" (didn't want to move
the code outside of c/b/renderer_host). If you think another place is better,
let's move there.

Thanks,
Dmitry

[jdduke (slow), 2015-01-20 17:12:38]

Hmm, yeah, I'm not sure there's a better place for such a utility method,
ui_events_helper is probably as good a place as any. Another thought would be to
have a 

|CompositorFrameMetadata::IsMobileOptimized{Frame|Site|Page}() const|

query that returns true for the same cases. We could then reuse that for some
other cases (replacing InputRouter::OnViewUpdated()). aelias@: thoughts?

[jdduke (slow), 2015-01-20 17:17:38]

On 2015/01/20 17:12:38, jdduke wrote:
> Hmm, yeah, I'm not sure there's a better place for such a utility method,
> ui_events_helper is probably as good a place as any. Another thought would be
to
> have a 
> 
> |CompositorFrameMetadata::IsMobileOptimized{Frame|Site|Page}() const|
> 
> query that returns true for the same cases. We could then reuse that for some
> other cases (replacing InputRouter::OnViewUpdated()). aelias@: thoughts?

or maybe |IsMobileOptimizedContent|, similar to the
|WebViewImpl::shouldDisableDesktopWorkarounds| query.

[dgozman, 2015-01-22 13:11:18]

On 2015/01/20 17:17:38, jdduke wrote:
> On 2015/01/20 17:12:38, jdduke wrote:
> > Hmm, yeah, I'm not sure there's a better place for such a utility method,
> > ui_events_helper is probably as good a place as any. Another thought would
be
> to
> > have a 
> > 
> > |CompositorFrameMetadata::IsMobileOptimized{Frame|Site|Page}() const|
> > 
> > query that returns true for the same cases. We could then reuse that for
some
> > other cases (replacing InputRouter::OnViewUpdated()). aelias@: thoughts?
> 
> or maybe |IsMobileOptimizedContent|, similar to the
> |WebViewImpl::shouldDisableDesktopWorkarounds| query.

Turned out ui_events_util is not compiled on android, so I've moved to dip_util.
Renamed to IsMobileOptimizedFrame. PTAL.

[jdduke (slow), 2015-01-22 16:28:27]

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/touch_emulator.cc (right):

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/touch_emulator.cc:96:
gesture_provider_->SetDoubleTapSupportForPageEnabled(double_tap_enabled_);
Hmm, is it intentional that we could support double-tap here for desktop
platforms, even when desktop platforms never support it in practice? Should we
|SetDoubleTapSupportForPlatformEnabled| depending on mobile/desktop config?
Alternatively we could have have double_tap_support_for_platform be a parameter
on GestureProvider::Config, and have its factory set the right value.

[dgozman, 2015-01-22 17:04:38]

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/touch_emulator.cc (right):

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/touch_emulator.cc:96:
gesture_provider_->SetDoubleTapSupportForPageEnabled(double_tap_enabled_);
On 2015/01/22 16:28:27, jdduke wrote:
> Hmm, is it intentional that we could support double-tap here for desktop
> platforms, even when desktop platforms never support it in practice? Should we
> |SetDoubleTapSupportForPlatformEnabled| depending on mobile/desktop config?
> Alternatively we could have have double_tap_support_for_platform be a
parameter
> on GestureProvider::Config, and have its factory set the right value.

I thought that config does already contain
SetDoubleTapSupportForPlatformEnabled. That seems like a right place for it.

[jdduke (slow), 2015-01-22 17:09:36]

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/input/touch_emulator.cc (right):

https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
content/browser/renderer_host/input/touch_emulator.cc:96:
gesture_provider_->SetDoubleTapSupportForPageEnabled(double_tap_enabled_);
On 2015/01/22 17:04:37, dgozman wrote:
> On 2015/01/22 16:28:27, jdduke wrote:
> > Hmm, is it intentional that we could support double-tap here for desktop
> > platforms, even when desktop platforms never support it in practice? Should
we
> > |SetDoubleTapSupportForPlatformEnabled| depending on mobile/desktop config?
> > Alternatively we could have have double_tap_support_for_platform be a
> parameter
> > on GestureProvider::Config, and have its factory set the right value.
> 
> I thought that config does already contain
> SetDoubleTapSupportForPlatformEnabled. That seems like a right place for it.

Do you want to do that in this patch? Or I can quickly whip up a patch to do
that and land before we land this patch?

[dgozman, 2015-01-23 12:56:29]

On 2015/01/22 17:09:36, jdduke wrote:
>
https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
> File content/browser/renderer_host/input/touch_emulator.cc (right):
> 
>
https://codereview.chromium.org/859973003/diff/20001/content/browser/renderer...
> content/browser/renderer_host/input/touch_emulator.cc:96:
> gesture_provider_->SetDoubleTapSupportForPageEnabled(double_tap_enabled_);
> On 2015/01/22 17:04:37, dgozman wrote:
> > On 2015/01/22 16:28:27, jdduke wrote:
> > > Hmm, is it intentional that we could support double-tap here for desktop
> > > platforms, even when desktop platforms never support it in practice?
Should
> we
> > > |SetDoubleTapSupportForPlatformEnabled| depending on mobile/desktop
config?
> > > Alternatively we could have have double_tap_support_for_platform be a
> > parameter
> > > on GestureProvider::Config, and have its factory set the right value.
> > 
> > I thought that config does already contain
> > SetDoubleTapSupportForPlatformEnabled. That seems like a right place for it.
> 
> Do you want to do that in this patch? Or I can quickly whip up a patch to do
> that and land before we land this patch?

Thank you. With https://codereview.chromium.org/868823002/ landed, this should
be good to go.

[jdduke (slow), 2015-01-23 16:06:56]

content/browser/renderer_host/input lgtm. I can't say I love putting the method
in dpi_util either, but I'm not sure of a better location for it (aelias@: Any
thoughts there?).

[aelias_OOO_until_Jul13, 2015-01-23 18:13:36]

Agreed it doesn't seem to fit in dip_util, I guess we should add a new file
frame_metadata_util.h/cc?

[dgozman, 2015-01-26 14:13:26]

On 2015/01/23 18:13:36, aelias wrote:
> Agreed it doesn't seem to fit in dip_util, I guess we should add a new file
> frame_metadata_util.h/cc?

Added frame_metatada_util. PTAL.

[aelias_OOO_until_Jul13, 2015-01-26 19:13:43]

lgtm

[dgozman, 2015-01-26 19:54:39]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2015-01-26 19:56:37]

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

[commit-bot: I haz the power, 2015-01-26 20:41:06]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-01-26 20:42:07]

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