Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(978)

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

Created:
5 years, 2 months ago by afakhry
Modified:
5 years, 1 month 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?
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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. ...
5 years, 1 month 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 ...
5 years, 1 month 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: > ...
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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: ...
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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: > ...
5 years, 1 month 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 > ...
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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: ...
5 years, 1 month 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: ...
5 years, 1 month 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.
5 years, 1 month 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. > ...
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month ago (2015-11-06 17:33:09 UTC) #27
Mark P
actions.xml lgtm
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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
5 years, 1 month ago (2015-11-10 01:13:47 UTC) #33
commit-bot: I haz the power
Committed patchset #13 (id:240001)
5 years, 1 month 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}
5 years, 1 month ago (2015-11-10 02:32:17 UTC) #35
dtapuska
5 years, 1 month 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