Revert of https://codereview.chromium.org/129193002/

Reason for revert: Did not fix chromeos failures

TBR=bartfab@chromium.org,benwells@chromium.org,derat@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=329930

Review URL: https://codereview.chromium.org/129303003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243671 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 49 insertions, 68 deletions

diff --git a/chromeos/dbus/fake_power_manager_client.cc b/chromeos/dbus/fake_power_manager_client.cc
index 81ca360..6007d753 100644
--- a/chromeos/dbus/fake_power_manager_client.cc
+++ b/chromeos/dbus/fake_power_manager_client.cc
@@ -8,7 +8,8 @@
 namespace chromeos {
 
 FakePowerManagerClient::FakePowerManagerClient()
-    : request_restart_call_count_(0) {
+    : num_request_restart_calls_(0),
+      num_set_policy_calls_(0) {
 }
 
 FakePowerManagerClient::~FakePowerManagerClient() {
@@ -31,6 +32,7 @@ void FakePowerManagerClient::RequestStatusUpdate() {
 void FakePowerManagerClient::SetPolicy(
     const power_manager::PowerManagementPolicy& policy) {
   policy_ = policy;
+  ++num_set_policy_calls_;
 }
 
 void FakePowerManagerClient::RequestShutdown() {
@@ -60,7 +62,7 @@ bool FakePowerManagerClient::HasObserver(Observer* observer) {
 }
 
 void FakePowerManagerClient::RequestRestart() {
-  ++request_restart_call_count_;
+  ++num_request_restart_calls_;
 }
 
 void FakePowerManagerClient::IncreaseKeyboardBrightness() {
diff --git a/chromeos/dbus/fake_power_manager_client.h b/chromeos/dbus/fake_power_manager_client.h
index ae1adc5..61ad673 100644
--- a/chromeos/dbus/fake_power_manager_client.h
+++ b/chromeos/dbus/fake_power_manager_client.h
@@ -24,6 +24,14 @@ class FakePowerManagerClient : public PowerManagerClient {
   FakePowerManagerClient();
   virtual ~FakePowerManagerClient();
 
+  power_manager::PowerManagementPolicy& policy() { return policy_; }
+  int num_request_restart_calls() const {
+    return num_request_restart_calls_;
+  }
+  int num_set_policy_calls() const {
+    return num_set_policy_calls_;
+  }
+
   // PowerManagerClient overrides
   virtual void Init(dbus::Bus* bus) OVERRIDE;
   virtual void AddObserver(Observer* observer) OVERRIDE;
@@ -49,13 +57,6 @@ class FakePowerManagerClient : public PowerManagerClient {
   virtual base::Closure GetSuspendReadinessCallback() OVERRIDE;
   virtual int GetNumPendingSuspendReadinessCallbacks() OVERRIDE;
 
-  power_manager::PowerManagementPolicy& get_policy() { return policy_; }
-
-  // Returns how many times RequestRestart() was called.
-  int request_restart_call_count() const {
-    return request_restart_call_count_;
-  }
-
   // Emulates that the dbus server sends a message "SuspendImminent" to the
   // client.
   void SendSuspendImminent();
@@ -66,10 +67,19 @@ class FakePowerManagerClient : public PowerManagerClient {
       const power_manager::SuspendState& suspend_state);
 
  private:
+  ObserverList<Observer> observers_;
+
+  // Last policy passed to SetPolicy().
   power_manager::PowerManagementPolicy policy_;
+
+  // Last time passed to a SUSPEND_TO_MEMORY call to SendSuspendStateChanged().
   base::Time last_suspend_wall_time_;
-  ObserverList<Observer> observers_;
-  int request_restart_call_count_;
+
+  // Number of times that RequestRestart() has been called.
+  int num_request_restart_calls_;
+
+  // Number of times that SetPolicy() has been called.
+  int num_set_policy_calls_;
 
   DISALLOW_COPY_AND_ASSIGN(FakePowerManagerClient);
 };
diff --git a/chromeos/dbus/power_policy_controller.cc b/chromeos/dbus/power_policy_controller.cc
index e7888ee..6cd696f 100644
--- a/chromeos/dbus/power_policy_controller.cc
+++ b/chromeos/dbus/power_policy_controller.cc
@@ -134,31 +134,22 @@ std::string PowerPolicyController::GetPolicyDebugString(
 }
 
 PowerPolicyController::PowerPolicyController()
-    : manager_(NULL),
-      client_(NULL),
+    : client_(NULL),
       prefs_were_set_(false),
       honor_screen_wake_locks_(true),
       next_wake_lock_id_(1) {
 }
 
 PowerPolicyController::~PowerPolicyController() {
-  DCHECK(manager_);
-  // The power manager's policy is reset before this point, in
-  // OnDBusThreadManagerDestroying().  At the time that
-  // PowerPolicyController is destroyed, PowerManagerClient's D-Bus proxy
-  // to the power manager is already gone.
-  client_->RemoveObserver(this);
-  client_ = NULL;
-  manager_->RemoveObserver(this);
-  manager_ = NULL;
+  if (client_) {
+    client_->RemoveObserver(this);
+    client_ = NULL;
+  }
 }
 
 void PowerPolicyController::Init(DBusThreadManager* manager) {
-  manager_ = manager;
-  manager_->AddObserver(this);
-  client_ = manager_->GetPowerManagerClient();
+  client_ = manager->GetPowerManagerClient();
   client_->AddObserver(this);
-  SendCurrentPolicy();
 }
 
 void PowerPolicyController::ApplyPrefs(const PrefValues& values) {
@@ -220,13 +211,6 @@ void PowerPolicyController::ApplyPrefs(const PrefValues& values) {
   SendCurrentPolicy();
 }
 
-void PowerPolicyController::ClearPrefs() {
-  prefs_policy_.Clear();
-  honor_screen_wake_locks_ = true;
-  prefs_were_set_ = false;
-  SendCurrentPolicy();
-}
-
 int PowerPolicyController::AddScreenWakeLock(const std::string& reason) {
   int id = next_wake_lock_id_++;
   screen_wake_locks_[id] = reason;
@@ -248,12 +232,6 @@ void PowerPolicyController::RemoveWakeLock(int id) {
     SendCurrentPolicy();
 }
 
-void PowerPolicyController::OnDBusThreadManagerDestroying(
-    DBusThreadManager* manager) {
-  DCHECK_EQ(manager, manager_);
-  SendEmptyPolicy();
-}
-
 void PowerPolicyController::PowerManagerRestarted() {
   SendCurrentPolicy();
 }
@@ -301,8 +279,4 @@ void PowerPolicyController::SendCurrentPolicy() {
   client_->SetPolicy(policy);
 }
 
-void PowerPolicyController::SendEmptyPolicy() {
-  client_->SetPolicy(power_manager::PowerManagementPolicy());
-}
-
 }  // namespace chromeos
diff --git a/chromeos/dbus/power_policy_controller.h b/chromeos/dbus/power_policy_controller.h
index 7db9091..cb994cd 100644
--- a/chromeos/dbus/power_policy_controller.h
+++ b/chromeos/dbus/power_policy_controller.h
@@ -11,7 +11,6 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #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"
 
@@ -22,8 +21,7 @@ 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 PowerManagerClient::Observer {
  public:
   // Note: Do not change these values; they are used by preferences.
   enum Action {
@@ -79,9 +77,6 @@ class CHROMEOS_EXPORT PowerPolicyController
   // Updates |prefs_policy_| with |values| and sends an updated policy.
   void ApplyPrefs(const PrefValues& values);
 
-  // Resets |prefs_policy_| to its defaults and sends an updated policy.
-  void ClearPrefs();
-
   // Registers a request to temporarily prevent the screen from getting
   // dimmed or turned off or the system from suspending in response to user
   // inactivity and sends an updated policy.  |reason| is a human-readable
@@ -94,10 +89,6 @@ class CHROMEOS_EXPORT PowerPolicyController
   // AddSystemWakeLock() and sends an updated policy.
   void RemoveWakeLock(int id);
 
-  // DBusThreadManagerObserver implementation:
-  virtual void OnDBusThreadManagerDestroying(DBusThreadManager* manager)
-      OVERRIDE;
-
   // PowerManagerClient::Observer implementation:
   virtual void PowerManagerRestarted() OVERRIDE;
 
@@ -109,11 +100,7 @@ class CHROMEOS_EXPORT PowerPolicyController
   // 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
+  PowerManagerClient* client_;  // weak
 
   // Policy derived from values passed to ApplyPrefs().
   power_manager::PowerManagementPolicy prefs_policy_;
diff --git a/chromeos/dbus/power_policy_controller_unittest.cc b/chromeos/dbus/power_policy_controller_unittest.cc
index 513dd38..a2c1eb6 100644
--- a/chromeos/dbus/power_policy_controller_unittest.cc
+++ b/chromeos/dbus/power_policy_controller_unittest.cc
@@ -91,7 +91,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
   expected_policy.set_reason("Prefs");
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   // Change some prefs and check that an updated policy is sent.
   prefs.ac_idle_warning_delay_ms = 700000;
@@ -106,7 +106,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
   expected_policy.clear_ac_brightness_percent();
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   // The enable-screen-lock pref should force the screen-lock delays to
   // match the screen-off delays plus a constant value.
@@ -118,7 +118,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
       360000 + PowerPolicyController::kScreenLockAfterOffDelayMs);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   // If the screen-lock-delay prefs are set to lower values than the
   // screen-off delays plus the constant, the lock prefs should take
@@ -130,7 +130,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
   expected_policy.mutable_battery_delays()->set_screen_lock_ms(60000);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   // If the artificial screen-lock delays would exceed the idle delay, they
   // shouldn't be set -- the power manager would ignore them since the
@@ -148,7 +148,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
   expected_policy.mutable_battery_delays()->set_screen_lock_ms(-1);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   // Set the "allow screen wake locks" pref to false.  The system should be
   // prevented from suspending due to user inactivity on AC power but the
@@ -161,7 +161,7 @@ TEST_F(PowerPolicyControllerTest, Prefs) {
   expected_policy.set_reason("Prefs, Screen");
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 }
 
 TEST_F(PowerPolicyControllerTest, WakeLocks) {
@@ -176,7 +176,7 @@ TEST_F(PowerPolicyControllerTest, WakeLocks) {
   expected_policy.set_reason(kSystemWakeLockReason);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   const char kScreenWakeLockReason[] = "screen";
   const int screen_id = policy_controller_->AddScreenWakeLock(
@@ -191,19 +191,27 @@ TEST_F(PowerPolicyControllerTest, WakeLocks) {
       std::string(kScreenWakeLockReason) + ", " + kSystemWakeLockReason);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   policy_controller_->RemoveWakeLock(system_id);
   expected_policy.set_reason(kScreenWakeLockReason);
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
 
   policy_controller_->RemoveWakeLock(screen_id);
   expected_policy.Clear();
   EXPECT_EQ(PowerPolicyController::GetPolicyDebugString(expected_policy),
             PowerPolicyController::GetPolicyDebugString(
-                fake_power_client_->get_policy()));
+                fake_power_client_->policy()));
+}
+
+TEST_F(PowerPolicyControllerTest, AvoidSendingEmptyPolicies) {
+  // Check that empty policies aren't sent when PowerPolicyController is created
+  // or destroyed.
+  EXPECT_EQ(0, fake_power_client_->num_set_policy_calls());
+  policy_controller_.reset();
+  EXPECT_EQ(0, fake_power_client_->num_set_policy_calls());
 }
 
 }  // namespace chromeos