Skip to content

Commit

Permalink
Reland "Consistent touchpad pinch behaviour for app windows and webvi…
Browse files Browse the repository at this point in the history
…ews"

This is a reland of be51aad

Original change's description:
> Consistent touchpad pinch behaviour for app windows and webviews
> 
> Currently, app windows (possibly containing webviews):
> 1) suppress all pinch events which also suppresses the
>    synthetic wheel event, but not touchscreen touch events, and
> 2) allow "smart zoom" on Mac which can change the page scale even
>    though other means of changing the page scale are suppressed.
> 
> As a result of (1), if we have, say, google maps in a webview, a
> user can pinch zoom with a touchscreen, but not with a touchpad.
> 
> We now suppress gesture events to app windows based on whether they
> cause a scale change. So now touchscreen and touchpad pinching are
> consistent since the page has access to the events so that it may
> implement custom pinch zoom behaviour, but unhandled events still
> do not result in a scale change for the app window. Also, smart
> zoom is suppressed to be consistent with other touchpad pinching.
> 
> Bug: 725970, 874132
> Change-Id: I03dd2048002d69dc7c8a822fc727140c67d64706
> Reviewed-on: https://chromium-review.googlesource.com/1174933
> Reviewed-by: Ben Wells <benwells@chromium.org>
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
> Commit-Queue: Kevin McNee <mcnee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585489}

Bug: 725970, 874132
Change-Id: I23420821c1166f7b641faf672798ddac0e928ad9
Reviewed-on: https://chromium-review.googlesource.com/1187345
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585856}
  • Loading branch information
kjmcnee authored and Commit Bot committed Aug 24, 2018
1 parent 64440a4 commit f761082
Show file tree
Hide file tree
Showing 14 changed files with 200 additions and 2 deletions.
18 changes: 18 additions & 0 deletions chrome/browser/apps/guest_view/web_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4430,3 +4430,21 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, AutoResizeMessages) {
embedder->GetRenderWidgetHostView()->GetRenderWidgetHost()->GetProcess(),
guest->GetRenderWidgetHostView()->GetRenderWidgetHost()));
}

// Test that a guest sees the synthetic wheel events of a touchpad pinch.
IN_PROC_BROWSER_TEST_F(WebViewTest, TouchpadPinchSyntheticWheelEvents) {
ASSERT_TRUE(StartEmbeddedTestServer());
LoadAppWithGuest("web_view/touchpad_pinch");
content::WebContents* guest_contents = GetGuestWebContents();

ExtensionTestMessageListener synthetic_wheel_listener("Seen wheel event",
false);

const gfx::Rect contents_rect = guest_contents->GetContainerBounds();
const gfx::Point pinch_position(contents_rect.width() / 2,
contents_rect.height() / 2);
content::SimulateGesturePinchSequence(guest_contents, pinch_position, 1.23,
blink::kWebGestureDeviceTouchpad);

ASSERT_TRUE(synthetic_wheel_listener.WaitUntilSatisfied());
}
23 changes: 23 additions & 0 deletions chrome/browser/apps/platform_apps/app_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1400,4 +1400,27 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, NewWindowAboutBlank) {
ASSERT_TRUE(RunPlatformAppTest("platform_apps/new_window_about_blank"));
}

// Test that an app window sees the synthetic wheel events of a touchpad pinch.
// While the app window itself does not scale in response to a pinch, we
// still offer the synthetic wheels for pages that want to implement custom
// pinch zoom behaviour.
IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
TouchpadPinchSyntheticWheelEvents) {
LoadAndLaunchPlatformApp("touchpad_pinch", "Launched");

WebContents* web_contents = GetFirstAppWindowWebContents();
ASSERT_TRUE(web_contents);

ExtensionTestMessageListener synthetic_wheel_listener("Seen wheel event",
false);

const gfx::Rect contents_rect = web_contents->GetContainerBounds();
const gfx::Point pinch_position(contents_rect.width() / 2,
contents_rect.height() / 2);
content::SimulateGesturePinchSequence(web_contents, pinch_position, 1.23,
blink::kWebGestureDeviceTouchpad);

ASSERT_TRUE(synthetic_wheel_listener.WaitUntilSatisfied());
}

} // namespace extensions
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

chrome.app.runtime.onLaunched.addListener(function() {
chrome.app.window.create('window.html');
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "Touchpad pinch synthetic wheel event",
"version": "1",
"manifest_version": 2,
"app": {
"background": {
"scripts": ["background.js"]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!--
* Copyright (c) 2018 The Chromium Authors. All rights reserved. Use of this
* source code is governed by a BSD-style license that can be found in the
* LICENSE file.
-->
<html>
<head>
<style>
html,body {
height: 100%;
}
</style>
</head>
<body>
<p>Hello.</p>
<script src="window.js"></script>
</body>
</html>
17 changes: 17 additions & 0 deletions chrome/test/data/extensions/platform_apps/touchpad_pinch/window.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

window.onload = () => {
document.body.addEventListener('wheel', (e) => {
chrome.test.sendMessage('Seen wheel event');
});

// We need to wait for the compositor thread to be made aware of the wheel
// listener before sending the pinch event sequence.
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
chrome.test.sendMessage('Launched');
});
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!--
* Copyright (c) 2018 The Chromium Authors. All rights reserved. Use of this
* source code is governed by a BSD-style license that can be found in the
* LICENSE file.
-->
<html>
<body>
<script src="embedder.js"></script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

window.onload = () => {
chrome.test.getConfig(function(config) {
var guestURL = 'http://localhost:' + config.testServer.port +
'/extensions/platform_apps/web_view/touchpad_pinch/guest.html';
var webview = document.createElement('webview');
webview.src = guestURL;
webview.addEventListener('loadstop', () => {
webview.contentWindow.postMessage({}, '*');
});
document.body.appendChild(webview);
});
};

window.addEventListener('message', (e) => {
chrome.test.sendMessage(e.data);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!--
* Copyright (c) 2018 The Chromium Authors. All rights reserved. Use of this
* source code is governed by a BSD-style license that can be found in the
* LICENSE file.
-->
<html>
<head>
<style>
html,body {
height: 100%;
}
</style>
</head>
<body>
<p>Hello.</p>
<script src="guest.js"></script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var parentWindow = null;

window.onload = () => {
document.body.addEventListener('wheel', (e) => {
parentWindow.postMessage('Seen wheel event', '*');
});
};

window.addEventListener('message', (e) => {
parentWindow = e.source;

// We need to wait for the compositor thread to be made aware of the wheel
// listener before sending the pinch event sequence.
window.requestAnimationFrame(() => {
window.requestAnimationFrame(() => {
parentWindow.postMessage('WebViewTest.LAUNCHED', '*');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "<webview> Touchpad pinch synthetic wheel event",
"version": "1",
"manifest_version": 2,
"permissions": [
"webview"
],
"app": {
"background": {
"scripts": ["test.js"]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

chrome.app.runtime.onLaunched.addListener(function() {
chrome.app.window.create('embedder.html');
});
6 changes: 5 additions & 1 deletion components/guest_view/browser/guest_view_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,11 @@ bool GuestViewBase::ShouldFocusPageAfterCrash() {

bool GuestViewBase::PreHandleGestureEvent(WebContents* source,
const blink::WebGestureEvent& event) {
return blink::WebInputEvent::IsPinchGestureEventType(event.GetType());
// Pinch events which cause a scale change should not be routed to a guest.
// We still allow synthetic wheel events for touchpad pinch to go to the page.
DCHECK(!blink::WebInputEvent::IsPinchGestureEventType(event.GetType()) ||
event.NeedsWheelEvent());
return false;
}

void GuestViewBase::UpdatePreferredSize(WebContents* target_web_contents,
Expand Down
12 changes: 11 additions & 1 deletion extensions/browser/app_window/app_web_contents_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,18 @@ AppWebContentsHelper::AppWebContentsHelper(
// static
bool AppWebContentsHelper::ShouldSuppressGestureEvent(
const blink::WebGestureEvent& event) {
// Disable "smart zoom" (double-tap with two fingers on Mac trackpad).
if (event.GetType() == blink::WebInputEvent::kGestureDoubleTap)
return true;

// Disable pinch zooming in app windows.
return blink::WebInputEvent::IsPinchGestureEventType(event.GetType());
if (blink::WebInputEvent::IsPinchGestureEventType(event.GetType())) {
// Only suppress pinch events that cause a scale change. We still
// allow synthetic wheel events for touchpad pinch to go to the page.
return !event.NeedsWheelEvent();
}

return false;
}

content::WebContents* AppWebContentsHelper::OpenURLFromTab(
Expand Down

0 comments on commit f761082

Please sign in to comment.