Skip to content

Commit

Permalink
Support double tap to zoom in tablet-mode pages
Browse files Browse the repository at this point in the history
This CL adds support for double taps to zoom pages that
are whitelisted to run in tablet mode (whose webkit prefs
were updated to have a mobile-like behavior). All other
webpages and system UI components should not be affected.

Summary:
- GestureDetector is informed whether it should process
  double tap events by the GestureListenerImpl.
- GestureListenerImpl asks its GestureProviderClient
  whether double tap events should be processed.
- GestureProviderAura as a GestureProviderClient asks
  its GestureConsumer if double tap events should be
  processed.
- GestureConsumers can specify if they can handle double
  tap events.
- aura::Window as a GestureConsumer asks its WindowDelegate
  if they can handle double tap events.
- RenderWidgetHostViewAura as a WindowDelegate checks its
  RenderViewHost's webkit preferences to see if double tap
  to zoom is enabled.
- On switching to tablet mode, if the webpage should have
  a mobile-like behavior, its webkit preferences are updated
  to set double tap to zoom enabled.

BUG=822455
TEST=browser_tests --gtest_filter=TabletModePageBehaviorTest.*
TEST=events_unittests --gtest_filter=GestureProviderTest*

Change-Id: Idfeb3af54af8e006bc71812135387dd4866e9c72
Reviewed-on: https://chromium-review.googlesource.com/1045488
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562686}
  • Loading branch information
Ahmed Fakhry authored and Commit Bot committed May 30, 2018
1 parent bd2798b commit 4222145
Show file tree
Hide file tree
Showing 33 changed files with 221 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/web_preferences.h"

Expand Down Expand Up @@ -68,6 +70,8 @@ void ChromeContentBrowserClientChromeOsPart::OverrideWebkitPrefs(
if (ShouldExcludePage(contents))
return;

web_prefs->double_tap_to_zoom_enabled = true;
web_prefs->text_autosizing_enabled = true;
web_prefs->viewport_enabled = true;
web_prefs->viewport_meta_enabled = true;
web_prefs->shrinks_viewport_contents_to_fit = true;
Expand Down
33 changes: 16 additions & 17 deletions chrome/browser/ui/ash/tablet_mode_page_behavior_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "chrome/common/url_constants.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/web_preferences.h"
#include "content/public/test/browser_test_utils.h"
Expand Down Expand Up @@ -48,9 +49,13 @@ class TabletModePageBehaviorTest : public InProcessBrowserTest {
return web_contents->GetRenderViewHost()->GetWebkitPreferences();
}

void ValidateWebPrefs(const content::WebPreferences& web_prefs,
void ValidateWebPrefs(const content::WebContents* web_contents,
bool tablet_mode_enabled) const {
const content::WebPreferences web_prefs =
GetWebKitPreferences(web_contents);
if (tablet_mode_enabled) {
EXPECT_TRUE(web_prefs.double_tap_to_zoom_enabled);
EXPECT_TRUE(web_prefs.text_autosizing_enabled);
EXPECT_TRUE(web_prefs.viewport_enabled);
EXPECT_TRUE(web_prefs.viewport_meta_enabled);
EXPECT_TRUE(web_prefs.shrinks_viewport_contents_to_fit);
Expand All @@ -59,6 +64,8 @@ class TabletModePageBehaviorTest : public InProcessBrowserTest {
EXPECT_FLOAT_EQ(web_prefs.default_minimum_page_scale_factor, 0.25f);
EXPECT_FLOAT_EQ(web_prefs.default_maximum_page_scale_factor, 5.0f);
} else {
EXPECT_FALSE(web_prefs.double_tap_to_zoom_enabled);
EXPECT_FALSE(web_prefs.text_autosizing_enabled);
EXPECT_FALSE(web_prefs.viewport_enabled);
EXPECT_FALSE(web_prefs.viewport_meta_enabled);
EXPECT_FALSE(web_prefs.shrinks_viewport_contents_to_fit);
Expand All @@ -82,30 +89,25 @@ IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest,

// Validate that before tablet mode is enabled, mobile-behavior-related prefs
// are disabled.
ValidateWebPrefs(GetWebKitPreferences(web_contents),
false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, false /* tablet_mode_enabled */);

// Now enable tablet mode, and expect that the same page's web prefs get
// updated.
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
ValidateWebPrefs(GetWebKitPreferences(web_contents),
true /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, true /* tablet_mode_enabled */);

// Any newly added pages should have the correct tablet mode prefs.
Browser* browser_2 = CreateBrowser(browser()->profile());
auto* web_contents_2 = GetActiveWebContents(browser_2);
ASSERT_TRUE(web_contents_2);
ValidateWebPrefs(GetWebKitPreferences(web_contents_2),
true /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents_2, true /* tablet_mode_enabled */);

// Disable tablet mode and expect both pages's prefs are updated.
ToggleTabletMode();
ASSERT_FALSE(GetTabletModeEnabled());
ValidateWebPrefs(GetWebKitPreferences(web_contents),
false /* tablet_mode_enabled */);
ValidateWebPrefs(GetWebKitPreferences(web_contents_2),
false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents_2, false /* tablet_mode_enabled */);
}

IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeInternalPages) {
Expand All @@ -122,8 +124,7 @@ IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeInternalPages) {
// remain unaffected as if tablet mode is off.
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
ValidateWebPrefs(GetWebKitPreferences(web_contents),
false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, false /* tablet_mode_enabled */);
}

IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeHostedApps) {
Expand All @@ -145,8 +146,7 @@ IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeHostedApps) {
// app remain unaffected as if tablet mode is off.
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
ValidateWebPrefs(GetWebKitPreferences(web_contents),
false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, false /* tablet_mode_enabled */);
}

IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeNTPs) {
Expand All @@ -161,8 +161,7 @@ IN_PROC_BROWSER_TEST_F(TabletModePageBehaviorTest, ExcludeNTPs) {
// NTPs should not be affected in tablet mode.
ToggleTabletMode();
ASSERT_TRUE(GetTabletModeEnabled());
ValidateWebPrefs(GetWebKitPreferences(web_contents),
false /* tablet_mode_enabled */);
ValidateWebPrefs(web_contents, false /* tablet_mode_enabled */);
}

} // namespace
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ bool StylusTextSelector::OnTouchEvent(const MotionEvent& event) {
if (!gesture_detector_)
gesture_detector_ = CreateGestureDetector(this);

gesture_detector_->OnTouchEvent(event);
gesture_detector_->OnTouchEvent(event, false /* should_process_double_tap */);

// Always return true, even if |gesture_detector_| technically doesn't
// consume the event. This prevents forwarding of a partial touch stream.
Expand Down
4 changes: 4 additions & 0 deletions content/browser/renderer_host/input/touch_emulator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,10 @@ void TouchEmulator::OnGestureEvent(const ui::GestureEventData& gesture) {
}
}

bool TouchEmulator::RequiresDoubleTapGestureEvents() const {
return true;
}

void TouchEmulator::InjectTouchEvent(const blink::WebTouchEvent& event,
base::OnceClosure callback) {
DCHECK(enabled() && mode_ == Mode::kInjectingTouchEvents);
Expand Down
1 change: 1 addition & 0 deletions content/browser/renderer_host/input/touch_emulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class CONTENT_EXPORT TouchEmulator : public ui::GestureProviderClient {
private:
// ui::GestureProviderClient implementation.
void OnGestureEvent(const ui::GestureEventData& gesture) override;
bool RequiresDoubleTapGestureEvents() const override;

// Returns cursor size in DIP.
gfx::SizeF InitCursorFromResource(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,10 @@ void RenderWidgetHostViewAndroid::OnGestureEvent(
SendGestureEvent(web_gesture);
}

bool RenderWidgetHostViewAndroid::RequiresDoubleTapGestureEvents() const {
return true;
}

void RenderWidgetHostViewAndroid::OnSizeChanged() {
if (ime_adapter_android_)
ime_adapter_android_->UpdateAfterViewSizeChanged();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid

// ui::GestureProviderClient implementation.
void OnGestureEvent(const ui::GestureEventData& gesture) override;
bool RequiresDoubleTapGestureEvents() const override;

// ui::WindowAndroidObserver implementation.
void OnCompositingDidCommit() override {}
Expand Down
5 changes: 5 additions & 0 deletions content/browser/renderer_host/render_widget_host_view_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1604,6 +1604,11 @@ bool RenderWidgetHostViewAura::HasHitTestMask() const {
void RenderWidgetHostViewAura::GetHitTestMask(gfx::Path* mask) const {
}

bool RenderWidgetHostViewAura::RequiresDoubleTapGestureEvents() const {
RenderViewHost* rvh = RenderViewHost::From(host());
return rvh && rvh->GetWebkitPreferences().double_tap_to_zoom_enabled;
}

////////////////////////////////////////////////////////////////////////////////
// RenderWidgetHostViewAura, ui::EventHandler implementation:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
void OnWindowTargetVisibilityChanged(bool visible) override;
bool HasHitTestMask() const override;
void GetHitTestMask(gfx::Path* mask) const override;
bool RequiresDoubleTapGestureEvents() const override;

// Overridden from ui::EventHandler:
void OnKeyEvent(ui::KeyEvent* event) override;
Expand Down
4 changes: 2 additions & 2 deletions content/public/common/common_param_traits_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,13 @@ IPC_STRUCT_TRAITS_BEGIN(content::WebPreferences)
IPC_STRUCT_TRAITS_MEMBER(user_gesture_required_for_presentation)
IPC_STRUCT_TRAITS_MEMBER(text_track_margin_percentage)
IPC_STRUCT_TRAITS_MEMBER(save_previous_document_resources)
#if defined(OS_ANDROID)
IPC_STRUCT_TRAITS_MEMBER(text_autosizing_enabled)
IPC_STRUCT_TRAITS_MEMBER(double_tap_to_zoom_enabled)
#if defined(OS_ANDROID)
IPC_STRUCT_TRAITS_MEMBER(font_scale_factor)
IPC_STRUCT_TRAITS_MEMBER(device_scale_adjustment)
IPC_STRUCT_TRAITS_MEMBER(force_enable_zoom)
IPC_STRUCT_TRAITS_MEMBER(fullscreen_supported)
IPC_STRUCT_TRAITS_MEMBER(double_tap_to_zoom_enabled)
IPC_STRUCT_TRAITS_MEMBER(media_playback_gesture_whitelist_scope)
IPC_STRUCT_TRAITS_MEMBER(default_video_poster_url)
IPC_STRUCT_TRAITS_MEMBER(support_deprecated_target_density_dpi)
Expand Down
7 changes: 5 additions & 2 deletions content/public/common/web_preferences.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,16 @@ WebPreferences::WebPreferences()
user_gesture_required_for_presentation(true),
text_track_margin_percentage(0.0f),
immersive_mode_enabled(false),
#if defined(OS_ANDROID)
#if !defined(OS_ANDROID)
text_autosizing_enabled(false),
double_tap_to_zoom_enabled(false),
#else
text_autosizing_enabled(true),
double_tap_to_zoom_enabled(true),
font_scale_factor(1.0f),
device_scale_adjustment(1.0f),
force_enable_zoom(false),
fullscreen_supported(true),
double_tap_to_zoom_enabled(true),
support_deprecated_target_density_dpi(false),
use_legacy_background_size_shorthand_behavior(false),
wide_viewport_quirk(false),
Expand Down
6 changes: 4 additions & 2 deletions content/public/common/web_preferences.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,15 @@ struct CONTENT_EXPORT WebPreferences {

bool immersive_mode_enabled;

#if defined(OS_ANDROID)
bool text_autosizing_enabled;

bool double_tap_to_zoom_enabled;

#if defined(OS_ANDROID)
float font_scale_factor;
float device_scale_adjustment;
bool force_enable_zoom;
bool fullscreen_supported;
bool double_tap_to_zoom_enabled;
std::string media_playback_gesture_whitelist_scope;
GURL default_video_poster_url;
bool support_deprecated_target_density_dpi;
Expand Down
5 changes: 3 additions & 2 deletions content/renderer/render_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -844,15 +844,16 @@ void RenderView::ApplyWebPreferences(const WebPreferences& prefs,
static_cast<WebSettings::SavePreviousDocumentResources>(
prefs.save_previous_document_resources));

settings->SetTextAutosizingEnabled(prefs.text_autosizing_enabled);
settings->SetDoubleTapToZoomEnabled(prefs.double_tap_to_zoom_enabled);

#if defined(OS_ANDROID)
settings->SetAllowCustomScrollbarInMainFrame(false);
settings->SetTextAutosizingEnabled(prefs.text_autosizing_enabled);
settings->SetAccessibilityFontScaleFactor(prefs.font_scale_factor);
settings->SetDeviceScaleAdjustment(prefs.device_scale_adjustment);
settings->SetFullscreenSupported(prefs.fullscreen_supported);
web_view->SetIgnoreViewportTagScaleLimits(prefs.force_enable_zoom);
settings->SetAutoZoomFocusedNodeToLegibleScale(true);
settings->SetDoubleTapToZoomEnabled(prefs.double_tap_to_zoom_enabled);
settings->SetMediaPlaybackGestureWhitelistScope(
blink::WebString::FromUTF8(prefs.media_playback_gesture_whitelist_scope));
settings->SetDefaultVideoPosterURL(
Expand Down
1 change: 1 addition & 0 deletions ui/aura/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ jumbo_component("aura") {
"scoped_simple_keyboard_hook.cc",
"scoped_window_targeter.cc",
"window.cc",
"window_delegate.cc",
"window_event_dispatcher.cc",
"window_observer.cc",
"window_occlusion_tracker.cc",
Expand Down
4 changes: 4 additions & 0 deletions ui/aura/window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,10 @@ bool Window::IsEmbeddingClient() const {
return embeds_external_client_;
}

bool Window::RequiresDoubleTapGestureEvents() const {
return delegate_ && delegate_->RequiresDoubleTapGestureEvents();
}

void Window::OnPaintLayer(const ui::PaintContext& context) {
Paint(context);
}
Expand Down
3 changes: 3 additions & 0 deletions ui/aura/window.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ class AURA_EXPORT Window : public ui::LayerDelegate,
// Returns whether this window is embedding another client.
bool IsEmbeddingClient() const;

// ui::GestureConsumer:
bool RequiresDoubleTapGestureEvents() const override;

protected:
// Deletes (or removes if not owned by parent) all child windows. Intended for
// use from the destructor.
Expand Down
13 changes: 13 additions & 0 deletions ui/aura/window_delegate.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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.

#include "ui/aura/window_delegate.h"

namespace aura {

bool WindowDelegate::RequiresDoubleTapGestureEvents() const {
return false;
}

} // namespace aura
4 changes: 4 additions & 0 deletions ui/aura/window_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ class AURA_EXPORT WindowDelegate : public ui::EventHandler {
// |surface_info| for the first time.
virtual void OnFirstSurfaceActivation(const viz::SurfaceInfo& surface_info) {}

// Returns whether the window wants to receive and handle double tap gesture
// events. Defaults to false.
virtual bool RequiresDoubleTapGestureEvents() const;

protected:
~WindowDelegate() override {}
};
Expand Down
1 change: 1 addition & 0 deletions ui/events/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ component("events") {
"events_export.h",
"events_stub.cc",
"gestures/gesture_recognizer_impl_mac.cc",
"gestures/gesture_types.cc",
"gestures/gesture_types.h",
"keyboard_hook.h",
"keyboard_hook_base.cc",
Expand Down
1 change: 1 addition & 0 deletions ui/events/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class EVENTS_EXPORT Event {
case ET_GESTURE_SCROLL_END:
case ET_GESTURE_SCROLL_UPDATE:
case ET_GESTURE_TAP:
case ET_GESTURE_DOUBLE_TAP:
case ET_GESTURE_TAP_CANCEL:
case ET_GESTURE_TAP_DOWN:
case ET_GESTURE_BEGIN:
Expand Down
4 changes: 4 additions & 0 deletions ui/events/gesture_detection/filtered_gesture_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ void FilteredGestureProvider::OnGestureEvent(const GestureEventData& event) {
GestureEventDataPacket::FromTouchTimeout(event));
}

bool FilteredGestureProvider::RequiresDoubleTapGestureEvents() const {
return client_->RequiresDoubleTapGestureEvents();
}

void FilteredGestureProvider::ForwardGestureEvent(
const GestureEventData& event) {
client_->OnGestureEvent(event);
Expand Down
1 change: 1 addition & 0 deletions ui/events/gesture_detection/filtered_gesture_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class GESTURE_DETECTION_EXPORT FilteredGestureProvider
private:
// GestureProviderClient implementation.
void OnGestureEvent(const ui::GestureEventData& event) override;
bool RequiresDoubleTapGestureEvents() const override;

// TouchDispositionGestureFilterClient implementation.
void ForwardGestureEvent(const ui::GestureEventData& event) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class FilteredGestureProviderTest : public GestureProviderClient,

// GestureProviderClient implementation.
void OnGestureEvent(const GestureEventData&) override {}
bool RequiresDoubleTapGestureEvents() const override { return false; }

private:
base::test::ScopedTaskEnvironment scoped_task_environment_;
Expand Down
8 changes: 7 additions & 1 deletion ui/events/gesture_detection/gesture_configuration_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
namespace ui {
namespace {

#if defined(OS_CHROMEOS)
constexpr bool kDoubleTapAuraSupport = true;
#else
constexpr bool kDoubleTapAuraSupport = false;
#endif // defined(OS_CHROMEOS)

class GestureConfigurationAura : public GestureConfiguration {
public:
~GestureConfigurationAura() override {
Expand All @@ -23,7 +29,7 @@ class GestureConfigurationAura : public GestureConfiguration {

private:
GestureConfigurationAura() : GestureConfiguration() {
set_double_tap_enabled(false);
set_double_tap_enabled(kDoubleTapAuraSupport);
set_double_tap_timeout_in_ms(semi_long_press_time_in_ms());
set_gesture_begin_end_types_enabled(true);
set_min_gesture_bounds_length(default_radius());
Expand Down
Loading

0 comments on commit 4222145

Please sign in to comment.