Clean up wake on wifi handling

chrome/browser/chromeos/preferences.cc

Index: chrome/browser/chromeos/preferences.cc
diff --git a/chrome/browser/chromeos/preferences.cc b/chrome/browser/chromeos/preferences.cc
index c95ddba71b002c5e2f785f36c49d228afe266f88..a3e3cf8f238ff3ba95ca1d810240388b7cf28e8b 100644
--- a/chrome/browser/chromeos/preferences.cc
+++ b/chrome/browser/chromeos/preferences.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/chromeos/input_method/input_method_util.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
+#include "chrome/browser/chromeos/net/wake_on_wifi_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/system/input_device_settings.h"
 #include "chrome/browser/download/download_prefs.h"
@@ -274,6 +275,21 @@ void Preferences::RegisterProfilePrefs(
       language_prefs::kXkbAutoRepeatIntervalInMs,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 
+  // We don't sync wake-on-wifi related prefs because they are device specific.
+  // TODO(chirantan): Default this to on when we are ready to launch.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kWakeOnPackets)) {
+    registry->RegisterIntegerPref(
+        prefs::kWakeOnWiFiEnabled,
+        WakeOnWifiManager::WAKE_ON_PACKET_AND_SSID,
+        user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+  } else {
+    registry->RegisterIntegerPref(
+        prefs::kWakeOnWiFiEnabled,
+        WakeOnWifiManager::WAKE_ON_NONE,
+        user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+  }
+
   // Mobile plan notifications default to on.
   registry->RegisterBooleanPref(
       prefs::kShowPlanNotifications,
@@ -348,6 +364,8 @@ void Preferences::InitUserPrefs(PrefServiceSyncable* prefs) {
       prefs::kLanguageXkbAutoRepeatDelay, prefs, callback);
   xkb_auto_repeat_interval_pref_.Init(
       prefs::kLanguageXkbAutoRepeatInterval, prefs, callback);
+
+  wake_on_wifi_enabled_.Init(prefs::kWakeOnWiFiEnabled, prefs, callback);
 }
 
 void Preferences::Init(Profile* profile, const user_manager::User* user) {
@@ -553,6 +571,13 @@ void Preferences::ApplyPreferences(ApplyReason reason,
       UpdateAutoRepeatRate();
   }
 
+  if (reason != REASON_PREF_CHANGED ||

[michaelpg, 2014/11/13 03:18:48]

Seems like we need to clarify how the machine setting/owner-controlled
preference works.

This CL makes kWakeOnWiFiEnabled a per-profile preference that only the owner
can set -- that's kind of weird. For example, when a non-owner signs in, the
initialization here will use the default value (e.g., PACKET_AND_SSID) because
the check on 575 means a non-owner can never set his preference. That doesn't
seem right, since we're not respecting the owner's preference OR the user's
preference.

Since this isn't a profile setting but a per-machine (owner) setting, we
probably don't want to add this preference to the user's pref store, but to the
local state. (That said, I'm not convinced this shouldn't be a per-user
decision, like the sleep settings on Windows.)

Would it make more sense to register the pref in WakeOnWifiManager so we don't
need to involve it in the Preferences class?

You could then observe this pref directly by using
WakeOnWifiManager::OnPreferenceChanged as the callback when initing the pref in
WakeOnWifiManager. You could still check that the user is the owner, but you
would be setting the local state preference, not the current user's preference.

The question then is what to do about a managed device (it has no owner). Stick
with the default? make this a policy setting? allow any user to change the
setting?

[Chirantan Ekbote, 2014/11/13 21:32:14]

So I discussed this again with Sameer and I think you're right.  This makes more
sense as a per-user thing rather than an owner thing, especially considering the
managed device case.  I'm going to run it by drewry@, who was the one who had
initially asked for the flag to be owner only but I'm going to go ahead right
now with the assumption that making it per-user is ok.
I'd like to do that as well but I was unsure about how to plumb it in so that
the WakeOnWifiManager will get access to the PrefRegistry and the PrefService
for the user.

[stevenjb, 2014/11/13 22:20:17]

I am not entirely up to speed on how our pref store works, but essencially we
want to associate WakeOnWifiManager with the primary user profile only, and use
the preference in that user's pref store.

I believe that there should always be a "primary user profile" even on managed /
kisok devices.

[Chirantan Ekbote, 2014/11/13 23:12:37]

We can change this to

    if (user_is_primary_ && (reason != REASON_PREF_CHANGED ||
                             pref_name == prefs::kWakeOnWifiEnabled))

which I think covers all the cases.

+      (pref_name == prefs::kWakeOnWiFiEnabled && user_is_owner)) {
+    WakeOnWifiManager::Get()->OnPreferenceChanged(
+        static_cast<WakeOnWifiManager::WakeOnWifiFeature>(
+            wake_on_wifi_enabled_.GetValue()));
+  }
+
   if (reason == REASON_INITIALIZATION)
     SetInputMethodList();