Enabled/disable touch screen in TabletPowerButtonController

chrome/browser/chromeos/system/input_device_settings.cc

 // Copyright (c) 2012 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 "chrome/browser/chromeos/system/input_device_settings.h"
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ skipping 216 matching lines @@
   bool keyboard_driven = false;
   if (chromeos::system::StatisticsProvider::GetInstance()->GetMachineFlag(
           kOemKeyboardDrivenOobeKey, &keyboard_driven)) {
     return keyboard_driven;
   }
 
   return false;
 }
 
 // static
+void InputDeviceSettings::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(::prefs::kLocalTouchScreenEnabled, true);
+}
+
+// static
 void InputDeviceSettings::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(::prefs::kTouchScreenEnabled, true);
   registry->RegisterBooleanPref(::prefs::kTouchPadEnabled, true);
 }
 
-void InputDeviceSettings::UpdateTouchDevicesStatusFromActiveProfilePrefs() {
+void InputDeviceSettings::UpdateTouchDevicesStatusFromPrefs() {
+  UpdateTouchscreenStatusFromPrefs();
+
   PrefService* user_prefs = GetActiveProfilePrefs();
   if (!user_prefs)
     return;
 
-  const bool touch_screen_status =
-      user_prefs->HasPrefPath(::prefs::kTouchScreenEnabled)
-          ? user_prefs->GetBoolean(::prefs::kTouchScreenEnabled)
-          : true;
-
   const bool touch_pad_status =
       user_prefs->HasPrefPath(::prefs::kTouchPadEnabled)
           ? user_prefs->GetBoolean(::prefs::kTouchPadEnabled)
           : true;
-
-  SetTouchscreensEnabled(touch_screen_status);
   SetInternalTouchpadEnabled(touch_pad_status);
 }
 
-void InputDeviceSettings::ToggleTouchscreen() {
-  PrefService* user_prefs = GetActiveProfilePrefs();
-  if (!user_prefs)
-    return;
+bool InputDeviceSettings::IsTouchscreenEnabledInPrefs(bool is_local) const {
+  if (is_local) {
+    PrefService* local_state = g_browser_process->local_state();
+    DCHECK(local_state);
 
-  const bool touch_screen_status =
-      user_prefs->HasPrefPath(::prefs::kTouchScreenEnabled)
-          ? user_prefs->GetBoolean(::prefs::kTouchScreenEnabled)
-          : true;
+    return local_state->HasPrefPath(::prefs::kLocalTouchScreenEnabled)
+               ? local_state->GetBoolean(::prefs::kLocalTouchScreenEnabled)
+               : true;
+  } else {
+    PrefService* user_prefs = GetActiveProfilePrefs();
+    if (!user_prefs)
+      return true;
 
-  user_prefs->SetBoolean(::prefs::kTouchScreenEnabled, !touch_screen_status);
-  SetTouchscreensEnabled(!touch_screen_status);
+    return user_prefs->HasPrefPath(::prefs::kTouchScreenEnabled)
+               ? user_prefs->GetBoolean(::prefs::kTouchScreenEnabled)
+               : true;
+  }
+}
+
+void InputDeviceSettings::SetTouchscreenEnabledInPrefs(bool enabled,
+                                                       bool is_local) {
+  if (is_local) {
+    PrefService* local_state = g_browser_process->local_state();
+    DCHECK(local_state);
+    local_state->SetBoolean(::prefs::kLocalTouchScreenEnabled, enabled);
+  } else {
+    PrefService* user_prefs = GetActiveProfilePrefs();
+    if (!user_prefs)

[Daniel Erat, 2016/12/02 21:48:56]

is this expected to happen or should a warning/error be logged?

[Qiang(Joe) Xu, 2016/12/02 23:55:11]

I take a look at this:
https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager....

See Line 425 and line 443 (and line 448 has a CHECK). So only Line 425 could
introduce nullptr in the call chain I think. I don't know if we need a
LOG(WARNING) here, maybe in line 425 there?

+      return;
+    user_prefs->SetBoolean(::prefs::kTouchScreenEnabled, enabled);
+  }
+}
+
+void InputDeviceSettings::UpdateTouchscreenStatusFromPrefs() {
+  SetTouchscreensEnabled(IsTouchscreenEnabledInPrefs(true) ==

[Daniel Erat, 2016/12/02 21:48:56]

this is a bit tricky, so i'd probably write it out as:

  bool enabled_in_local_state = IsTouchscreenEnabledInPrefs(true);
  bool enabled_in_user_prefs = IsTouchscreenEnabledInPrefs(false);
  SetTouchscreensEnabled(enabled_in_local_state == enabled_in_user_prefs);

but i actually don't understand this statement. we enable the touchscreen if
it's enabled in both places but also if it's disabled in both places?

i see that there was earlier discussion abou tthis, but i think it'd be
worthwhile to figure it out now (and add a comment if this is correct, because
it looks strange just from reading the code).

[afakhry, 2016/12/02 22:33:20]

I discussed with him offline. This should be an AND, since we're just updating
the status here from both local_state and prefs. Toggling the status as a result
of the shortcut is already done in debug_commands.cc.

[Qiang(Joe) Xu, 2016/12/02 23:55:11]

Done.

+                         IsTouchscreenEnabledInPrefs(false));
 }
 
 void InputDeviceSettings::ToggleTouchpad() {
   PrefService* user_prefs = GetActiveProfilePrefs();
   if (!user_prefs)
     return;
 
   const bool touch_pad_status =
       user_prefs->HasPrefPath(::prefs::kTouchPadEnabled)
           ? user_prefs->GetBoolean(::prefs::kTouchPadEnabled)
           : true;
 
   user_prefs->SetBoolean(::prefs::kTouchPadEnabled, !touch_pad_status);
   SetInternalTouchpadEnabled(!touch_pad_status);
 }
 
 }  // namespace system
 }  // namespace chromeos