New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 627926 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 631135



Sign in to add a comment

Investigate pinch gestures for OOPIFs

Project Member Reported by kenrb@chromium.org, Jul 13 2016

Issue description

It looks like pinch isn't implemented correctly when routing events between multiple processes rendering a single page. Pinch zoom does appear to work correctly on BrowserPlugin which uses input routing for gesture events, although we aren't clear why.

Pinch gestures should always go to top-level frames to trigger page zoom. Accordingly, top-level RenderWidgetHostViews should send them directly to their renderers rather than trying to pass them through RenderWidgetHostInputEventRouter.

 
Owner: wjmaclean@chromium.org
Status: Assigned (was: Available)

Comment 2 by kenrb@chromium.org, Jul 18 2016

Cc: bokan@chromium.org moh...@chromium.org
Relevant comment by bokan@ from https://codereview.chromium.org/2034213002/, outlining a problem that needs to be fixed:

"... [T]oday we send the Pinch gesture to Blink, it synthesizes a
wheel event, and if that's unhandled we pinch-zoom. This breaks for OOPIF (and
is probably already broken today in Mac with --enable-site-isolation). It works
outside site-isolation because the WebViewImpl handles the pinch-zoom so it can
apply it globally. If this is to work with OOPIF we need to move this logic up
into the browser. So the browser should synthesize a wheel event, send it to the
renderer associated with the frame, if the ACK comes back unhandled, send the
pinch-zoom event to the root view."
Status: Started (was: Assigned)
We can keep one bug open fo this, but I think the work to fix this for touch-screen should probably land separately from the work for touch-pad. I'll work on the touch-screen CL first unless there are any reasons to do the touch-pad work first.

Comment 4 by moh...@chromium.org, Jul 18 2016

If I recall correctly, the logic mentioned in c#2 was actually once in the browser, but later moved to the renderer. Please take a look at CL landed for  issue 451559 .

Comment 5 by bokan@chromium.org, Jul 18 2016

No, that's making the touchpad activate pinch-zoom rather than browser zoom (i.e. ctrl+/-). Prior to that, there was no "touchpad" pinch zoom to speak of.

Comment 6 by nasko@chromium.org, Jul 18 2016

Considering we are shipping first to desktop platforms, shouldn't we be doing touchpad support first and touchscreen support later?

Comment 7 by bokan@chromium.org, Jul 18 2016

FWIW, touchscreens do exist on desktop (laptops mostly). 
FWIW, I expect the work for touchscreen to be quite small by comparison, so it should be out of the way quickly, then we'll get to the touchpad shortly afterwards.

Comment 9 by nasko@chromium.org, Jul 18 2016

#c7, I'm aware, but the percentage is likely small. #c8 - sgtm
Blockedon: 631135
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/237a3cdd338750a6a35d336912c5830868380272

commit 237a3cdd338750a6a35d336912c5830868380272
Author: wjmaclean <wjmaclean@chromium.org>
Date: Fri Aug 05 22:35:13 2016

Re-route Touchscreen GesturePinch events to root view.

This CL fixes the routing of GesturePinch* events so that they always go
to the root view. Also, any other gesture events between
GesturePinchBegin and GesturePinchEnd (namely GestureScrollUpdate) are
also routed to the root view, so that the visual viewport pans
correctly.

BUG= 627926 

Review-Url: https://codereview.chromium.org/2186983002
Cr-Commit-Position: refs/heads/master@{#410198}

[modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/237a3cdd338750a6a35d336912c5830868380272/content/browser/site_per_process_browsertest.cc

This is now fixed for touchscreen pinch gestures.
Status: Fixed (was: Started)
With https://codereview.chromium.org/2745783002/ this is also workig for TouchPad gestures (Mac) for both Non-OOPIF WebView, and OOPIF WebView.

I think this is working correctly for everything except propagation of page scale factor, which is being tracked in its own bug, so closing.

Sign in to add a comment