chromeos: Move default power management policy into Chrome.

chromeos/dbus/power_policy_controller.h

Index: chromeos/dbus/power_policy_controller.h
diff --git a/chromeos/dbus/power_policy_controller.h b/chromeos/dbus/power_policy_controller.h
index 3d45769b3e8384e90f2b00b829d835466481ea69..602a8178cba6dce49b1f33a74dcd69ef1b4a502d 100644
--- a/chromeos/dbus/power_policy_controller.h
+++ b/chromeos/dbus/power_policy_controller.h
@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/prefs/pref_member.h"
 #include "base/prefs/pref_service.h"

[bartfab (slow), 2013/04/17 11:51:12]

Nit: It would be sufficient to forward-declare PolicyService here and do the
include in the implementation file.

[Daniel Erat, 2013/04/18 03:59:16]

n/a

 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_thread_manager_observer.h"
@@ -34,27 +35,32 @@ class CHROMEOS_EXPORT PowerPolicyController
     ACTION_DO_NOTHING   = 3,
   };
 
+  // Names of various power-management-related preferences.  Used to
+  // initialize |*_pref_| members.
+  struct PrefNames {
+    std::string ac_screen_dim_delay_ms;
+    std::string ac_screen_off_delay_ms;
+    std::string ac_screen_lock_delay_ms;
+    std::string ac_idle_warning_delay_ms;
+    std::string ac_idle_delay_ms;
+    std::string battery_screen_dim_delay_ms;
+    std::string battery_screen_off_delay_ms;
+    std::string battery_screen_lock_delay_ms;
+    std::string battery_idle_warning_delay_ms;
+    std::string battery_idle_delay_ms;
+    std::string idle_action;
+    std::string lid_closed_action;
+    std::string use_audio_activity;
+    std::string use_video_activity;
+    std::string enable_screen_lock;
+    std::string presentation_idle_delay_factor;
+  };
+
   PowerPolicyController(DBusThreadManager* manager, PowerManagerClient* client);
   virtual ~PowerPolicyController();
 
-  // Sends an updated policy to the power manager based on the current
-  // values of the passed-in prefs.
-  void UpdatePolicyFromPrefs(
-      const PrefService::Preference& ac_screen_dim_delay_ms_pref,
-      const PrefService::Preference& ac_screen_off_delay_ms_pref,
-      const PrefService::Preference& ac_screen_lock_delay_ms_pref,
-      const PrefService::Preference& ac_idle_warning_delay_ms_pref,
-      const PrefService::Preference& ac_idle_delay_ms_pref,
-      const PrefService::Preference& battery_screen_dim_delay_ms_pref,
-      const PrefService::Preference& battery_screen_off_delay_ms_pref,
-      const PrefService::Preference& battery_screen_lock_delay_ms_pref,
-      const PrefService::Preference& battery_idle_warning_delay_ms_pref,
-      const PrefService::Preference& battery_idle_delay_ms_pref,
-      const PrefService::Preference& idle_action_pref,
-      const PrefService::Preference& lid_closed_action_pref,
-      const PrefService::Preference& use_audio_activity_pref,
-      const PrefService::Preference& use_video_activity_pref,
-      const PrefService::Preference& presentation_idle_delay_factor_pref);
+  // Initializes |*_prefs_| members.
+  void InitPrefs(PrefService* pref_service, const PrefNames& pref_names);
 
   // Registers a request to temporarily prevent the screen from getting
   // dimmed or turned off or the system from suspending in response to user
@@ -77,6 +83,9 @@ class CHROMEOS_EXPORT PowerPolicyController
  private:
   typedef std::map<int, std::string> BlockMap;
 
+  // Called when |*_pref_| members' values change.
+  void OnPrefChanged(const std::string& pref_name);
+
   // Sends a policy based on |prefs_policy_| to the power manager.
   void SendCurrentPolicy();
 
@@ -86,12 +95,27 @@ class CHROMEOS_EXPORT PowerPolicyController
   DBusThreadManager* manager_;  // not owned
   PowerManagerClient* client_;  // not owned
 
-  // Policy specified by the prefs that were last passed to
-  // UpdatePolicyFromPrefs().
-  power_manager::PowerManagementPolicy prefs_policy_;
-
-  // Are one or more fields set in |prefs_policy_|?
-  bool prefs_were_set_;
+  // NULL until InitPrefs() has been called.
+  PrefService* pref_service_;
+  PrefNames pref_names_;

[bartfab (slow), 2013/04/17 11:51:12]

I don't think managing the PrefMembers in this class is a good idea. Passing
pref names from chrome/ to chromeos/dbus/ seems awkward. Also, the PrefNames 
struct ends up occupying a good chunk of memory to store copies of pref names.

[Daniel Erat, 2013/04/17 13:52:13]

What's your preference for how this should work instead?  Should the PrefMembers
remain in chromeos::Preferences, this struct instead contain pref values (e.g.
int, bool, etc.), and it get passed to a HandleUpdatedPreferences() method?

+
+  // Used to watch for changes to prefs.
+  IntegerPrefMember ac_screen_dim_delay_ms_pref_;
+  IntegerPrefMember ac_screen_off_delay_ms_pref_;
+  IntegerPrefMember ac_screen_lock_delay_ms_pref_;
+  IntegerPrefMember ac_idle_warning_delay_ms_pref_;
+  IntegerPrefMember ac_idle_delay_ms_pref_;
+  IntegerPrefMember battery_screen_dim_delay_ms_pref_;
+  IntegerPrefMember battery_screen_off_delay_ms_pref_;
+  IntegerPrefMember battery_screen_lock_delay_ms_pref_;
+  IntegerPrefMember battery_idle_warning_delay_ms_pref_;
+  IntegerPrefMember battery_idle_delay_ms_pref_;
+  IntegerPrefMember idle_action_pref_;
+  IntegerPrefMember lid_closed_action_pref_;
+  BooleanPrefMember use_audio_activity_pref_;
+  BooleanPrefMember use_video_activity_pref_;
+  BooleanPrefMember enable_screen_lock_pref_;
+  DoublePrefMember presentation_idle_delay_factor_pref_;
 
   // Maps from an ID representing a request to prevent the screen from
   // getting dimmed or turned off or to prevent the system from suspending
@@ -102,6 +126,8 @@ class CHROMEOS_EXPORT PowerPolicyController
   // Next ID to be used by AddScreenBlock() or AddSuspendBlock().
   int next_block_id_;
 
+  base::WeakPtrFactory<PowerPolicyController> weak_ptr_factory_;

[bartfab (slow), 2013/04/17 11:51:12]

Nit: Move the #include "base/memory/weak_ptr.h" from the implementation file to
the header file.

[Daniel Erat, 2013/04/18 03:59:16]

n/a

+
   DISALLOW_COPY_AND_ASSIGN(PowerPolicyController);
 };