Commit c997aa78 authored by Michael Giuffrida's avatar Michael Giuffrida Committed by Commit Bot

Revert "cros: move TapToClick prefs to ash"

This reverts commit 9f818c41.

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/1199873Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588212}
parent 1e8e715a
......@@ -287,12 +287,6 @@ const char kSystemBluetoothAdapterEnabled[] =
// A boolean pref which determines whether tap-dragging is enabled.
const char kTapDraggingEnabled[] = "settings.touchpad.enable_tap_dragging";
// A boolean pref set to true if touchpad tap-to-click is enabled.
const char kTapToClickEnabled[] = "settings.touchpad.enable_tap_to_click";
// Copy of owner tap-to-click option to use on login screen.
const char kOwnerTapToClickEnabled[] = "owner.touchpad.enable_tap_to_click";
// Boolean prefs for the status of the touchscreen and the touchpad.
const char kTouchpadEnabled[] = "events.touch_pad.enabled";
const char kTouchscreenEnabled[] = "events.touch_screen.enabled";
......
......@@ -111,8 +111,6 @@ ASH_PUBLIC_EXPORT extern const char kUserBluetoothAdapterEnabled[];
ASH_PUBLIC_EXPORT extern const char kSystemBluetoothAdapterEnabled[];
ASH_PUBLIC_EXPORT extern const char kTapDraggingEnabled[];
ASH_PUBLIC_EXPORT extern const char kTapToClickEnabled[];
ASH_PUBLIC_EXPORT extern const char kOwnerTapToClickEnabled[];
ASH_PUBLIC_EXPORT extern const char kTouchpadEnabled[];
ASH_PUBLIC_EXPORT extern const char kTouchscreenEnabled[];
......
......@@ -256,7 +256,7 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test) {
PaletteTray::RegisterProfilePrefs(registry);
PaletteWelcomeBubble::RegisterProfilePrefs(registry);
ShelfController::RegisterProfilePrefs(registry);
TouchDevicesController::RegisterProfilePrefs(registry, for_test);
TouchDevicesController::RegisterProfilePrefs(registry);
CapsLockNotificationController::RegisterProfilePrefs(registry, for_test);
}
......@@ -404,7 +404,6 @@ void Shell::RegisterLocalStatePrefs(PrefRegistrySimple* registry,
WallpaperController::RegisterLocalStatePrefs(registry);
BluetoothPowerController::RegisterLocalStatePrefs(registry);
DetachableBaseHandler::RegisterPrefs(registry);
TouchDevicesController::RegisterLocalStatePrefs(registry, for_test);
// Note: DisplayPrefs are registered in chrome in AshShellInit::RegisterPrefs
// (see comment there for details).
if (for_test)
......
......@@ -52,20 +52,8 @@ PrefService* GetActivePrefService() {
} // namespace
// static
void TouchDevicesController::RegisterLocalStatePrefs(
PrefRegistrySimple* registry,
bool for_test) {
// TODO(jamescook|xiyuan): move ownership to ash once user session info could
// distinguish owner and non-owner (http://crbug.com/857103).
if (for_test)
registry->RegisterBooleanPref(prefs::kOwnerTapToClickEnabled, true);
else
registry->RegisterForeignPref(prefs::kOwnerTapToClickEnabled);
}
// static
void TouchDevicesController::RegisterProfilePrefs(PrefRegistrySimple* registry,
bool for_test) {
void TouchDevicesController::RegisterProfilePrefs(
PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(
prefs::kTapDraggingEnabled, false,
user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF |
......@@ -73,16 +61,6 @@ void TouchDevicesController::RegisterProfilePrefs(PrefRegistrySimple* registry,
registry->RegisterBooleanPref(prefs::kTouchpadEnabled, PrefRegistry::PUBLIC);
registry->RegisterBooleanPref(prefs::kTouchscreenEnabled,
PrefRegistry::PUBLIC);
// TODO(jamescook|xiyuan): move ownership to ash once user session info could
// distinguish owner and non-owner (http://crbug.com/857103).
if (for_test) {
registry->RegisterBooleanPref(
prefs::kTapToClickEnabled, true,
user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF);
} else {
registry->RegisterForeignPref(prefs::kTapToClickEnabled);
}
}
TouchDevicesController::TouchDevicesController() {
......@@ -155,8 +133,6 @@ void TouchDevicesController::OnUserSessionAdded(const AccountId& account_id) {
uma_record_callback_ = base::BindOnce([](PrefService* prefs) {
UMA_HISTOGRAM_BOOLEAN("Touchpad.TapDragging.Started",
prefs->GetBoolean(prefs::kTapDraggingEnabled));
UMA_HISTOGRAM_BOOLEAN("Touchpad.TapToClick.Started",
prefs->GetBoolean(prefs::kTapToClickEnabled));
});
}
......@@ -180,10 +156,6 @@ void TouchDevicesController::ObservePrefs(PrefService* prefs) {
prefs::kTapDraggingEnabled,
base::BindRepeating(&TouchDevicesController::UpdateTapDraggingEnabled,
base::Unretained(this)));
pref_change_registrar_->Add(
prefs::kTapToClickEnabled,
base::BindRepeating(&TouchDevicesController::UpdateTapToClickEnabled,
base::Unretained(this)));
pref_change_registrar_->Add(
prefs::kTouchpadEnabled,
base::BindRepeating(&TouchDevicesController::UpdateTouchpadEnabled,
......@@ -194,7 +166,6 @@ void TouchDevicesController::ObservePrefs(PrefService* prefs) {
base::Unretained(this)));
// Load current state.
UpdateTapDraggingEnabled();
UpdateTapToClickEnabled();
UpdateTouchpadEnabled();
UpdateTouchscreenEnabled();
}
......@@ -216,23 +187,6 @@ void TouchDevicesController::UpdateTapDraggingEnabled() {
GetInputDeviceControllerClient()->SetTapDragging(enabled);
}
void TouchDevicesController::UpdateTapToClickEnabled() {
PrefService* prefs = GetActivePrefService();
const bool enabled = prefs->GetBoolean(prefs::kTapToClickEnabled);
if (tap_to_click_enabled_ == enabled)
return;
tap_to_click_enabled_ = enabled;
UMA_HISTOGRAM_BOOLEAN("Touchpad.TapToClick.Changed", enabled);
if (!GetInputDeviceControllerClient())
return; // Happens in tests.
GetInputDeviceControllerClient()->SetTapToClick(enabled);
}
void TouchDevicesController::UpdateTouchpadEnabled() {
if (!GetInputDeviceControllerClient())
return; // Happens in tests.
......
......@@ -33,9 +33,7 @@ class ASH_EXPORT TouchDevicesController : public SessionObserver {
TouchDevicesController();
~TouchDevicesController() override;
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry,
bool for_test);
static void RegisterProfilePrefs(PrefRegistrySimple* registry, bool for_test);
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
// Toggles the status of touchpad between enabled and disabled.
void ToggleTouchpad();
......@@ -57,7 +55,6 @@ class ASH_EXPORT TouchDevicesController : public SessionObserver {
void SetTouchscreenEnabled(bool enabled, TouchDeviceEnabledSource source);
bool tap_dragging_enabled_for_test() { return tap_dragging_enabled_; }
bool tap_to_click_enabled_for_test() { return tap_to_click_enabled_; }
private:
// Overridden from SessionObserver:
......@@ -72,9 +69,6 @@ class ASH_EXPORT TouchDevicesController : public SessionObserver {
// Updates tap dragging enabled state from prefs.
void UpdateTapDraggingEnabled();
// Updates tap to click enabled state from prefs.
void UpdateTapToClickEnabled();
// Updates the actual enabled/disabled status of the touchpad.
void UpdateTouchpadEnabled();
......@@ -89,9 +83,6 @@ class ASH_EXPORT TouchDevicesController : public SessionObserver {
// Saves the tap dragging enabled state from prefs.
bool tap_dragging_enabled_ = false;
// Saves the tap to click enabled state from prefs.
bool tap_to_click_enabled_ = true;
// The touchscreen state which is associated with the global touch device
// enabled source.
bool global_touchscreen_enabled_ = true;
......
......@@ -11,7 +11,6 @@
#include "ash/session/test_session_controller_client.h"
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/test/ash_test_helper.h"
#include "base/command_line.h"
#include "base/test/metrics/histogram_tester.h"
#include "components/prefs/pref_service.h"
......@@ -52,13 +51,6 @@ void SetTapDraggingEnabled(bool enabled) {
prefs->CommitPendingWrite();
}
void SetTapToClickEnabled(bool enabled) {
PrefService* prefs =
Shell::Get()->session_controller()->GetLastActiveUserPrefService();
prefs->SetBoolean(prefs::kTapToClickEnabled, enabled);
prefs->CommitPendingWrite();
}
class TouchDevicesControllerSigninTest : public NoSessionAshTestBase {
public:
TouchDevicesControllerSigninTest() = default;
......@@ -92,16 +84,10 @@ class TouchDevicesControllerSigninTest : public NoSessionAshTestBase {
DISALLOW_COPY_AND_ASSIGN(TouchDevicesControllerSigninTest);
};
TEST_F(TouchDevicesControllerSigninTest, LocalStatePrefsAreRegistered) {
PrefService* prefs = ash_test_helper()->GetLocalStatePrefService();
EXPECT_TRUE(prefs->FindPreference(prefs::kOwnerTapToClickEnabled));
}
TEST_F(TouchDevicesControllerSigninTest, PrefsAreRegistered) {
PrefService* prefs =
Shell::Get()->session_controller()->GetLastActiveUserPrefService();
EXPECT_TRUE(prefs->FindPreference(prefs::kTapDraggingEnabled));
EXPECT_TRUE(prefs->FindPreference(prefs::kTapToClickEnabled));
EXPECT_TRUE(prefs->FindPreference(prefs::kTouchpadEnabled));
EXPECT_TRUE(prefs->FindPreference(prefs::kTouchscreenEnabled));
}
......@@ -122,22 +108,6 @@ TEST_F(TouchDevicesControllerSigninTest, SetTapDraggingEnabled) {
EXPECT_FALSE(controller->tap_dragging_enabled_for_test());
}
TEST_F(TouchDevicesControllerSigninTest, SetTapToClickEnabled) {
auto* controller = Shell::Get()->touch_devices_controller();
ASSERT_TRUE(controller->tap_to_click_enabled_for_test());
SetTapToClickEnabled(false);
EXPECT_FALSE(controller->tap_to_click_enabled_for_test());
// Switch to user 2 and switch back.
SwitchActiveUser(kUser2Email);
EXPECT_TRUE(controller->tap_to_click_enabled_for_test());
SwitchActiveUser(kUser1Email);
EXPECT_FALSE(controller->tap_to_click_enabled_for_test());
SetTapToClickEnabled(true);
EXPECT_TRUE(controller->tap_to_click_enabled_for_test());
}
// Tests that touchpad enabled user pref works properly under debug accelerator.
TEST_F(TouchDevicesControllerSigninTest, ToggleTouchpad) {
ASSERT_TRUE(GetUserPrefTouchpadEnabled());
......
......@@ -9,7 +9,6 @@
#include "ash/accessibility/focus_ring_controller.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "ash/system/tray/system_tray.h"
......@@ -1163,11 +1162,8 @@ void ShowLoginWizard(OobeScreen first_screen) {
// login screen.
system::InputDeviceSettings::Get()->SetPrimaryButtonRight(
prefs->GetBoolean(prefs::kOwnerPrimaryMouseButtonRight));
// TODO(jamescook): move to ash in OnLocalStatePrefServiceInitialized() once
// user session info could distinguish between owner and non-owner
// (http://crbug.com/857103).
system::InputDeviceSettings::Get()->SetTapToClick(
prefs->GetBoolean(ash::prefs::kOwnerTapToClickEnabled));
prefs->GetBoolean(prefs::kOwnerTapToClickEnabled));
}
system::InputDeviceSettings::Get()->SetNaturalScroll(
base::CommandLine::ForCurrentProcess()->HasSwitch(
......
......@@ -160,9 +160,8 @@ Preferences::~Preferences() {
// static
void Preferences::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(prefs::kOwnerPrimaryMouseButtonRight, false);
registry->RegisterBooleanPref(prefs::kOwnerTapToClickEnabled, true);
// TODO(jamescook): Move ownership and registration into ash.
registry->RegisterBooleanPref(ash::prefs::kOwnerTapToClickEnabled, true,
PrefRegistry::PUBLIC);
registry->RegisterStringPref(prefs::kLogoutStartedLast, std::string());
registry->RegisterStringPref(prefs::kSigninScreenTimezone, std::string());
registry->RegisterBooleanPref(prefs::kResolveDeviceTimezoneByGeolocation,
......@@ -199,9 +198,9 @@ void Preferences::RegisterProfilePrefs(
registry->RegisterBooleanPref(prefs::kPerformanceTracingEnabled, false);
registry->RegisterBooleanPref(
ash::prefs::kTapToClickEnabled, true,
user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF |
PrefRegistry::PUBLIC);
prefs::kTapToClickEnabled,
true,
user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF);
registry->RegisterBooleanPref(prefs::kEnableTouchpadThreeFingerClick, false);
// This preference can only be set to true by policy or command_line flag
// and it should not carry over to sessions were neither of these is set.
......@@ -518,7 +517,7 @@ void Preferences::InitUserPrefs(sync_preferences::PrefServiceSyncable* prefs) {
performance_tracing_enabled_.Init(prefs::kPerformanceTracingEnabled,
prefs, callback);
tap_to_click_enabled_.Init(ash::prefs::kTapToClickEnabled, prefs, callback);
tap_to_click_enabled_.Init(prefs::kTapToClickEnabled, prefs, callback);
three_finger_click_enabled_.Init(prefs::kEnableTouchpadThreeFingerClick,
prefs, callback);
unified_desktop_enabled_by_default_.Init(
......@@ -660,16 +659,20 @@ void Preferences::ApplyPreferences(ApplyReason reason,
tracing_manager_.reset();
SystemTrayClient::Get()->SetPerformanceTracingIconVisible(enabled);
}
if (reason != REASON_PREF_CHANGED ||
pref_name == ash::prefs::kTapToClickEnabled) {
if (reason != REASON_PREF_CHANGED || pref_name == prefs::kTapToClickEnabled) {
const bool enabled = tap_to_click_enabled_.GetValue();
if (user_is_active)
touchpad_settings.SetTapToClick(enabled);
if (reason == REASON_PREF_CHANGED)
UMA_HISTOGRAM_BOOLEAN("Touchpad.TapToClick.Changed", enabled);
else if (reason == REASON_INITIALIZATION)
UMA_HISTOGRAM_BOOLEAN("Touchpad.TapToClick.Started", enabled);
// Save owner preference in local state to use on login screen.
// TODO(jamescook): move to ash once user session info could distinguish
// between owner and non-owner (http://crbug.com/857103).
if (user_is_owner) {
PrefService* prefs = g_browser_process->local_state();
if (prefs->GetBoolean(ash::prefs::kOwnerTapToClickEnabled) != enabled)
prefs->SetBoolean(ash::prefs::kOwnerTapToClickEnabled, enabled);
if (prefs->GetBoolean(prefs::kOwnerTapToClickEnabled) != enabled)
prefs->SetBoolean(prefs::kOwnerTapToClickEnabled, enabled);
}
}
if (reason != REASON_PREF_CHANGED ||
......
......@@ -5,7 +5,6 @@
#include <stddef.h>
#include <sys/types.h>
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/cpp/ash_switches.h"
#include "base/command_line.h"
#include "base/macros.h"
......@@ -65,6 +64,7 @@ class PreferencesTest : public LoginManagerTest {
// |variant| value. For opposite |variant| values all preferences receive
// different values.
void SetPrefs(PrefService* prefs, bool variant) {
prefs->SetBoolean(prefs::kTapToClickEnabled, variant);
prefs->SetBoolean(prefs::kPrimaryMouseButtonRight, !variant);
prefs->SetBoolean(prefs::kMouseReverseScroll, variant);
prefs->SetBoolean(prefs::kEnableTouchpadThreeFingerClick, !variant);
......@@ -80,6 +80,8 @@ class PreferencesTest : public LoginManagerTest {
}
void CheckSettingsCorrespondToPrefs(PrefService* prefs) {
EXPECT_EQ(prefs->GetBoolean(prefs::kTapToClickEnabled),
input_settings_->current_touchpad_settings().GetTapToClick());
EXPECT_EQ(prefs->GetBoolean(prefs::kPrimaryMouseButtonRight),
input_settings_->current_mouse_settings()
.GetPrimaryButtonRight());
......@@ -108,8 +110,8 @@ class PreferencesTest : public LoginManagerTest {
void CheckLocalStateCorrespondsToPrefs(PrefService* prefs) {
PrefService* local_state = g_browser_process->local_state();
EXPECT_EQ(local_state->GetBoolean(ash::prefs::kOwnerTapToClickEnabled),
prefs->GetBoolean(ash::prefs::kTapToClickEnabled));
EXPECT_EQ(local_state->GetBoolean(prefs::kOwnerTapToClickEnabled),
prefs->GetBoolean(prefs::kTapToClickEnabled));
EXPECT_EQ(local_state->GetBoolean(prefs::kOwnerPrimaryMouseButtonRight),
prefs->GetBoolean(prefs::kPrimaryMouseButtonRight));
}
......@@ -201,11 +203,11 @@ IN_PROC_BROWSER_TEST_F(PreferencesTestForceWebUiLogin, MultiProfiles) {
// state prefs and vice versa.
EXPECT_EQ(user_manager->GetOwnerAccountId(), test_users_[0]);
CheckLocalStateCorrespondsToPrefs(prefs1);
prefs2->SetBoolean(ash::prefs::kTapToClickEnabled,
!prefs1->GetBoolean(ash::prefs::kTapToClickEnabled));
prefs2->SetBoolean(prefs::kTapToClickEnabled,
!prefs1->GetBoolean(prefs::kTapToClickEnabled));
CheckLocalStateCorrespondsToPrefs(prefs1);
prefs1->SetBoolean(ash::prefs::kTapToClickEnabled,
!prefs1->GetBoolean(ash::prefs::kTapToClickEnabled));
prefs1->SetBoolean(prefs::kTapToClickEnabled,
!prefs1->GetBoolean(prefs::kTapToClickEnabled));
CheckLocalStateCorrespondsToPrefs(prefs1);
// Switch user back.
......
......@@ -443,7 +443,7 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetWhitelistedKeys() {
settings_api::PrefType::PREF_TYPE_LIST;
// Device settings.
(*s_whitelist)[ash::prefs::kTapToClickEnabled] =
(*s_whitelist)[::prefs::kTapToClickEnabled] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_whitelist)[::prefs::kNaturalScroll] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
......
......@@ -475,6 +475,9 @@ const char kDefaultAppsInstallState[] = "default_apps_install_state";
const char kHideWebStoreIcon[] = "hide_web_store_icon";
#if defined(OS_CHROMEOS)
// A boolean pref set to true if touchpad tap-to-click is enabled.
const char kTapToClickEnabled[] = "settings.touchpad.enable_tap_to_click";
// A boolean pref set to true if touchpad three-finger-click is enabled.
const char kEnableTouchpadThreeFingerClick[] =
"settings.touchpad.enable_three_finger_click";
......@@ -1801,6 +1804,9 @@ const char kExternalStorageReadOnly[] = "hardware.external_storage_read_only";
// Copy of owner swap mouse buttons option to use on login screen.
const char kOwnerPrimaryMouseButtonRight[] = "owner.mouse.primary_right";
// Copy of owner tap-to-click option to use on login screen.
const char kOwnerTapToClickEnabled[] = "owner.touchpad.enable_tap_to_click";
// The length of device uptime after which an automatic reboot is scheduled,
// expressed in seconds.
const char kUptimeLimit[] = "automatic_reboot.uptime_limit";
......
......@@ -186,6 +186,7 @@ extern const char kNetworkPredictionOptions[];
extern const char kDefaultAppsInstallState[];
extern const char kHideWebStoreIcon[];
#if defined(OS_CHROMEOS)
extern const char kTapToClickEnabled[];
extern const char kEnableTouchpadThreeFingerClick[];
extern const char kNaturalScroll[];
extern const char kPrimaryMouseButtonRight[];
......@@ -608,6 +609,7 @@ extern const char kUserActivityTimes[];
extern const char kExternalStorageDisabled[];
extern const char kExternalStorageReadOnly[];
extern const char kOwnerPrimaryMouseButtonRight[];
extern const char kOwnerTapToClickEnabled[];
extern const char kUptimeLimit[];
extern const char kRebootAfterUpdate[];
extern const char kDeviceRobotAnyApiRefreshToken[];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment