New issue
Advanced search Search tips

Issue 817643 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Move several touch device related prefs to ash

Project Member Reported by warx@chromium.org, Mar 1 2018

Issue description

This would include several prefs in chrome/common/pref_names.h:
kTapToClickEnabled
kTapDraggingEnabled
kEnableTouchpadThreeFingerClick, and maybe others.

I am also thinking whether ash::TouchDevicesController should rename to ash::InputDevicesController so that it could also take prefs such as mouse-related, scroll-related.

 
Moving the prefs sgtm.

I would not use the name InputDevice(s)Controller because there is already an InputDeviceController in the mojo ui/window service:
https://cs.chromium.org/chromium/src/services/ui/public/cpp/input_devices/input_device_controller.h?q=inputdeviceco&sq=package:chromium&l=20

InputDeviceConfig? Just keep the existing name?

Comment 2 by warx@chromium.org, Mar 1 2018

Thanks, I will just keep the name for now.

Comment 3 by warx@chromium.org, Mar 12 2018

For the record, prefs::kTapDraggingEnabled is done in crrev.com/c/942686
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b7b1845877b5bc9f7395cccfefdc2fea33d000c

commit 6b7b1845877b5bc9f7395cccfefdc2fea33d000c
Author: Qiang Xu <warx@google.com>
Date: Mon Mar 12 19:00:08 2018

cros: use callback to record TapDragging started uma

changes:
Instead of using ScopedUmaRecorder, which is essentially implementing
in a "callback" way. This CL replaces it with base::OnceCallback, which
makes code more concise.

Bug: 817643
Test: covered by tests
Change-Id: Id99e9c08e8fe49886ca7c30a5901fb4128b04a42
Reviewed-on: https://chromium-review.googlesource.com/957816
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542546}
[modify] https://crrev.com/6b7b1845877b5bc9f7395cccfefdc2fea33d000c/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/6b7b1845877b5bc9f7395cccfefdc2fea33d000c/ash/touch/touch_devices_controller.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e534b672f07210fd804e234f54685678b2cc8d50

commit e534b672f07210fd804e234f54685678b2cc8d50
Author: Qiang Xu <warx@google.com>
Date: Fri Apr 06 22:19:53 2018

cros: remove for_test in register prefs in TouchDevicesController

Bug: 817643
Test: compile and tests
Change-Id: Ib5d5e1e1caa0b467d900efa9f8f197e2ec6aa613
Reviewed-on: https://chromium-review.googlesource.com/1000116
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Qiang Xu <warx@google.com>
Cr-Commit-Position: refs/heads/master@{#548948}
[modify] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/ash/shell.cc
[modify] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/ash/touch/touch_devices_controller.h
[modify] https://crrev.com/e534b672f07210fd804e234f54685678b2cc8d50/chrome/browser/chromeos/preferences.cc

Components: UI>Shell>TouchView
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f818c4100f203bea480a190cae7b08d06f4f4fb

commit 9f818c4100f203bea480a190cae7b08d06f4f4fb
Author: Qiang Xu <warx@google.com>
Date: Wed Jun 27 18:12:20 2018

cros: move TapToClick prefs to ash

changes:
* Move kTapToClickEnabled and kOwnerTapToClickEnabled handling to ash
* In this CL, above pref registrations are still in chrome side because
  ash side doesn't have device owner info yet.

TBR=stevenjb@chromium.org

Bug: 817643
Test: manual and test coverage
Change-Id: I6b11eb65a40968eada67ebf2e8b4fb3f1f0eaddc
Reviewed-on: https://chromium-review.googlesource.com/1116210
Commit-Queue: Qiang Xu <warx@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570842}
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/public/cpp/ash_pref_names.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/public/cpp/ash_pref_names.h
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/shell.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/touch/touch_devices_controller.h
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/ash/touch/touch_devices_controller_unittest.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/browser/chromeos/login/ui/login_display_host_webui.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/browser/chromeos/preferences_chromeos_browsertest.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/common/pref_names.cc
[modify] https://crrev.com/9f818c4100f203bea480a190cae7b08d06f4f4fb/chrome/common/pref_names.h

Comment 8 by warx@chromium.org, Jun 29 2018

Owner: ----
Status: Untriaged (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c997aa787214c031b99f2a83a37898937aeae0fe

commit c997aa787214c031b99f2a83a37898937aeae0fe
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Aug 31 23:38:59 2018

Revert "cros: move TapToClick prefs to ash"

This reverts commit 9f818c4100f203bea480a190cae7b08d06f4f4fb.

Reason for revert:  https://crbug.com/872786 

Original change's description:
> cros: move TapToClick prefs to ash
>
> changes:
> * Move kTapToClickEnabled and kOwnerTapToClickEnabled handling to ash
> * In this CL, above pref registrations are still in chrome side because
>   ash side doesn't have device owner info yet.
>
> TBR=stevenjb@chromium.org
>
> Bug: 817643
> Test: manual and test coverage
> Change-Id: I6b11eb65a40968eada67ebf2e8b4fb3f1f0eaddc
> Reviewed-on: https://chromium-review.googlesource.com/1116210
> Commit-Queue: Qiang Xu <warx@google.com>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570842}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  872786 
Test: On another device, sign in with an account and disable touchpad
      tap-to-click.
      Then on the target device, complete OOBE with the same account.
      In the first user session, tapping should not cause clicks. After
      signing out, tapping should also not cause clicks.

Change-Id: I67a67ba51b56bd14e656e332f8f64e219e3f80ef
Reviewed-on: https://chromium-review.googlesource.com/1199873
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588212}
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/public/cpp/ash_pref_names.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/public/cpp/ash_pref_names.h
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/shell.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/touch/touch_devices_controller.h
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/ash/touch/touch_devices_controller_unittest.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/browser/chromeos/login/ui/login_display_host_webui.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/browser/chromeos/preferences_chromeos_browsertest.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/common/pref_names.cc
[modify] https://crrev.com/c997aa787214c031b99f2a83a37898937aeae0fe/chrome/common/pref_names.h

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 5

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93bfd37480c01f638b0732d5444f5484f2b02c8b

commit 93bfd37480c01f638b0732d5444f5484f2b02c8b
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Tue Sep 04 19:14:10 2018

Revert "cros: move TapToClick prefs to ash"

This reverts commit 9f818c4100f203bea480a190cae7b08d06f4f4fb.

Reason for revert:  https://crbug.com/872786 

Original change's description:
> cros: move TapToClick prefs to ash
>
> changes:
> * Move kTapToClickEnabled and kOwnerTapToClickEnabled handling to ash
> * In this CL, above pref registrations are still in chrome side because
>   ash side doesn't have device owner info yet.
>
> TBR=stevenjb@chromium.org
>
> Bug: 817643
> Test: manual and test coverage
> Change-Id: I6b11eb65a40968eada67ebf2e8b4fb3f1f0eaddc
> Reviewed-on: https://chromium-review.googlesource.com/1116210
> Commit-Queue: Qiang Xu <warx@google.com>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570842}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  872786 
Test: On another device, sign in with an account and disable touchpad
      tap-to-click.
      Then on the target device, complete OOBE with the same account.
      In the first user session, tapping should not cause clicks. After
      signing out, tapping should also not cause clicks.

Change-Id: I67a67ba51b56bd14e656e332f8f64e219e3f80ef
Reviewed-on: https://chromium-review.googlesource.com/1199873
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588212}(cherry picked from commit c997aa787214c031b99f2a83a37898937aeae0fe)
Reviewed-on: https://chromium-review.googlesource.com/1204833
Cr-Commit-Position: refs/branch-heads/3538@{#33}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/public/cpp/ash_pref_names.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/public/cpp/ash_pref_names.h
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/shell.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/touch/touch_devices_controller.h
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/ash/touch/touch_devices_controller_unittest.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/browser/chromeos/login/ui/login_display_host_webui.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/browser/chromeos/preferences_chromeos_browsertest.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/common/pref_names.cc
[modify] https://crrev.com/93bfd37480c01f638b0732d5444f5484f2b02c8b/chrome/common/pref_names.h

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 5

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6da367938a43f868f91669a8ce8106ce17c78f3

commit e6da367938a43f868f91669a8ce8106ce17c78f3
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Wed Sep 05 19:48:59 2018

Revert "cros: move TapToClick prefs to ash"

This reverts commit 9f818c4100f203bea480a190cae7b08d06f4f4fb.

Reason for revert:  https://crbug.com/872786 

Original change's description:
> cros: move TapToClick prefs to ash
>
> changes:
> * Move kTapToClickEnabled and kOwnerTapToClickEnabled handling to ash
> * In this CL, above pref registrations are still in chrome side because
>   ash side doesn't have device owner info yet.
>
> TBR=stevenjb@chromium.org
>
> Bug: 817643
> Test: manual and test coverage
> Change-Id: I6b11eb65a40968eada67ebf2e8b4fb3f1f0eaddc
> Reviewed-on: https://chromium-review.googlesource.com/1116210
> Commit-Queue: Qiang Xu <warx@google.com>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570842}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  872786 
Test: On another device, sign in with an account and disable touchpad
      tap-to-click.
      Then on the target device, complete OOBE with the same account.
      In the first user session, tapping should not cause clicks. After
      signing out, tapping should also not cause clicks.

Change-Id: I67a67ba51b56bd14e656e332f8f64e219e3f80ef
Reviewed-on: https://chromium-review.googlesource.com/1199624
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#883}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/public/cpp/ash_pref_names.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/public/cpp/ash_pref_names.h
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/shell.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/touch/touch_devices_controller.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/touch/touch_devices_controller.h
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/ash/touch/touch_devices_controller_unittest.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/browser/chromeos/login/ui/login_display_host_webui.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/browser/chromeos/preferences_chromeos_browsertest.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/browser/prefs/pref_service_incognito_whitelist.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/common/pref_names.cc
[modify] https://crrev.com/e6da367938a43f868f91669a8ce8106ce17c78f3/chrome/common/pref_names.h

Sign in to add a comment