shill: Use SuspendDone signal instead of PowerStateChanged.
PowerStateChanged will be removed from powerd soon; clients
should use SuspendImminent and SuspendDone instead.
Also simplify the interface exposed by PowerManager for
adding and removing suspend delays.
BUG=chromium:359619
TEST=updated tests
Change-Id: I7f96774570170591c2e5944245c368e165aaf972
Reviewed-on: https://chromium-review.googlesource.com/195690
Reviewed-by: Daniel Erat <derat@chromium.org>
Tested-by: Daniel Erat <derat@chromium.org>
Commit-Queue: Daniel Erat <derat@chromium.org>
diff --git a/dbus_bindings/power_manager.xml b/dbus_bindings/power_manager.xml
index 18e0170..24d5a58 100644
--- a/dbus_bindings/power_manager.xml
+++ b/dbus_bindings/power_manager.xml
@@ -17,8 +17,8 @@
<signal name="SuspendImminent">
<arg name="serialized_proto" type="ay"/>
</signal>
- <signal name="PowerStateChanged">
- <arg name="new_power_state" type="s"/>
+ <signal name="SuspendDone">
+ <arg name="serialized_proto" type="ay"/>
</signal>
</interface>
</node>
diff --git a/manager.cc b/manager.cc
index 73ef057..92d94be 100644
--- a/manager.cc
+++ b/manager.cc
@@ -121,7 +121,6 @@
use_startup_portal_list_(false),
termination_actions_(dispatcher),
suspend_delay_registered_(false),
- suspend_delay_id_(0),
default_service_callback_tag_(0),
crypto_util_proxy_(new CryptoUtilProxy(dispatcher, glib)),
health_checker_remote_ips_(new IPAddressStore()) {
@@ -201,13 +200,6 @@
power_manager_.reset(
new PowerManager(dispatcher_, ProxyFactory::GetInstance()));
- power_manager_->AddStateChangeCallback(
- kPowerManagerKey,
- Bind(&Manager::OnPowerStateChanged, AsWeakPtr()));
- // TODO(ers): weak ptr for metrics_?
- PowerManager::PowerStateCallback cb =
- Bind(&Metrics::NotifyPowerStateChange, Unretained(metrics_));
- power_manager_->AddStateChangeCallback(Metrics::kMetricPowerManagerKey, cb);
CHECK(base::CreateDirectory(run_path_)) << run_path_.value();
resolver_->set_path(run_path_.Append("resolv.conf"));
@@ -1092,16 +1084,15 @@
void Manager::AddTerminationAction(const string &name,
const base::Closure &start) {
- if (termination_actions_.IsEmpty() && power_manager_.get()) {
- power_manager_->AddSuspendDelayCallback(
+ if (termination_actions_.IsEmpty() && power_manager_.get() &&
+ !suspend_delay_registered_) {
+ suspend_delay_registered_ = power_manager_->AddSuspendDelay(
kPowerManagerKey,
- Bind(&Manager::OnSuspendImminent, AsWeakPtr()));
- CHECK(!suspend_delay_registered_);
- suspend_delay_registered_ = power_manager_->RegisterSuspendDelay(
+ kSuspendDelayDescription,
base::TimeDelta::FromMilliseconds(
kTerminationActionsTimeoutMilliseconds),
- kSuspendDelayDescription,
- &suspend_delay_id_);
+ Bind(&Manager::OnSuspendImminent, AsWeakPtr()),
+ Bind(&Manager::OnSuspendDone, AsWeakPtr()));
}
termination_actions_.Add(name, start);
}
@@ -1117,14 +1108,12 @@
return;
}
termination_actions_.Remove(name);
- if (termination_actions_.IsEmpty() && power_manager_.get()) {
- if (suspend_delay_registered_) {
- SLOG(Manager, 2) << "Unregistering suspend delay.";
- power_manager_->UnregisterSuspendDelay(suspend_delay_id_);
+ if (termination_actions_.IsEmpty() && power_manager_.get() &&
+ suspend_delay_registered_) {
+ SLOG(Manager, 2) << "Unregistering suspend delay.";
+ if (power_manager_->RemoveSuspendDelay(kPowerManagerKey)) {
suspend_delay_registered_ = false;
- suspend_delay_id_ = 0;
}
- power_manager_->RemoveSuspendDelayCallback(kPowerManagerKey);
}
}
@@ -1286,32 +1275,29 @@
}
}
-void Manager::OnPowerStateChanged(
- PowerManagerProxyDelegate::SuspendState power_state) {
- if (power_state == PowerManagerProxyDelegate::kOn) {
- vector<ServiceRefPtr>::iterator sit;
- for (sit = services_.begin(); sit != services_.end(); ++sit) {
- (*sit)->OnAfterResume();
- }
- SortServices();
- vector<DeviceRefPtr>::iterator it;
- for (it = devices_.begin(); it != devices_.end(); ++it) {
- (*it)->OnAfterResume();
- }
- } else if (power_state == PowerManagerProxyDelegate::kMem) {
- vector<DeviceRefPtr>::iterator it;
- for (it = devices_.begin(); it != devices_.end(); ++it) {
- (*it)->OnBeforeSuspend();
- }
+void Manager::OnSuspendImminent(int suspend_id) {
+ vector<DeviceRefPtr>::iterator it;
+ for (it = devices_.begin(); it != devices_.end(); ++it) {
+ (*it)->OnBeforeSuspend();
+ }
+ if (!RunTerminationActionsAndNotifyMetrics(
+ Bind(&Manager::OnSuspendActionsComplete, AsWeakPtr(), suspend_id),
+ Metrics::kTerminationActionReasonSuspend)) {
+ LOG(INFO) << "No asynchronous suspend actions were run.";
+ power_manager_->ReportSuspendReadiness(kPowerManagerKey, suspend_id);
}
}
-void Manager::OnSuspendImminent(int suspend_id) {
- if (!RunTerminationActionsAndNotifyMetrics(
- Bind(&Manager::OnSuspendActionsComplete, AsWeakPtr(), suspend_id),
- Metrics::kTerminationActionReasonSuspend)) {
- LOG(INFO) << "No suspend actions were run.";
- power_manager_->ReportSuspendReadiness(suspend_delay_id_, suspend_id);
+void Manager::OnSuspendDone(int suspend_id) {
+ metrics_->NotifySuspendDone();
+ vector<ServiceRefPtr>::iterator sit;
+ for (sit = services_.begin(); sit != services_.end(); ++sit) {
+ (*sit)->OnAfterResume();
+ }
+ SortServices();
+ vector<DeviceRefPtr>::iterator it;
+ for (it = devices_.begin(); it != devices_.end(); ++it) {
+ (*it)->OnAfterResume();
}
}
@@ -1319,7 +1305,7 @@
LOG(INFO) << "Finished suspend actions. Result: " << error;
metrics_->NotifyTerminationActionsCompleted(
Metrics::kTerminationActionReasonSuspend, error.IsSuccess());
- power_manager_->ReportSuspendReadiness(suspend_delay_id_, suspend_id);
+ power_manager_->ReportSuspendReadiness(kPowerManagerKey, suspend_id);
}
void Manager::FilterByTechnology(Technology::Identifier tech,
@@ -1463,10 +1449,8 @@
LOG(INFO) << "Auto-connect suppressed -- not running.";
return;
}
- if (power_manager_.get() &&
- power_manager_->power_state() != PowerManagerProxyDelegate::kOn &&
- power_manager_->power_state() != PowerManagerProxyDelegate::kUnknown) {
- LOG(INFO) << "Auto-connect suppressed -- power state is not 'on'.";
+ if (power_manager_ && power_manager_->suspending()) {
+ LOG(INFO) << "Auto-connect suppressed -- system is suspending.";
return;
}
if (services_.empty()) {
diff --git a/manager.h b/manager.h
index 2f969b1..4e5ba64 100644
--- a/manager.h
+++ b/manager.h
@@ -291,8 +291,6 @@
bool GetArpGateway() const { return props_.arp_gateway; }
const std::string &GetHostName() const { return props_.host_name; }
- int suspend_delay_id_for_testing() const { return suspend_delay_id_; }
-
virtual void UpdateEnabledTechnologies();
virtual void UpdateUninitializedTechnologies();
@@ -525,9 +523,14 @@
// Error::kSuccess. Otherwise, it is called with Error::kOperationTimeout.
void RunTerminationActions(const base::Callback<void(const Error &)> &done);
- void OnPowerStateChanged(PowerManagerProxyDelegate::SuspendState power_state);
+ // Called when the system is about to be suspended. Each call will be
+ // followed by a call to OnSuspendDone().
void OnSuspendImminent(int suspend_id);
+ // Called when the system has completed a suspend attempt (possibly without
+ // actually suspending, in the event of the user canceling the attempt).
+ void OnSuspendDone(int suspend_id);
+
void OnSuspendActionsComplete(int suspend_id, const Error &error);
void VerifyToEncryptLink(std::string public_key, std::string data,
ResultStringCallback cb, const Error &error,
@@ -613,10 +616,6 @@
// Is a suspend delay currently registered with the power manager?
bool suspend_delay_registered_;
- // If |suspend_delay_registered_| is true, contains the unique ID
- // corresponding to the suspend delay.
- int suspend_delay_id_;
-
// Maps tags to callbacks for monitoring default service changes.
std::map<int, ServiceCallback> default_service_callbacks_;
int default_service_callback_tag_;
diff --git a/manager_unittest.cc b/manager_unittest.cc
index fb44c26..335be36 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -360,8 +360,8 @@
DISALLOW_COPY_AND_ASSIGN(DisableTechnologyReplyHandler);
};
- void SetPowerState(PowerManagerProxyDelegate::SuspendState state) {
- power_manager_->power_state_ = state;
+ void SetSuspending(bool suspending) {
+ power_manager_->suspending_ = suspending;
}
void SetPowerManager() {
@@ -372,14 +372,14 @@
return &manager()->termination_actions_;
}
- void OnPowerStateChanged(PowerManagerProxyDelegate::SuspendState state) {
- manager()->OnPowerStateChanged(state);
- }
-
void OnSuspendImminent(int suspend_id) {
manager()->OnSuspendImminent(suspend_id);
}
+ void OnSuspendDone(int suspend_id) {
+ manager()->OnSuspendDone(suspend_id);
+ }
+
void OnSuspendActionsComplete(int suspend_id, const Error &error) {
manager()->OnSuspendActionsComplete(suspend_id, error);
}
@@ -2970,36 +2970,18 @@
dispatcher()->DispatchPendingEvents();
}
-TEST_F(ManagerTest, AutoConnectOnPowerStateSuspending) {
+TEST_F(ManagerTest, AutoConnectOnSuspending) {
MockServiceRefPtr service = MakeAutoConnectableService();
- SetPowerState(PowerManagerProxyDelegate::kSuspending);
+ SetSuspending(true);
SetPowerManager();
EXPECT_CALL(*service, AutoConnect()).Times(0);
manager()->RegisterService(service);
dispatcher()->DispatchPendingEvents();
}
-TEST_F(ManagerTest, AutoConnectOnPowerStateMem) {
+TEST_F(ManagerTest, AutoConnectOnNotSuspending) {
MockServiceRefPtr service = MakeAutoConnectableService();
- SetPowerState(PowerManagerProxyDelegate::kMem);
- SetPowerManager();
- EXPECT_CALL(*service, AutoConnect()).Times(0);
- manager()->RegisterService(service);
- dispatcher()->DispatchPendingEvents();
-}
-
-TEST_F(ManagerTest, AutoConnectOnPowerStateOn) {
- MockServiceRefPtr service = MakeAutoConnectableService();
- SetPowerState(PowerManagerProxyDelegate::kOn);
- SetPowerManager();
- EXPECT_CALL(*service, AutoConnect());
- manager()->RegisterService(service);
- dispatcher()->DispatchPendingEvents();
-}
-
-TEST_F(ManagerTest, AutoConnectOnPowerStateUnknown) {
- MockServiceRefPtr service = MakeAutoConnectableService();
- SetPowerState(PowerManagerProxyDelegate::kUnknown);
+ SetSuspending(false);
SetPowerManager();
EXPECT_CALL(*service, AutoConnect());
manager()->RegisterService(service);
@@ -3014,31 +2996,30 @@
dispatcher()->DispatchPendingEvents();
}
-TEST_F(ManagerTest, OnPowerStateChanged) {
+TEST_F(ManagerTest, Suspend) {
MockServiceRefPtr service = MakeAutoConnectableService();
- SetPowerState(PowerManagerProxyDelegate::kOn);
SetPowerManager();
EXPECT_CALL(*service, AutoConnect());
manager()->RegisterService(service);
manager()->RegisterDevice(mock_devices_[0]);
dispatcher()->DispatchPendingEvents();
- EXPECT_CALL(*mock_devices_[0], OnAfterResume());
- OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
- EXPECT_CALL(*service, AutoConnect());
+ const int kSuspendId = 1;
+ EXPECT_CALL(*mock_devices_[0], OnBeforeSuspend());
+ OnSuspendImminent(kSuspendId);
+ EXPECT_CALL(*service, AutoConnect()).Times(0);
dispatcher()->DispatchPendingEvents();
Mock::VerifyAndClearExpectations(mock_devices_[0]);
- EXPECT_CALL(*mock_devices_[0], OnBeforeSuspend());
- OnPowerStateChanged(PowerManagerProxyDelegate::kMem);
- EXPECT_CALL(*service, AutoConnect()).Times(0);
+ EXPECT_CALL(*mock_devices_[0], OnAfterResume());
+ OnSuspendDone(kSuspendId);
+ EXPECT_CALL(*service, AutoConnect());
dispatcher()->DispatchPendingEvents();
Mock::VerifyAndClearExpectations(mock_devices_[0]);
}
TEST_F(ManagerTest, AddTerminationAction) {
- EXPECT_CALL(*power_manager_, AddSuspendDelayCallback(_, _));
- EXPECT_CALL(*power_manager_, RegisterSuspendDelay(_, _, _));
+ EXPECT_CALL(*power_manager_, AddSuspendDelay(_, _, _, _, _));
SetPowerManager();
EXPECT_TRUE(GetTerminationActions()->IsEmpty());
manager()->AddTerminationAction("action1", base::Closure());
@@ -3049,39 +3030,35 @@
TEST_F(ManagerTest, RemoveTerminationAction) {
const char kKey1[] = "action1";
const char kKey2[] = "action2";
- const int kSuspendDelayId = 123;
MockPowerManager &power_manager = *power_manager_;
SetPowerManager();
// Removing an action when the hook table is empty should not result in any
// calls to the power manager.
- EXPECT_CALL(power_manager, UnregisterSuspendDelay(_)).Times(0);
- EXPECT_CALL(power_manager, RemoveSuspendDelayCallback(_)).Times(0);
+ EXPECT_CALL(power_manager, RemoveSuspendDelay(_)).Times(0);
EXPECT_TRUE(GetTerminationActions()->IsEmpty());
manager()->RemoveTerminationAction("unknown");
Mock::VerifyAndClearExpectations(&power_manager);
- EXPECT_CALL(power_manager, RegisterSuspendDelay(_, _, _))
- .WillOnce(DoAll(SetArgumentPointee<2>(kSuspendDelayId), Return(true)));
- EXPECT_CALL(power_manager, AddSuspendDelayCallback(_, _)).Times(1);
+ EXPECT_CALL(power_manager, AddSuspendDelay(_, _, _, _, _))
+ .WillOnce(Return(true));
manager()->AddTerminationAction(kKey1, base::Closure());
EXPECT_FALSE(GetTerminationActions()->IsEmpty());
manager()->AddTerminationAction(kKey2, base::Closure());
+ Mock::VerifyAndClearExpectations(&power_manager);
// Removing an action that ends up with a non-empty hook table should not
// result in any calls to the power manager.
- EXPECT_CALL(power_manager, UnregisterSuspendDelay(_)).Times(0);
- EXPECT_CALL(power_manager, RemoveSuspendDelayCallback(_)).Times(0);
+ EXPECT_CALL(power_manager, RemoveSuspendDelay(_)).Times(0);
manager()->RemoveTerminationAction(kKey1);
EXPECT_FALSE(GetTerminationActions()->IsEmpty());
Mock::VerifyAndClearExpectations(&power_manager);
// Removing the last action should trigger unregistering from the power
// manager.
- EXPECT_CALL(power_manager, UnregisterSuspendDelay(kSuspendDelayId))
+ EXPECT_CALL(power_manager, RemoveSuspendDelay(_))
.WillOnce(Return(true));
- EXPECT_CALL(power_manager, RemoveSuspendDelayCallback(_));
manager()->RemoveTerminationAction(kKey2);
EXPECT_TRUE(GetTerminationActions()->IsEmpty());
}
@@ -3106,9 +3083,7 @@
TEST_F(ManagerTest, OnSuspendImminent) {
const int kSuspendId = 123;
EXPECT_TRUE(GetTerminationActions()->IsEmpty());
- EXPECT_CALL(*power_manager_,
- ReportSuspendReadiness(
- manager()->suspend_delay_id_for_testing(), kSuspendId));
+ EXPECT_CALL(*power_manager_, ReportSuspendReadiness(_, kSuspendId));
SetPowerManager();
OnSuspendImminent(kSuspendId);
}
@@ -3116,9 +3091,7 @@
TEST_F(ManagerTest, OnSuspendActionsComplete) {
const int kSuspendId = 54321;
Error error;
- EXPECT_CALL(*power_manager_,
- ReportSuspendReadiness(
- manager()->suspend_delay_id_for_testing(), kSuspendId));
+ EXPECT_CALL(*power_manager_, ReportSuspendReadiness(_, kSuspendId));
SetPowerManager();
OnSuspendActionsComplete(kSuspendId, error);
}
diff --git a/metrics.cc b/metrics.cc
index 78fec18..90c1e90 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -647,12 +647,8 @@
kMetricSignalAtDisconnectNumBuckets);
}
-void Metrics::NotifyPowerStateChange(PowerManager::SuspendState new_state) {
- if (new_state == PowerManagerProxyDelegate::kOn) {
- time_resume_to_ready_timer_->Start();
- } else {
- time_resume_to_ready_timer_->Reset();
- }
+void Metrics::NotifySuspendDone() {
+ time_resume_to_ready_timer_->Start();
}
void Metrics::NotifyTerminationActionsStarted(
diff --git a/metrics.h b/metrics.h
index ba1d36c..f0f9459 100644
--- a/metrics.h
+++ b/metrics.h
@@ -518,8 +518,8 @@
void NotifySignalAtDisconnect(const Service &service,
int16_t signal_strength);
- // Notifies this object of a power management state change.
- void NotifyPowerStateChange(PowerManager::SuspendState new_state);
+ // Notifies this object of the end of a suspend attempt.
+ void NotifySuspendDone();
// Notifies this object that termination actions started executing.
void NotifyTerminationActionsStarted(TerminationActionReason reason);
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index 1ff9ed4..0b4f927 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -209,8 +209,7 @@
Metrics::kTimerHistogramNumBuckets));
EXPECT_CALL(*mock_time_resume_to_ready_timer, GetElapsedTime(_)).
WillOnce(DoAll(SetArgumentPointee<0>(non_zero_time_delta), Return(true)));
- metrics_.NotifyPowerStateChange(PowerManagerProxyDelegate::kMem);
- metrics_.NotifyPowerStateChange(PowerManagerProxyDelegate::kOn);
+ metrics_.NotifySuspendDone();
metrics_.NotifyServiceStateChanged(*wep_wifi_service_,
Service::kStateConnected);
Mock::VerifyAndClearExpectations(&library_);
diff --git a/mock_power_manager.h b/mock_power_manager.h
index 34aa670..f2e272a 100644
--- a/mock_power_manager.h
+++ b/mock_power_manager.h
@@ -19,20 +19,15 @@
MockPowerManager(EventDispatcher *dispatcher, ProxyFactory *proxy_factory);
virtual ~MockPowerManager();
- MOCK_METHOD2(AddStateChangeCallback,
- void(const std::string &key,
- const PowerStateCallback &callback));
- MOCK_METHOD2(AddSuspendDelayCallback,
- void(const std::string &key,
- const SuspendDelayCallback &callback));
- MOCK_METHOD1(RemoveStateChangeCallback, void(const std::string &key));
- MOCK_METHOD1(RemoveSuspendDelayCallback, void(const std::string &key));
- MOCK_METHOD3(RegisterSuspendDelay,
- bool(base::TimeDelta timeout,
+ MOCK_METHOD5(AddSuspendDelay,
+ bool(const std::string &key,
const std::string &description,
- int *delay_id_out));
- MOCK_METHOD1(UnregisterSuspendDelay, bool(int delay_id));
- MOCK_METHOD2(ReportSuspendReadiness, bool(int delay_id, int suspend_id));
+ base::TimeDelta timeout,
+ const SuspendImminentCallback &immiment_callback,
+ const SuspendDoneCallback &done_callback));
+ MOCK_METHOD1(RemoveSuspendDelay, bool(const std::string &key));
+ MOCK_METHOD2(ReportSuspendReadiness,
+ bool(const std::string &key, int suspend_id));
private:
DISALLOW_COPY_AND_ASSIGN(MockPowerManager);
diff --git a/power_manager.cc b/power_manager.cc
index beb6343..cae3e41 100644
--- a/power_manager.cc
+++ b/power_manager.cc
@@ -25,31 +25,63 @@
ProxyFactory *proxy_factory)
: dispatcher_(dispatcher),
power_manager_proxy_(proxy_factory->CreatePowerManagerProxy(this)),
- power_state_(kUnknown) {}
+ suspending_(false) {}
PowerManager::~PowerManager() {
}
-void PowerManager::AddStateChangeCallback(const string &key,
- const PowerStateCallback &callback) {
- SLOG(Power, 2) << __func__ << " key " << key;
- AddCallback(key, callback, &state_change_callbacks_);
+bool PowerManager::AddSuspendDelay(
+ const std::string &key,
+ const std::string &description,
+ base::TimeDelta timeout,
+ const SuspendImminentCallback &imminent_callback,
+ const SuspendDoneCallback &done_callback) {
+ CHECK(!imminent_callback.is_null());
+ CHECK(!done_callback.is_null());
+
+ if (ContainsKey(suspend_delays_, key)) {
+ LOG(ERROR) << "Ignoring request to insert duplicate key " << key;
+ return false;
+ }
+
+ int delay_id = 0;
+ if (!power_manager_proxy_->RegisterSuspendDelay(
+ timeout, description, &delay_id)) {
+ return false;
+ }
+
+ SuspendDelay delay;
+ delay.imminent_callback = imminent_callback;
+ delay.done_callback = done_callback;
+ delay.delay_id = delay_id;
+ suspend_delays_[key] = delay;
+ return true;
}
-void PowerManager::AddSuspendDelayCallback(
- const string &key, const SuspendDelayCallback &callback) {
- SLOG(Power, 2) << __func__ << " key " << key;
- AddCallback(key, callback, &suspend_delay_callbacks_);
+bool PowerManager::RemoveSuspendDelay(const std::string &key) {
+ SuspendDelayMap::const_iterator it = suspend_delays_.find(key);
+ if (it == suspend_delays_.end()) {
+ LOG(ERROR) << "Ignoring unregistered key " << key;
+ return false;
+ }
+
+ if (!power_manager_proxy_->UnregisterSuspendDelay(it->second.delay_id)) {
+ return false;
+ }
+
+ suspend_delays_.erase(it);
+ return true;
}
-void PowerManager::RemoveStateChangeCallback(const string &key) {
- SLOG(Power, 2) << __func__ << " key " << key;
- RemoveCallback(key, &state_change_callbacks_);
-}
-
-void PowerManager::RemoveSuspendDelayCallback(const string &key) {
- SLOG(Power, 2) << __func__ << " key " << key;
- RemoveCallback(key, &suspend_delay_callbacks_);
+bool PowerManager::ReportSuspendReadiness(const std::string &key,
+ int suspend_id) {
+ SuspendDelayMap::const_iterator it = suspend_delays_.find(key);
+ if (it == suspend_delays_.end()) {
+ LOG(ERROR) << "Ignoring unregistered key " << key;
+ return false;
+ }
+ return power_manager_proxy_->ReportSuspendReadiness(
+ it->second.delay_id, suspend_id);
}
void PowerManager::OnSuspendImminent(int suspend_id) {
@@ -58,78 +90,36 @@
// that the manager can suppress auto-connect, for example. Schedules a
// suspend timeout in case the suspend attempt failed or got interrupted, and
// there's no proper notification from the power manager.
- power_state_ = kSuspending;
+ suspending_ = true;
suspend_timeout_.Reset(
- base::Bind(&PowerManager::OnSuspendTimeout, base::Unretained(this)));
+ base::Bind(&PowerManager::OnSuspendTimeout, base::Unretained(this),
+ suspend_id));
dispatcher_->PostDelayedTask(suspend_timeout_.callback(),
kSuspendTimeoutMilliseconds);
- OnEvent(suspend_id, &suspend_delay_callbacks_);
+
+ for (SuspendDelayMap::const_iterator it = suspend_delays_.begin();
+ it != suspend_delays_.end(); ++it) {
+ SuspendImminentCallback callback = it->second.imminent_callback;
+ CHECK(!callback.is_null());
+ callback.Run(suspend_id);
+ }
}
-void PowerManager::OnPowerStateChanged(SuspendState new_power_state) {
- LOG(INFO) << "Power state changed: "
- << power_state_ << "->" << new_power_state;
+void PowerManager::OnSuspendDone(int suspend_id) {
+ LOG(INFO) << __func__ << "(" << suspend_id << ")";
suspend_timeout_.Cancel();
- power_state_ = new_power_state;
- OnEvent(new_power_state, &state_change_callbacks_);
+ suspending_ = false;
+ for (SuspendDelayMap::const_iterator it = suspend_delays_.begin();
+ it != suspend_delays_.end(); ++it) {
+ SuspendDoneCallback callback = it->second.done_callback;
+ CHECK(!callback.is_null());
+ callback.Run(suspend_id);
+ }
}
-bool PowerManager::RegisterSuspendDelay(base::TimeDelta timeout,
- const string &description,
- int *delay_id_out) {
- return power_manager_proxy_->RegisterSuspendDelay(timeout, description,
- delay_id_out);
-}
-
-bool PowerManager::UnregisterSuspendDelay(int delay_id) {
- return power_manager_proxy_->UnregisterSuspendDelay(delay_id);
-}
-
-bool PowerManager::ReportSuspendReadiness(int delay_id, int suspend_id) {
- return power_manager_proxy_->ReportSuspendReadiness(delay_id, suspend_id);
-}
-
-void PowerManager::OnSuspendTimeout() {
+void PowerManager::OnSuspendTimeout(int suspend_id) {
LOG(ERROR) << "Suspend timed out -- assuming power-on state.";
- OnPowerStateChanged(kOn);
+ OnSuspendDone(suspend_id);
}
-template<class Callback>
-void PowerManager::AddCallback(const string &key, const Callback &callback,
- map<const string, Callback> *callback_map) {
- CHECK(callback_map != NULL);
- CHECK(!callback.is_null());
- if (ContainsKey(*callback_map, key)) {
- LOG(DFATAL) << "Inserting duplicate key " << key;
- LOG(INFO) << "Removing previous callback for key " << key;
- RemoveCallback(key, callback_map);
- }
- (*callback_map)[key] = callback;
-}
-
-template<class Callback>
-void PowerManager::RemoveCallback(const string &key,
- map<const string, Callback> *callback_map) {
- CHECK(callback_map != NULL);
- DCHECK(ContainsKey(*callback_map, key)) << "Removing unknown key " << key;
- typename map<const string, Callback>::iterator it = callback_map->find(key);
- if (it != callback_map->end()) {
- callback_map->erase(it);
- }
-}
-
-template<class Param, class Callback>
-void PowerManager::OnEvent(const Param ¶m,
- map<const string, Callback> *callback_map) const {
- CHECK(callback_map != NULL);
- for (typename map<const string, Callback>::const_iterator it =
- callback_map->begin();
- it != callback_map->end(); ++it) {
- CHECK(!it->second.is_null());
- it->second.Run(param);
- }
-}
-
-
-
} // namespace shill
diff --git a/power_manager.h b/power_manager.h
index 42ca2ea..e28500c 100644
--- a/power_manager.h
+++ b/power_manager.h
@@ -8,27 +8,6 @@
// This class instantiates a PowerManagerProxy and distributes power events to
// registered users. It also provides a means for calling methods on the
// PowerManagerProxy.
-//
-// Usage:
-//
-// Registering for power state changes is done as follows:
-//
-// class Foo {
-// public:
-// void HandleStateChange(PowerManager::SuspendState new_state);
-// };
-// Foo foo;
-// PowerManager power_manager(ProxyFactory::GetInstance());
-// PowerManager::PowerStateCallback cb = Bind(&Foo::HandleStateChange, &foo);
-// power_manager.AddStateChangeCallback("foo_key", cb);
-//
-// Note that depending on the definition of Foo, "&foo" may need to appear
-// inside an appropriate wrapper, such as base::Unretained.
-//
-// Whenever the power state changes, foo.HandleStateChange() is called with the
-// new state passed in. To unregister:
-//
-// power_manager.RemoveStateChangeCallback("foo_key");
#include <map>
#include <string>
@@ -46,19 +25,16 @@
class PowerManager : public PowerManagerProxyDelegate {
public:
- typedef PowerManagerProxyDelegate::SuspendState SuspendState;
-
- // Callbacks registered with the power manager are of this type. They take
- // one argument, the new power state. The callback function or method should
- // look like this:
- //
- // void HandlePowerStateChange(PowerStateCallbacks::SuspendState);
- typedef base::Callback<void(SuspendState)> PowerStateCallback;
-
- // This callback is called prior to a suspend event. When it is OK for the
+ // This callback is called prior to a suspend attempt. When it is OK for the
// system to suspend, this callback should call ReportSuspendReadiness(),
- // passing it the suspend ID passed to this callback.
- typedef base::Callback<void(int)> SuspendDelayCallback;
+ // passing it the key passed to AddSuspendDelay() and the suspend ID passed to
+ // this callback.
+ typedef base::Callback<void(int)> SuspendImminentCallback;
+
+ // This callback is called after the completion of a suspend attempt. The
+ // receiver should undo any pre-suspend work that was done by the
+ // SuspendImminentCallback.
+ typedef base::Callback<void(int)> SuspendDoneCallback;
static const int kSuspendTimeoutMilliseconds;
@@ -68,74 +44,61 @@
ProxyFactory *proxy_factory);
virtual ~PowerManager();
- SuspendState power_state() const { return power_state_; }
+ bool suspending() const { return suspending_; }
+
+ // Registers a suspend delay with the power manager under the unique name
+ // |key|. See PowerManagerProxyInterface::RegisterSuspendDelay() for
+ // information about |description| and |timeout|. |imminent_callback| will be
+ // invoked when a suspend attempt is commenced and |done_callback| will be
+ // invoked when the attempt is completed. Returns false on failure.
+ virtual bool AddSuspendDelay(const std::string &key,
+ const std::string &description,
+ base::TimeDelta timeout,
+ const SuspendImminentCallback &immiment_callback,
+ const SuspendDoneCallback &done_callback);
+
+ // Removes a suspend delay previously created via AddSuspendDelay(). Returns
+ // false on failure.
+ virtual bool RemoveSuspendDelay(const std::string &key);
+
+ // Reports readiness for suspend attempt |suspend_id| on behalf of the suspend
+ // delay described by |key|, the value originally passed to AddSuspendDelay().
+ // Returns false on failure.
+ virtual bool ReportSuspendReadiness(const std::string &key, int suspend_id);
// Methods inherited from PowerManagerProxyDelegate.
virtual void OnSuspendImminent(int suspend_id);
- virtual void OnPowerStateChanged(SuspendState new_power_state);
-
- // See corresponding methods in PowerManagerProxyInterface.
- virtual bool RegisterSuspendDelay(base::TimeDelta timeout,
- const std::string &description,
- int *delay_id_out);
- virtual bool UnregisterSuspendDelay(int delay_id);
- virtual bool ReportSuspendReadiness(int delay_id, int suspend_id);
-
- // Registers a power state change callback with the power manager. When a
- // power state change occurs, this callback will be called with the new power
- // state as its argument. |key| must be unique.
- virtual void AddStateChangeCallback(const std::string &key,
- const PowerStateCallback &callback);
-
- // Registers a suspend delay callback with the power manager. This callback
- // will be called prior to a suspend event by the amount specified in the most
- // recent call to RegisterSuspendDelay(). |key| must be unique.
- virtual void AddSuspendDelayCallback(const std::string &key,
- const SuspendDelayCallback &callback);
-
- // Unregisters a power state change callback identified by |key|. The
- // callback must have been previously registered and not yet removed.
- virtual void RemoveStateChangeCallback(const std::string &key);
-
- // Unregisters a suspend delay callback identified by |key|. The callback
- // must have been previously registered and not yet removed.
- virtual void RemoveSuspendDelayCallback(const std::string &key);
+ virtual void OnSuspendDone(int suspend_id);
private:
friend class ManagerTest;
friend class PowerManagerTest;
friend class ServiceTest;
- typedef std::map<const std::string, PowerStateCallback>
- StateChangeCallbackMap;
+ // Information about a suspend delay added via AddSuspendDelay().
+ struct SuspendDelay {
+ SuspendImminentCallback imminent_callback;
+ SuspendDoneCallback done_callback;
+ int delay_id;
+ };
- typedef std::map<const std::string, SuspendDelayCallback>
- SuspendDelayCallbackMap;
+ // Keyed by the |key| argument to AddSuspendDelay().
+ typedef std::map<const std::string, SuspendDelay> SuspendDelayMap;
- void OnSuspendTimeout();
-
- template<class Callback>
- void AddCallback(const std::string &key, const Callback &callback,
- std::map<const std::string, Callback> *callback_map);
-
- template<class Callback>
- void RemoveCallback(const std::string &key,
- std::map<const std::string, Callback> *callback_map);
-
- template<class Param, class Callback>
- void OnEvent(const Param ¶m,
- std::map<const std::string, Callback> *callback_map) const;
+ // Run by |suspend_timeout_| to manually invoke OnSuspendDone() if the signal
+ // never arrives.
+ void OnSuspendTimeout(int suspend_id);
EventDispatcher *dispatcher_;
// The power manager proxy created by this class. It dispatches the inherited
// delegate methods of this object when changes in the power state occur.
const scoped_ptr<PowerManagerProxyInterface> power_manager_proxy_;
- StateChangeCallbackMap state_change_callbacks_;
- SuspendDelayCallbackMap suspend_delay_callbacks_;
+ SuspendDelayMap suspend_delays_;
base::CancelableClosure suspend_timeout_;
- SuspendState power_state_;
+ // Set to true by OnSuspendImminent() and to false by OnSuspendDone().
+ bool suspending_;
DISALLOW_COPY_AND_ASSIGN(PowerManager);
};
diff --git a/power_manager_proxy.cc b/power_manager_proxy.cc
index 4823171..e5b95c7 100644
--- a/power_manager_proxy.cc
+++ b/power_manager_proxy.cc
@@ -132,23 +132,15 @@
delegate_->OnSuspendImminent(proto.suspend_id());
}
-void PowerManagerProxy::Proxy::PowerStateChanged(
- const string &new_power_state) {
- LOG(INFO) << __func__ << "(" << new_power_state << ")";
-
- PowerManagerProxyDelegate::SuspendState suspend_state;
- if (new_power_state == "on") {
- suspend_state = PowerManagerProxyDelegate::kOn;
- } else if (new_power_state == "standby") {
- suspend_state = PowerManagerProxyDelegate::kStandby;
- } else if (new_power_state == "mem") {
- suspend_state = PowerManagerProxyDelegate::kMem;
- } else if (new_power_state == "disk") {
- suspend_state = PowerManagerProxyDelegate::kDisk;
- } else {
- suspend_state = PowerManagerProxyDelegate::kUnknown;
+void PowerManagerProxy::Proxy::SuspendDone(
+ const vector<uint8_t> &serialized_proto) {
+ LOG(INFO) << __func__;
+ power_manager::SuspendDone proto;
+ if (!DeserializeProtocolBuffer(serialized_proto, &proto)) {
+ LOG(ERROR) << "Failed to parse SuspendDone signal.";
+ return;
}
- delegate_->OnPowerStateChanged(suspend_state);
+ delegate_->OnSuspendDone(proto.suspend_id());
}
} // namespace shill
diff --git a/power_manager_proxy.h b/power_manager_proxy.h
index 7e0c8fc..12f5e4b 100644
--- a/power_manager_proxy.h
+++ b/power_manager_proxy.h
@@ -51,20 +51,13 @@
private:
// Signal callbacks inherited from org::chromium::PowerManager_proxy.
virtual void SuspendImminent(const std::vector<uint8_t> &serialized_proto);
- virtual void PowerStateChanged(const std::string &new_power_state);
+ virtual void SuspendDone(const std::vector<uint8_t> &serialized_proto);
PowerManagerProxyDelegate *const delegate_;
DISALLOW_COPY_AND_ASSIGN(Proxy);
};
- // The PowerStateChanged event occurs on this path. The root path ("/") is
- // used because the powerd_suspend script sends signals on that path. When
- // the powerd_suspend script is fixed to use the same path as the rest of the
- // power manager, then this proxy can use the usual power manager path
- // power_manager::kPowerManagerServicePath.
- static const char kRootPath[];
-
// Constructs a PowerManager DBus object proxy with signals dispatched to
// |delegate|.
PowerManagerProxy(PowerManagerProxyDelegate *delegate,
diff --git a/power_manager_proxy_interface.h b/power_manager_proxy_interface.h
index 8b590a4..b8be440 100644
--- a/power_manager_proxy_interface.h
+++ b/power_manager_proxy_interface.h
@@ -45,28 +45,16 @@
// PowerManager signal delegate to be associated with the proxy.
class PowerManagerProxyDelegate {
public:
- // Possible states broadcast from the powerd_suspend script.
- enum SuspendState {
- kOn,
- kStandby,
- kMem,
- kDisk,
- kSuspending, // Internal to shill.
- // Place new states above kUnknown.
- kUnknown
- };
-
virtual ~PowerManagerProxyDelegate() {}
- // Broadcasted by the power manager when it's about to suspend. RPC clients
+ // Broadcast by the power manager when it's about to suspend. RPC clients
// that have registered through RegisterSuspendDelay() should tell the power
// manager that they're ready to suspend by calling ReportSuspendReadiness()
// with the delay ID returned by RegisterSuspendDelay() and |suspend_id|.
virtual void OnSuspendImminent(int suspend_id) = 0;
- // This method will be called when suspending or resuming. |new_power_state|
- // will be "kMem" when suspending (to memory), or "kOn" when resuming.
- virtual void OnPowerStateChanged(SuspendState new_power_state) = 0;
+ // Broadcast by the power manager when a suspend attempt has completed.
+ virtual void OnSuspendDone(int suspend_id) = 0;
};
} // namespace shill
diff --git a/power_manager_unittest.cc b/power_manager_unittest.cc
index 42b85c6..949d88e 100644
--- a/power_manager_unittest.cc
+++ b/power_manager_unittest.cc
@@ -21,7 +21,10 @@
using std::map;
using std::string;
using testing::_;
+using testing::DoAll;
+using testing::Mock;
using testing::Return;
+using testing::SetArgumentPointee;
using testing::Test;
namespace shill {
@@ -53,205 +56,240 @@
public:
static const char kKey1[];
static const char kKey2[];
+ static const char kDescription1[];
+ static const char kDescription2[];
static const int kSuspendId1 = 123;
static const int kSuspendId2 = 456;
+ static const int kDelayId1 = 4;
+ static const int kDelayId2 = 16;
PowerManagerTest()
- : power_manager_(&dispatcher_, &factory_),
+ : kTimeout(base::TimeDelta::FromSeconds(3)),
+ power_manager_(&dispatcher_, &factory_),
+ proxy_(factory_.proxy()),
delegate_(factory_.delegate()) {
- state_change_callback1_ =
- Bind(&PowerManagerTest::StateChangeAction1, Unretained(this));
- state_change_callback2_ =
- Bind(&PowerManagerTest::StateChangeAction2, Unretained(this));
- suspend_delay_callback1_ =
- Bind(&PowerManagerTest::SuspendDelayAction1, Unretained(this));
- suspend_delay_callback2_ =
- Bind(&PowerManagerTest::SuspendDelayAction2, Unretained(this));
+ suspend_imminent_callback1_ =
+ Bind(&PowerManagerTest::SuspendImminentAction1, Unretained(this));
+ suspend_imminent_callback2_ =
+ Bind(&PowerManagerTest::SuspendImminentAction2, Unretained(this));
+ suspend_done_callback1_ =
+ Bind(&PowerManagerTest::SuspendDoneAction1, Unretained(this));
+ suspend_done_callback2_ =
+ Bind(&PowerManagerTest::SuspendDoneAction2, Unretained(this));
}
- MOCK_METHOD1(StateChangeAction1, void(PowerManager::SuspendState));
- MOCK_METHOD1(StateChangeAction2, void(PowerManager::SuspendState));
- MOCK_METHOD1(SuspendDelayAction1, void(int));
- MOCK_METHOD1(SuspendDelayAction2, void(int));
+ MOCK_METHOD1(SuspendImminentAction1, void(int));
+ MOCK_METHOD1(SuspendImminentAction2, void(int));
+ MOCK_METHOD1(SuspendDoneAction1, void(int));
+ MOCK_METHOD1(SuspendDoneAction2, void(int));
protected:
+ void AddProxyRegisterSuspendDelayExpectation(
+ base::TimeDelta timeout,
+ const std::string &description,
+ int delay_id,
+ bool return_value) {
+ EXPECT_CALL(*proxy_, RegisterSuspendDelay(timeout, description, _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(delay_id),
+ Return(return_value)));
+ }
+
+ void AddProxyUnregisterSuspendDelayExpectation(int delay_id,
+ bool return_value) {
+ EXPECT_CALL(*proxy_, UnregisterSuspendDelay(delay_id))
+ .WillOnce(Return(return_value));
+ }
+
+ void AddProxyReportSuspendReadinessExpectation(int delay_id,
+ int suspend_id,
+ bool return_value) {
+ EXPECT_CALL(*proxy_, ReportSuspendReadiness(delay_id, suspend_id))
+ .WillOnce(Return(return_value));
+ }
+
void OnSuspendImminent(int suspend_id) {
EXPECT_CALL(dispatcher_,
PostDelayedTask(_, PowerManager::kSuspendTimeoutMilliseconds));
factory_.delegate()->OnSuspendImminent(suspend_id);
- EXPECT_EQ(PowerManagerProxyDelegate::kSuspending,
- power_manager_.power_state());
+ EXPECT_TRUE(power_manager_.suspending());
}
- void OnSuspendTimeout() {
- power_manager_.OnSuspendTimeout();
+ void OnSuspendDone(int suspend_id) {
+ factory_.delegate()->OnSuspendDone(suspend_id);
+ EXPECT_FALSE(power_manager_.suspending());
}
+ void OnSuspendTimeout(int suspend_id) {
+ power_manager_.OnSuspendTimeout(suspend_id);
+ }
+
+ // This is non-static since it's a non-POD type.
+ const base::TimeDelta kTimeout;
+
MockEventDispatcher dispatcher_;
FakeProxyFactory factory_;
PowerManager power_manager_;
+ MockPowerManagerProxy *const proxy_;
PowerManagerProxyDelegate *const delegate_;
- PowerManager::PowerStateCallback state_change_callback1_;
- PowerManager::PowerStateCallback state_change_callback2_;
- PowerManager::SuspendDelayCallback suspend_delay_callback1_;
- PowerManager::SuspendDelayCallback suspend_delay_callback2_;
+ PowerManager::SuspendImminentCallback suspend_imminent_callback1_;
+ PowerManager::SuspendImminentCallback suspend_imminent_callback2_;
+ PowerManager::SuspendDoneCallback suspend_done_callback1_;
+ PowerManager::SuspendDoneCallback suspend_done_callback2_;
};
const char PowerManagerTest::kKey1[] = "Zaphod";
const char PowerManagerTest::kKey2[] = "Beeblebrox";
+const char PowerManagerTest::kDescription1[] = "Foo";
+const char PowerManagerTest::kDescription2[] = "Bar";
-TEST_F(PowerManagerTest, OnPowerStateChanged) {
- EXPECT_EQ(PowerManagerProxyDelegate::kUnknown, power_manager_.power_state());
- power_manager_.OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
- EXPECT_EQ(PowerManagerProxyDelegate::kOn, power_manager_.power_state());
+TEST_F(PowerManagerTest, SuspendingState) {
+ const int kSuspendId = 3;
+ EXPECT_FALSE(power_manager_.suspending());
+ OnSuspendImminent(kSuspendId);
+ EXPECT_TRUE(power_manager_.suspending());
+ OnSuspendDone(kSuspendId);
+ EXPECT_FALSE(power_manager_.suspending());
}
-TEST_F(PowerManagerTest, AddStateChangeCallback) {
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kOn));
- power_manager_.AddStateChangeCallback(kKey1, state_change_callback1_);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
- power_manager_.RemoveStateChangeCallback(kKey1);
-}
+TEST_F(PowerManagerTest, MultipleSuspendDelays) {
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
-TEST_F(PowerManagerTest, AddSuspendDelayCallback) {
- EXPECT_CALL(*this, SuspendDelayAction1(kSuspendId1));
- power_manager_.AddSuspendDelayCallback(kKey1, suspend_delay_callback1_);
- EXPECT_EQ(PowerManagerProxyDelegate::kUnknown, power_manager_.power_state());
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription2, kDelayId2, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey2, kDescription2, kTimeout,
+ suspend_imminent_callback2_, suspend_done_callback2_));
+ Mock::VerifyAndClearExpectations(proxy_);
+
+ EXPECT_CALL(*this, SuspendImminentAction1(kSuspendId1));
+ EXPECT_CALL(*this, SuspendImminentAction2(kSuspendId1));
OnSuspendImminent(kSuspendId1);
- power_manager_.RemoveSuspendDelayCallback(kKey1);
-}
+ Mock::VerifyAndClearExpectations(this);
-TEST_F(PowerManagerTest, AddMultipleStateChangeRunMultiple) {
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kOn));
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kMem));
- power_manager_.AddStateChangeCallback(kKey1, state_change_callback1_);
+ AddProxyReportSuspendReadinessExpectation(kDelayId1, kSuspendId1, true);
+ EXPECT_TRUE(power_manager_.ReportSuspendReadiness(kKey1, kSuspendId1));
+ Mock::VerifyAndClearExpectations(proxy_);
- EXPECT_CALL(*this, StateChangeAction2(PowerManagerProxyDelegate::kOn));
- EXPECT_CALL(*this, StateChangeAction2(PowerManagerProxyDelegate::kMem));
- power_manager_.AddStateChangeCallback(kKey2, state_change_callback2_);
+ AddProxyReportSuspendReadinessExpectation(kDelayId2, kSuspendId2, true);
+ EXPECT_TRUE(power_manager_.ReportSuspendReadiness(kKey2, kSuspendId2));
+ Mock::VerifyAndClearExpectations(proxy_);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kMem);
-}
+ EXPECT_CALL(*this, SuspendDoneAction1(kSuspendId1));
+ EXPECT_CALL(*this, SuspendDoneAction2(kSuspendId1));
+ OnSuspendDone(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
-TEST_F(PowerManagerTest, AddMultipleSuspendDelayRunMultiple) {
- EXPECT_CALL(*this, SuspendDelayAction1(kSuspendId1));
- EXPECT_CALL(*this, SuspendDelayAction1(kSuspendId2));
- power_manager_.AddSuspendDelayCallback(kKey1, suspend_delay_callback1_);
+ AddProxyUnregisterSuspendDelayExpectation(kDelayId1, true);
+ EXPECT_TRUE(power_manager_.RemoveSuspendDelay(kKey1));
+ Mock::VerifyAndClearExpectations(proxy_);
- EXPECT_CALL(*this, SuspendDelayAction2(kSuspendId1));
- EXPECT_CALL(*this, SuspendDelayAction2(kSuspendId2));
- power_manager_.AddSuspendDelayCallback(kKey2, suspend_delay_callback2_);
+ AddProxyUnregisterSuspendDelayExpectation(kDelayId2, true);
+ EXPECT_TRUE(power_manager_.RemoveSuspendDelay(kKey2));
+ Mock::VerifyAndClearExpectations(proxy_);
- OnSuspendImminent(kSuspendId1);
+ // Start a second suspend attempt with no registered delays.
OnSuspendImminent(kSuspendId2);
+ OnSuspendDone(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
}
-TEST_F(PowerManagerTest, RemoveStateChangeCallback) {
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kOn));
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kMem));
- power_manager_.AddStateChangeCallback(kKey1, state_change_callback1_);
+TEST_F(PowerManagerTest, RegisterSuspendDelayFailure) {
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, false);
+ EXPECT_FALSE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
- EXPECT_CALL(*this, StateChangeAction2(PowerManagerProxyDelegate::kOn));
- EXPECT_CALL(*this, StateChangeAction2(PowerManagerProxyDelegate::kMem))
- .Times(0);
- power_manager_.AddStateChangeCallback(kKey2, state_change_callback2_);
-
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
-
- power_manager_.RemoveStateChangeCallback(kKey2);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kMem);
-
- power_manager_.RemoveStateChangeCallback(kKey1);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
-}
-
-TEST_F(PowerManagerTest, RemoveSuspendDelayCallback) {
- EXPECT_CALL(*this, SuspendDelayAction1(kSuspendId1));
- EXPECT_CALL(*this, SuspendDelayAction1(kSuspendId2));
- power_manager_.AddSuspendDelayCallback(kKey1, suspend_delay_callback1_);
-
- EXPECT_CALL(*this, SuspendDelayAction2(kSuspendId1));
- EXPECT_CALL(*this, SuspendDelayAction2(kSuspendId2)).Times(0);
- power_manager_.AddSuspendDelayCallback(kKey2, suspend_delay_callback2_);
-
+ // No callbacks should be invoked.
OnSuspendImminent(kSuspendId1);
+ OnSuspendDone(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
+}
- power_manager_.RemoveSuspendDelayCallback(kKey2);
- OnSuspendImminent(kSuspendId2);
+TEST_F(PowerManagerTest, ReportSuspendReadinessFailure) {
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
- power_manager_.RemoveSuspendDelayCallback(kKey1);
+ AddProxyReportSuspendReadinessExpectation(kDelayId1, kSuspendId1, false);
+ EXPECT_FALSE(power_manager_.ReportSuspendReadiness(kKey1, kSuspendId1));
+ Mock::VerifyAndClearExpectations(proxy_);
+}
+
+TEST_F(PowerManagerTest, UnregisterSuspendDelayFailure) {
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
+
+ // Make the proxy report failure for unregistering.
+ AddProxyUnregisterSuspendDelayExpectation(kDelayId1, false);
+ EXPECT_FALSE(power_manager_.RemoveSuspendDelay(kKey1));
+ Mock::VerifyAndClearExpectations(proxy_);
+
+ // As a result, the callback should still be invoked.
+ EXPECT_CALL(*this, SuspendImminentAction1(kSuspendId1));
OnSuspendImminent(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
+
+ // Let the unregister request complete successfully now.
+ AddProxyUnregisterSuspendDelayExpectation(kDelayId1, true);
+ EXPECT_TRUE(power_manager_.RemoveSuspendDelay(kKey1));
+ Mock::VerifyAndClearExpectations(proxy_);
}
-typedef PowerManagerTest PowerManagerDeathTest;
+TEST_F(PowerManagerTest, DuplicateKey) {
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
-TEST_F(PowerManagerDeathTest, AddStateChangeCallbackDuplicateKey) {
- power_manager_.AddStateChangeCallback(
- kKey1, Bind(&PowerManagerTest::StateChangeAction1, Unretained(this)));
+ // The new request should be ignored.
+ EXPECT_FALSE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription2, kTimeout,
+ suspend_imminent_callback2_, suspend_done_callback2_));
-#ifndef NDEBUG
- // Adding another callback with the same key is an error and causes a crash in
- // debug mode.
- EXPECT_DEATH(power_manager_.AddStateChangeCallback(
- kKey1, Bind(&PowerManagerTest::StateChangeAction2, Unretained(this))),
- "Inserting duplicate key");
-#else // NDEBUG
- EXPECT_CALL(*this, StateChangeAction2(PowerManagerProxyDelegate::kOn));
- power_manager_.AddStateChangeCallback(
- kKey1, Bind(&PowerManagerTest::StateChangeAction2, Unretained(this)));
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
-#endif // NDEBUG
+ // The first delay's callback should be invoked.
+ EXPECT_CALL(*this, SuspendImminentAction1(kSuspendId1));
+ OnSuspendImminent(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
}
-TEST_F(PowerManagerDeathTest, RemoveStateChangeCallbackUnknownKey) {
- power_manager_.AddStateChangeCallback(
- kKey1, Bind(&PowerManagerTest::StateChangeAction1, Unretained(this)));
-
-#ifndef NDEBUG
- // Attempting to remove a callback key that was not added is an error and
- // crashes in debug mode.
- EXPECT_DEATH(power_manager_.RemoveStateChangeCallback(kKey2),
- "Removing unknown key");
-#else // NDEBUG
- EXPECT_CALL(*this, StateChangeAction1(PowerManagerProxyDelegate::kOn));
-
- // In non-debug mode, removing an unknown key does nothing.
- power_manager_.RemoveStateChangeCallback(kKey2);
- factory_.delegate()->OnPowerStateChanged(PowerManagerProxyDelegate::kOn);
-#endif // NDEBUG
-}
-
-TEST_F(PowerManagerTest, RegisterSuspendDelay) {
- const base::TimeDelta kTimeout = base::TimeDelta::FromMilliseconds(100);
- const char kDescription[] = "description";
- int delay_id = 0;
- EXPECT_CALL(*factory_.proxy(), RegisterSuspendDelay(kTimeout, kDescription,
- &delay_id))
- .WillOnce(Return(true));
- EXPECT_TRUE(power_manager_.RegisterSuspendDelay(kTimeout, kDescription,
- &delay_id));
-}
-
-TEST_F(PowerManagerTest, UnregisterSuspendDelay) {
- const int kDelayId = 123;
- EXPECT_CALL(*factory_.proxy(), UnregisterSuspendDelay(kDelayId))
- .WillOnce(Return(true));
- EXPECT_TRUE(power_manager_.UnregisterSuspendDelay(kDelayId));
-}
-
-TEST_F(PowerManagerTest, ReportSuspendReadiness) {
- const int kDelayId = 678;
- const int kSuspendId = 12345;
- EXPECT_CALL(*factory_.proxy(), ReportSuspendReadiness(kDelayId, kSuspendId))
- .WillOnce(Return(true));
- EXPECT_TRUE(power_manager_.ReportSuspendReadiness(kDelayId, kSuspendId));
+TEST_F(PowerManagerTest, UnknownKey) {
+ EXPECT_FALSE(power_manager_.RemoveSuspendDelay(kKey1));
+ Mock::VerifyAndClearExpectations(proxy_);
}
TEST_F(PowerManagerTest, OnSuspendTimeout) {
- EXPECT_EQ(PowerManagerProxyDelegate::kUnknown, power_manager_.power_state());
- OnSuspendTimeout();
- EXPECT_EQ(PowerManagerProxyDelegate::kOn, power_manager_.power_state());
+ AddProxyRegisterSuspendDelayExpectation(
+ kTimeout, kDescription1, kDelayId1, true);
+ EXPECT_TRUE(power_manager_.AddSuspendDelay(
+ kKey1, kDescription1, kTimeout,
+ suspend_imminent_callback1_, suspend_done_callback1_));
+ Mock::VerifyAndClearExpectations(proxy_);
+
+ EXPECT_CALL(*this, SuspendImminentAction1(kSuspendId1));
+ OnSuspendImminent(kSuspendId1);
+ Mock::VerifyAndClearExpectations(this);
+
+ // After the timeout, the "done" callback should be run.
+ EXPECT_CALL(*this, SuspendDoneAction1(kSuspendId1));
+ OnSuspendTimeout(kSuspendId1);
+ EXPECT_FALSE(power_manager_.suspending());
+ Mock::VerifyAndClearExpectations(this);
}
} // namespace shill
diff --git a/service.cc b/service.cc
index a79837c..895970e 100644
--- a/service.cc
+++ b/service.cc
@@ -884,11 +884,9 @@
SLOG(Service, 2) << "Disconnect while manager stopped ignored.";
return;
}
- // Ignore the event if the power state is not on (e.g., when suspending).
+ // Ignore the event if the system is suspending.
PowerManager *power_manager = manager_->power_manager();
- if (!power_manager ||
- (power_manager->power_state() != PowerManager::kOn &&
- power_manager->power_state() != PowerManager::kUnknown)) {
+ if (!power_manager || power_manager->suspending()) {
SLOG(Service, 2) << "Disconnect in transitional power state ignored.";
return;
}
diff --git a/service_unittest.cc b/service_unittest.cc
index e583b32..c4172d6 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -102,8 +102,8 @@
void SetManagerRunning(bool running) { mock_manager_.running_ = running; }
- void SetPowerState(PowerManager::SuspendState state) {
- power_manager_->power_state_ = state;
+ void SetSuspending(bool suspending) {
+ power_manager_->suspending_ = suspending;
}
void SetExplicitlyDisconnected(bool explicitly) {
@@ -1538,7 +1538,7 @@
// Disconnect while suspending is a non-event.
SetManagerRunning(true);
- SetPowerState(PowerManager::kSuspending);
+ SetSuspending(true);
NoteDisconnectEvent();
EXPECT_TRUE(GetDisconnects()->empty());
EXPECT_TRUE(GetMisconnects()->empty());