From 6d636e6bd3c7859b40b6ead3512f4e182cea3905 Mon Sep 17 00:00:00 2001 From: "girard@chromium.org" Date: Wed, 31 Jul 2013 03:15:56 +0000 Subject: [PATCH] Refactored touch-events flag test, correcting "enabled" logic. We previously ignored the "enabled" flag if there was no touch device detected. We also ignored "disabled" in many places, so that we always ended up with touch if there was a touch device. Also, remove reference to gestures - no longer reachable. BUG=176058 Review URL: https://chromiumcodereview.appspot.com/19805005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214535 0039d316-1c4b-4281-b951-d872f2087c98 --- .../ui/views/omnibox/omnibox_view_win.cc | 4 +- chrome/chrome.user32.delay.imports | 1 - .../render_widget_host_view_win.cc | 37 +++---------------- .../browser/web_contents/web_contents_impl.cc | 22 +++-------- ui/base/touch/touch_enabled.cc | 30 +++++++++++++++ ui/base/touch/touch_enabled.h | 18 +++++++++ ui/ui.gyp | 2 + ui/views/win/hwnd_message_handler.cc | 6 ++- 8 files changed, 68 insertions(+), 52 deletions(-) create mode 100644 ui/base/touch/touch_enabled.cc create mode 100644 ui/base/touch/touch_enabled.h diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc index df7e448cae67fd..2aa344633e8a26 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc @@ -61,6 +61,7 @@ #include "ui/base/keycodes/keyboard_codes.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util_win.h" +#include "ui/base/touch/touch_enabled.h" #include "ui/base/win/hwnd_util.h" #include "ui/base/win/mouse_wheel_util.h" #include "ui/base/win/touch_input.h" @@ -1441,7 +1442,8 @@ LRESULT OmniboxViewWin::OnCreate(const CREATESTRUCTW* /*create_struct*/) { // Enable TSF support of RichEdit. SetEditStyle(SES_USECTF, SES_USECTF); } - if (base::win::GetVersion() >= base::win::VERSION_WIN8) { + if ((base::win::GetVersion() >= base::win::VERSION_WIN8) && + ui::AreTouchEventsEnabled()) { BOOL touch_mode = RegisterTouchWindow(m_hWnd, TWF_WANTPALM); DCHECK(touch_mode); } diff --git a/chrome/chrome.user32.delay.imports b/chrome/chrome.user32.delay.imports index baa12316c218c0..ee35cf04a5b5a6 100644 --- a/chrome/chrome.user32.delay.imports +++ b/chrome/chrome.user32.delay.imports @@ -23,7 +23,6 @@ 'GetTouchInputInfo@16', 'IsTouchWindow@8', 'RegisterTouchWindow@8', - 'SetGestureConfig@20', 'UnregisterTouchWindow@4', ], } diff --git a/content/browser/renderer_host/render_widget_host_view_win.cc b/content/browser/renderer_host/render_widget_host_view_win.cc index 50a658aa5151db..f6f6759971bd65 100644 --- a/content/browser/renderer_host/render_widget_host_view_win.cc +++ b/content/browser/renderer_host/render_widget_host_view_win.cc @@ -48,6 +48,7 @@ #include "content/public/browser/native_web_keyboard_event.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" +#include "content/public/browser/render_view_host.h" #include "content/public/common/content_switches.h" #include "content/public/common/page_zoom.h" #include "content/public/common/process_type.h" @@ -62,6 +63,7 @@ #include "ui/base/l10n/l10n_util_win.h" #include "ui/base/text/text_elider.h" #include "ui/base/touch/touch_device.h" +#include "ui/base/touch/touch_enabled.h" #include "ui/base/ui_base_switches.h" #include "ui/base/view_prop.h" #include "ui/base/win/dpi.h" @@ -407,7 +409,7 @@ RenderWidgetHostViewWin::RenderWidgetHostViewWin(RenderWidgetHost* widget) touch_state_(new WebTouchState(this)), pointer_down_context_(false), last_touch_location_(-1, -1), - touch_events_enabled_(false), + touch_events_enabled_(ui::AreTouchEventsEnabled()), gesture_recognizer_(ui::GestureRecognizer::Create(this)) { render_widget_host_->SetView(this); registrar_.Add(this, @@ -926,35 +928,10 @@ void RenderWidgetHostViewWin::ProcessAckedTouchEvent( void RenderWidgetHostViewWin::UpdateDesiredTouchMode() { // Make sure that touch events even make sense. - CommandLine* cmdline = CommandLine::ForCurrentProcess(); - static bool touch_mode = base::win::GetVersion() >= base::win::VERSION_WIN7 && - ui::IsTouchDevicePresent() && ( - !cmdline->HasSwitch(switches::kTouchEvents) || - cmdline->GetSwitchValueASCII(switches::kTouchEvents) != - switches::kTouchEventsDisabled); - - if (!touch_mode) + if (base::win::GetVersion() < base::win::VERSION_WIN7) return; - - // Now we know that the window's current state doesn't match the desired - // state. If we want touch mode, then we attempt to register for touch - // events, and otherwise to unregister. - touch_events_enabled_ = RegisterTouchWindow(m_hWnd, TWF_WANTPALM) == TRUE; - - if (!touch_events_enabled_) { - UnregisterTouchWindow(m_hWnd); - // Single finger panning is consistent with other windows applications. - const DWORD gesture_allow = GC_PAN_WITH_SINGLE_FINGER_VERTICALLY | - GC_PAN_WITH_SINGLE_FINGER_HORIZONTALLY; - const DWORD gesture_block = GC_PAN_WITH_GUTTER; - GESTURECONFIG gc[] = { - { GID_ZOOM, GC_ZOOM, 0 }, - { GID_PAN, gesture_allow , gesture_block}, - { GID_TWOFINGERTAP, GC_TWOFINGERTAP , 0}, - { GID_PRESSANDTAP, GC_PRESSANDTAP , 0} - }; - if (!SetGestureConfig(m_hWnd, 0, arraysize(gc), gc, sizeof(GESTURECONFIG))) - NOTREACHED(); + if (touch_events_enabled_) { + CHECK(RegisterTouchWindow(m_hWnd, TWF_WANTPALM)); } } @@ -2333,8 +2310,6 @@ LRESULT RenderWidgetHostViewWin::OnGestureEvent( UINT message, WPARAM wparam, LPARAM lparam, BOOL& handled) { TRACE_EVENT0("browser", "RenderWidgetHostViewWin::OnGestureEvent"); - // Note that as of M22, touch events are enabled by default on Windows. - // This code should not be reachable. DCHECK(!touch_events_enabled_); handled = FALSE; diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index fe34bc0ef8a11d..3633e3fcba7104 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/debug/trace_event.h" #include "base/lazy_instance.h" +#include "base/logging.h" #include "base/metrics/histogram.h" #include "base/metrics/stats_counters.h" #include "base/strings/string16.h" @@ -81,6 +82,7 @@ #include "net/url_request/url_request_context_getter.h" #include "ui/base/layout.h" #include "ui/base/touch/touch_device.h" +#include "ui/base/touch/touch_enabled.h" #include "ui/base/ui_base_switches.h" #include "ui/gfx/display.h" #include "ui/gfx/screen.h" @@ -559,23 +561,9 @@ WebPreferences WebContentsImpl::GetWebkitPrefs(RenderViewHost* rvh, switches::kDisableGestureRequirementForMediaPlayback); #endif - bool touch_device_present = false; - touch_device_present = ui::IsTouchDevicePresent(); - const std::string touch_enabled_switch = - command_line.HasSwitch(switches::kTouchEvents) ? - command_line.GetSwitchValueASCII(switches::kTouchEvents) : - switches::kTouchEventsAuto; - - if (touch_enabled_switch.empty() || - touch_enabled_switch == switches::kTouchEventsEnabled) { - prefs.touch_enabled = true; - } else if (touch_enabled_switch == switches::kTouchEventsAuto) { - prefs.touch_enabled = touch_device_present; - } else if (touch_enabled_switch != switches::kTouchEventsDisabled) { - LOG(ERROR) << "Invalid --touch-events option: " << touch_enabled_switch; - } - - prefs.device_supports_touch = prefs.touch_enabled && touch_device_present; + prefs.touch_enabled = ui::AreTouchEventsEnabled(); + prefs.device_supports_touch = prefs.touch_enabled && + ui::IsTouchDevicePresent(); #if defined(OS_ANDROID) prefs.device_supports_mouse = false; #endif diff --git a/ui/base/touch/touch_enabled.cc b/ui/base/touch/touch_enabled.cc new file mode 100644 index 00000000000000..79655dfb54eee0 --- /dev/null +++ b/ui/base/touch/touch_enabled.cc @@ -0,0 +1,30 @@ +// Copyright 2013 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/base/touch/touch_enabled.h" +#include "base/command_line.h" +#include "base/logging.h" +#include "ui/base/touch/touch_device.h" +#include "ui/base/ui_base_switches.h" + +namespace ui { + +bool AreTouchEventsEnabled() { + const CommandLine& command_line = *CommandLine::ForCurrentProcess(); + const std::string touch_enabled_switch = + command_line.HasSwitch(switches::kTouchEvents) ? + command_line.GetSwitchValueASCII(switches::kTouchEvents) : + switches::kTouchEventsAuto; + + if (touch_enabled_switch.empty() || + touch_enabled_switch == switches::kTouchEventsEnabled) + return true; + if (touch_enabled_switch == switches::kTouchEventsAuto) + return IsTouchDevicePresent(); + if (touch_enabled_switch != switches::kTouchEventsDisabled) + LOG(ERROR) << "Invalid --touch-events option: " << touch_enabled_switch; + return false; +} + +} // namespace ui diff --git a/ui/base/touch/touch_enabled.h b/ui/base/touch/touch_enabled.h new file mode 100644 index 00000000000000..4155674ddb6320 --- /dev/null +++ b/ui/base/touch/touch_enabled.h @@ -0,0 +1,18 @@ +// Copyright 2013 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. + +#ifndef UI_BASE_TOUCH_TOUCH_ENABLED_H_ +#define UI_BASE_TOUCH_TOUCH_ENABLED_H_ + +#include "ui/base/ui_export.h" + +namespace ui { + +// Returns true if the touch-enabled flag is enabled, or if it is set to auto +// and a touch device is present. +UI_EXPORT bool AreTouchEventsEnabled(); + +} // namespace ui + +#endif // UI_BASE_TOUCH_TOUCH_ENABLED_H_ diff --git a/ui/ui.gyp b/ui/ui.gyp index 8302f463231772..353dfe6f646688 100644 --- a/ui/ui.gyp +++ b/ui/ui.gyp @@ -331,6 +331,8 @@ 'base/touch/touch_device_win.cc', 'base/touch/touch_editing_controller.cc', 'base/touch/touch_editing_controller.h', + 'base/touch/touch_enabled.cc', + 'base/touch/touch_enabled.h', 'base/touch/touch_factory_x11.cc', 'base/touch/touch_factory_x11.h', 'base/ui_base_exports.cc', diff --git a/ui/views/win/hwnd_message_handler.cc b/ui/views/win/hwnd_message_handler.cc index 7ff3900c1c1bc5..92aa2a13e9d852 100644 --- a/ui/views/win/hwnd_message_handler.cc +++ b/ui/views/win/hwnd_message_handler.cc @@ -14,6 +14,7 @@ #include "ui/base/events/event_utils.h" #include "ui/base/gestures/gesture_sequence.h" #include "ui/base/keycodes/keyboard_code_conversion_win.h" +#include "ui/base/touch/touch_enabled.h" #include "ui/base/win/dpi.h" #include "ui/base/win/hwnd_util.h" #include "ui/base/win/mouse_wheel_util.h" @@ -1287,8 +1288,9 @@ LRESULT HWNDMessageHandler::OnCreate(CREATESTRUCT* create_struct) { // Get access to a modifiable copy of the system menu. GetSystemMenu(hwnd(), false); - if (base::win::GetVersion() >= base::win::VERSION_WIN7) - RegisterTouchWindow(hwnd(), 0); + if (base::win::GetVersion() >= base::win::VERSION_WIN7 && + ui::AreTouchEventsEnabled()) + RegisterTouchWindow(hwnd(), TWF_WANTPALM); // We need to allow the delegate to size its contents since the window may not // receive a size notification when its initial bounds are specified at window