Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(17)

Issue 1412623006: Developer Feature: Add Debug Accelerators to Toggle Touchscreen/Touchpad On or Off (CrOS) (Closed)

Created:
4 years, 5 months ago by afakhry
Modified:
4 years, 4 months ago
CC:
chromium-reviews, ozone-reviews_chromium.org, sadrul, yusukes+watch_chromium.org, derat+watch_chromium.org, asvitkine+watch_chromium.org, oshima+watch_chromium.org, kalyank, davemoore+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Developer Feature: Add Debug Accelerators to Toggle Touchscreen/Touchpad On or Off Add two debugging accelerators for toggling the touchscreen / touchpad on or off. These settings must be persistent between reboots. We ignore the case when users turn off the debugging accelerators flag. This is only for ChromeOS. We add an UMA user metric to track how often they're used. The two shortcuts are: Search+Shift+T ---> Toggle touch screen. Search+Shift+P ---> Toggle touchpad. BUG=546654 Committed: https://crrev.com/e8eac60b676dc33060e16c1d0e69f8c5150e68ca Cr-Commit-Position: refs/heads/master@{#358758}

Patch Set 1 #

Patch Set 2 : [WIP]: Toggle touchpad on X11 too. #

Patch Set 3 : Ignore touchpad re-enabling in ScopedDisableInternalMouseAndKeyboard if it's already disabled. #

Patch Set 4 : Fix actions.xml. #

Total comments: 16

Patch Set 5 : Rebase #

Patch Set 6 : Oshima's comments #

Total comments: 2

Patch Set 7 : oshima's nits and make touch_factory_x11 use the correct functions. #

Patch Set 8 : Fix compile error #

Patch Set 9 : Fix compile error 2 #

Total comments: 10

Patch Set 10 : sadrul's and spang's comments #

Patch Set 11 : Add enable/disable capability to ozone's touchscreen devices #

Total comments: 5

Patch Set 12 : Nits #

Total comments: 4

Patch Set 13 : sadrul's comment #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+332 lines, -86 lines) Patch
M ash/accelerators/accelerator_controller.cc View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download
M ash/accelerators/accelerator_table.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M ash/accelerators/accelerator_table.cc View 1 2 3 4 5 6 chunks +14 lines, -0 lines 0 comments Download
M ash/accelerators/debug_commands.cc View 1 2 3 4 5 3 chunks +25 lines, -0 lines 0 comments Download
M ash/shell_delegate.h View 1 chunk +4 lines, -0 lines 0 comments Download
M ash/wm/maximize_mode/maximize_mode_controller.cc View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_ozone.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_ozone.cc View 1 2 3 4 5 2 chunks +10 lines, -4 lines 0 comments Download
M ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc View 1 2 3 4 5 3 chunks +12 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/system/input_device_settings.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +16 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/system/input_device_settings.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +53 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/system/input_device_settings_impl_ozone.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +10 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/system/input_device_settings_impl_x11.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +36 lines, -0 lines 0 comments Download
M chrome/browser/prefs/browser_prefs.cc View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/chrome_shell_delegate.h View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/chrome_shell_delegate.cc View 2 chunks +13 lines, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/legacy_render_widget_host_win.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M tools/metrics/actions/actions.xml View 1 2 3 4 5 6 7 8 9 1 chunk +16 lines, -0 lines 0 comments Download
M ui/base/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -2 lines 0 comments Download
M ui/base/touch/touch_enabled.h View 1 2 3 4 5 6 1 chunk +0 lines, -18 lines 0 comments Download
M ui/base/touch/touch_enabled.cc View 1 2 3 4 5 6 1 chunk +0 lines, -32 lines 0 comments Download
M ui/base/ui_base.gyp View 1 2 3 4 5 6 1 chunk +0 lines, -2 lines 0 comments Download
M ui/events/base_event_utils.h View 1 2 3 4 5 6 1 chunk +10 lines, -0 lines 0 comments Download
M ui/events/base_event_utils.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +46 lines, -1 line 1 comment Download
M ui/events/devices/x11/device_data_manager_x11.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M ui/events/devices/x11/device_data_manager_x11.cc View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M ui/events/devices/x11/touch_factory_x11.h View 1 2 3 4 5 6 1 chunk +0 lines, -3 lines 0 comments Download
M ui/events/devices/x11/touch_factory_x11.cc View 1 2 3 4 5 6 7 chunks +6 lines, -12 lines 0 comments Download
M ui/events/ozone/evdev/input_controller_evdev.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M ui/events/ozone/evdev/input_controller_evdev.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +9 lines, -0 lines 0 comments Download
M ui/events/ozone/evdev/input_device_factory_evdev.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
M ui/events/ozone/evdev/input_device_settings_evdev.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M ui/ozone/public/input_controller.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -0 lines 0 comments Download
M ui/ozone/public/input_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +8 lines, -0 lines 0 comments Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 37 (9 generated)
afakhry
Oshima, can you please review?
4 years, 5 months ago (2015-10-26 19:36:22 UTC) #3
oshima
On 2015/10/26 19:36:22, afakhry wrote: > Oshima, can you please review? What happened to the ...
4 years, 5 months ago (2015-10-26 22:32:06 UTC) #4
Daniel Erat
please add some more background to the bug. why are these being added? what happens ...
4 years, 5 months ago (2015-10-26 22:40:20 UTC) #6
afakhry
On 2015/10/26 22:40:20, Daniel Erat wrote: > please add some more background to the bug. ...
4 years, 5 months ago (2015-10-28 17:15:20 UTC) #7
oshima
+sadrul@ https://codereview.chromium.org/1412623006/diff/60001/ash/accelerators/accelerator_controller.cc File ash/accelerators/accelerator_controller.cc (right): https://codereview.chromium.org/1412623006/diff/60001/ash/accelerators/accelerator_controller.cc#newcode664 ash/accelerators/accelerator_controller.cc:664: #endif // defined(OS_CHROMEOS) move them to debug_commands.cc https://codereview.chromium.org/1412623006/diff/60001/ash/accelerators/accelerator_table.h ...
4 years, 5 months ago (2015-10-29 23:38:43 UTC) #9
afakhry
https://codereview.chromium.org/1412623006/diff/60001/ash/accelerators/accelerator_controller.cc File ash/accelerators/accelerator_controller.cc (right): https://codereview.chromium.org/1412623006/diff/60001/ash/accelerators/accelerator_controller.cc#newcode664 ash/accelerators/accelerator_controller.cc:664: #endif // defined(OS_CHROMEOS) On 2015/10/29 23:38:43, oshima wrote: > ...
4 years, 5 months ago (2015-10-30 05:18:03 UTC) #10
oshima
lgtm my bits but please wait for review from derat and sadrul https://codereview.chromium.org/1412623006/diff/60001/ui/base/touch/touch_enabled.cc File ui/base/touch/touch_enabled.cc ...
4 years, 5 months ago (2015-10-31 00:27:49 UTC) #11
oshima
lgtm my bits but please wait for review from derat and sadrul https://codereview.chromium.org/1412623006/diff/60001/ui/base/touch/touch_enabled.cc File ui/base/touch/touch_enabled.cc ...
4 years, 5 months ago (2015-10-31 00:27:49 UTC) #12
afakhry
derat@ and sadrul@ Could you please take a look? https://codereview.chromium.org/1412623006/diff/60001/ui/base/touch/touch_enabled.cc File ui/base/touch/touch_enabled.cc (right): https://codereview.chromium.org/1412623006/diff/60001/ui/base/touch/touch_enabled.cc#newcode19 ui/base/touch/touch_enabled.cc:19: ...
4 years, 5 months ago (2015-11-03 18:02:56 UTC) #13
sadrul
https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc File chrome/browser/chromeos/system/input_device_settings.cc (right): https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc#newcode229 chrome/browser/chromeos/system/input_device_settings.cc:229: void InputDeviceSettings::InitTouchDevicesStatusFromLocalPrefs() { Should we really store developer-only flags ...
4 years, 5 months ago (2015-11-03 19:09:26 UTC) #14
spang
https://codereview.chromium.org/1412623006/diff/160001/ui/ozone/platform/drm/host/drm_window_host.cc File ui/ozone/platform/drm/host/drm_window_host.cc (right): https://codereview.chromium.org/1412623006/diff/160001/ui/ozone/platform/drm/host/drm_window_host.cc#newcode142 ui/ozone/platform/drm/host/drm_window_host.cc:142: if (!ui::AreTouchEventsEnabled()) If you must do this, this isn't ...
4 years, 5 months ago (2015-11-03 22:11:45 UTC) #16
afakhry
https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc File chrome/browser/chromeos/system/input_device_settings.cc (right): https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc#newcode229 chrome/browser/chromeos/system/input_device_settings.cc:229: void InputDeviceSettings::InitTouchDevicesStatusFromLocalPrefs() { On 2015/11/03 19:09:26, sadrul wrote: > ...
4 years, 5 months ago (2015-11-04 02:28:21 UTC) #17
spang
On 2015/11/04 02:28:21, afakhry wrote: > https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc > File chrome/browser/chromeos/system/input_device_settings.cc (right): > > https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc#newcode229 > ...
4 years, 5 months ago (2015-11-04 17:23:32 UTC) #18
afakhry
On 2015/11/04 17:23:32, spang wrote: > On 2015/11/04 02:28:21, afakhry wrote: > > > https://codereview.chromium.org/1412623006/diff/160001/chrome/browser/chromeos/system/input_device_settings.cc ...
4 years, 5 months ago (2015-11-04 19:17:08 UTC) #19
spang
thanks, various ozone files lgtm % couple nits https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h File ui/events/ozone/evdev/input_controller_evdev.h (right): https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h#newcode66 ui/events/ozone/evdev/input_controller_evdev.h:66: void ...
4 years, 5 months ago (2015-11-04 20:16:27 UTC) #20
spang
https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h File ui/events/ozone/evdev/input_controller_evdev.h (right): https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h#newcode66 ui/events/ozone/evdev/input_controller_evdev.h:66: void SetTouchscreenEnabled(bool enabled) override; On 2015/11/04 20:16:27, spang wrote: ...
4 years, 5 months ago (2015-11-04 20:16:58 UTC) #21
afakhry
Thanks, spang! Sadrul, can you please take another look? https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h File ui/events/ozone/evdev/input_controller_evdev.h (right): https://codereview.chromium.org/1412623006/diff/200001/ui/events/ozone/evdev/input_controller_evdev.h#newcode66 ui/events/ozone/evdev/input_controller_evdev.h:66: ...
4 years, 5 months ago (2015-11-05 01:22:11 UTC) #22
afakhry
Sadrul, friendly ping. +thestig@chromium.org for browser_prefs.cc. +mpearson@chromium.org for actions.xml.
4 years, 5 months ago (2015-11-06 03:05:20 UTC) #24
Lei Zhang
On 2015/11/06 03:05:20, afakhry wrote: > Sadrul, friendly ping. > > mailto:+thestig@chromium.org for browser_prefs.cc. > ...
4 years, 5 months ago (2015-11-06 03:55:10 UTC) #25
Mark P
https://codereview.chromium.org/1412623006/diff/220001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/1412623006/diff/220001/tools/metrics/actions/actions.xml#newcode731 tools/metrics/actions/actions.xml:731: Metric recorded when the user toggles the touchpad status ...
4 years, 5 months ago (2015-11-06 05:02:38 UTC) #26
afakhry
https://codereview.chromium.org/1412623006/diff/220001/tools/metrics/actions/actions.xml File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/1412623006/diff/220001/tools/metrics/actions/actions.xml#newcode731 tools/metrics/actions/actions.xml:731: Metric recorded when the user toggles the touchpad status ...
4 years, 5 months ago (2015-11-06 17:33:09 UTC) #27
Mark P
actions.xml lgtm
4 years, 5 months ago (2015-11-06 19:13:42 UTC) #28
sadrul
LGTM https://codereview.chromium.org/1412623006/diff/220001/ui/events/base_event_utils.cc File ui/events/base_event_utils.cc (right): https://codereview.chromium.org/1412623006/diff/220001/ui/events/base_event_utils.cc#newcode88 ui/events/base_event_utils.cc:88: UpdateAndCacheTouchStatus(); You can get rid of is_touch_events_status_cached. Define ...
4 years, 4 months ago (2015-11-09 21:38:53 UTC) #29
afakhry
https://codereview.chromium.org/1412623006/diff/220001/ui/events/base_event_utils.cc File ui/events/base_event_utils.cc (right): https://codereview.chromium.org/1412623006/diff/220001/ui/events/base_event_utils.cc#newcode88 ui/events/base_event_utils.cc:88: UpdateAndCacheTouchStatus(); On 2015/11/09 21:38:53, sadrul wrote: > You can ...
4 years, 4 months ago (2015-11-10 00:37:11 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1412623006/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1412623006/240001
4 years, 4 months ago (2015-11-10 01:13:47 UTC) #33
commit-bot: I haz the power
Committed patchset #13 (id:240001)
4 years, 4 months ago (2015-11-10 02:31:31 UTC) #34
commit-bot: I haz the power
Patchset 13 (id:??) landed as https://crrev.com/e8eac60b676dc33060e16c1d0e69f8c5150e68ca Cr-Commit-Position: refs/heads/master@{#358758}
4 years, 4 months ago (2015-11-10 02:32:17 UTC) #35
dtapuska
4 years, 4 months ago (2015-11-18 04:00:55 UTC) #37
Message was sent while issue was closed.
https://codereview.chromium.org/1412623006/diff/240001/ui/events/base_event_u...
File ui/events/base_event_utils.cc (right):

https://codereview.chromium.org/1412623006/diff/240001/ui/events/base_event_u...
ui/events/base_event_utils.cc:33: switches::kTouchEventsAuto;
If no command switch is set; auto mode is enable and then it is returned as
true.

Shouldn't it be if a touch device is present. That is what it was before.. We
are seeing a regression in M48; issue 555746

Powered by Google App Engine
This is Rietveld 408576698