chromeos: Move default power management policy into Chrome.

chromeos/dbus/power_policy_controller.h

 // Copyright (c) 2013 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.
 
 #ifndef CHROMEOS_DBUS_POWER_POLICY_CONTROLLER_H_
 #define CHROMEOS_DBUS_POWER_POLICY_CONTROLLER_H_
 
 #include <map>
 #include <string>
 
 #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"
 #include "chromeos/dbus/power_manager/policy.pb.h"
 #include "chromeos/dbus/power_manager_client.h"
 
 namespace chromeos {
 
 class DBusThreadManager;
 
 // PowerPolicyController is responsible for sending Chrome's assorted power
 // management preferences to the Chrome OS power manager.
 class CHROMEOS_EXPORT PowerPolicyController
     : public DBusThreadManagerObserver,
       public PowerManagerClient::Observer {
  public:
   // Note: Do not change these values; they are used by preferences.
   enum Action {
     ACTION_SUSPEND      = 0,
     ACTION_STOP_SESSION = 1,
     ACTION_SHUT_DOWN    = 2,
     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
   // inactivity.  Returns a unique ID that can be passed to RemoveBlock()
   // later.
   int AddScreenBlock(const std::string& reason);
   int AddSuspendBlock(const std::string& reason);
 
   // Unregisters a request previously created via AddScreenBlock() or
   // AddSuspendBlock().
   void RemoveBlock(int id);
 
   // DBusThreadManagerObserver implementation:
   virtual void OnDBusThreadManagerDestroying(DBusThreadManager* manager)
       OVERRIDE;
 
   // PowerManagerClient::Observer implementation:
   virtual void PowerManagerRestarted() OVERRIDE;
 
  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();
 
   // Sends an empty policy to the power manager to reset its configuration.
   void SendEmptyPolicy();
 
   DBusThreadManager* manager_;  // not owned
   PowerManagerClient* client_;  // not owned
 
-  // Policy specified by the prefs that were last passed to
-  // UpdatePolicyFromPrefs().
-  power_manager::PowerManagementPolicy prefs_policy_;
+  // 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?

 
-  // Are one or more fields set in |prefs_policy_|?
-  bool prefs_were_set_;
+  // 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
   // to the reason for the request.
   BlockMap screen_blocks_;
   BlockMap suspend_blocks_;
 
   // 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);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_POWER_POLICY_CONTROLLER_H_