shill: Clean up HookTable implementation.
This CL cleans up the HookTable implementation as follows:
1. Replace base::Callback<void(const Error &)> with shill::ResultCallback.
2. Make HookTable::AllActionsComplete() a const method.
3. Add verbose logging to HookTable::Add() and HookTable::Remove().
4. Remove iterator-based for-loop with range-based for-loop.
5. Simplify HookTable::Remove() by calling std::map::erase() to remove
an entry by key.
6. Fix some typos.
7. Rename some callback variable names to be more consistent with other
shill code.
8. Use std::map::emplace() instead of std::map::insert().
BUG=None
TEST=Tested the following:
1. `FEATURES=test emerge-$BOARD platform2`
2. Verify that shill executes the termination action upon suspend on a
DUT with a cellular device.
Change-Id: I5bec3c53ba7cdb9ac5a3f7a5d1f3d765e6402865
Reviewed-on: https://chromium-review.googlesource.com/203191
Reviewed-by: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>
diff --git a/hook_table.cc b/hook_table.cc
index 8d5f3ab..73c1aca 100644
--- a/hook_table.cc
+++ b/hook_table.cc
@@ -16,9 +16,7 @@
#include "shill/logging.h"
using base::Bind;
-using base::Callback;
using base::Closure;
-using base::ConstRef;
using base::Unretained;
using std::list;
using std::string;
@@ -28,21 +26,19 @@
HookTable::HookTable(EventDispatcher *event_dispatcher)
: event_dispatcher_(event_dispatcher) {}
-void HookTable::Add(const string &name, const Closure &start) {
+void HookTable::Add(const string &name, const Closure &start_callback) {
+ SLOG(Manager, 2) << __func__ << ": " << name;
Remove(name);
- hook_table_.insert(
- HookTableMap::value_type(name, HookAction(start)));
+ hook_table_.emplace(name, HookAction(start_callback));
}
HookTable::~HookTable() {
- timeout_cb_.Cancel();
+ timeout_callback_.Cancel();
}
void HookTable::Remove(const std::string &name) {
- HookTableMap::iterator it = hook_table_.find(name);
- if (it != hook_table_.end()) {
- hook_table_.erase(it);
- }
+ SLOG(Manager, 2) << __func__ << ": " << name;
+ hook_table_.erase(name);
}
void HookTable::ActionComplete(const std::string &name) {
@@ -54,23 +50,22 @@
action->completed = true;
}
}
- if (AllActionsComplete() && !done_cb_.is_null()) {
- timeout_cb_.Cancel();
- done_cb_.Run(Error(Error::kSuccess));
- done_cb_.Reset();
+ if (AllActionsComplete() && !done_callback_.is_null()) {
+ timeout_callback_.Cancel();
+ done_callback_.Run(Error(Error::kSuccess));
+ done_callback_.Reset();
}
}
-void HookTable::Run(int timeout_ms,
- const Callback<void(const Error &)> &done) {
+void HookTable::Run(int timeout_ms, const ResultCallback &done) {
SLOG(Manager, 2) << __func__;
if (hook_table_.empty()) {
done.Run(Error(Error::kSuccess));
return;
}
- done_cb_ = done;
- timeout_cb_.Reset(Bind(&HookTable::ActionsTimedOut, Unretained(this)));
- event_dispatcher_->PostDelayedTask(timeout_cb_.callback(), timeout_ms);
+ done_callback_ = done;
+ timeout_callback_.Reset(Bind(&HookTable::ActionsTimedOut, Unretained(this)));
+ event_dispatcher_->PostDelayedTask(timeout_callback_.callback(), timeout_ms);
// Mark all actions as having started before we execute any actions.
// Otherwise, if the first action completes inline, its call to
@@ -84,7 +79,7 @@
list<Closure> action_start_callbacks;
for (auto &hook_entry : hook_table_) {
HookAction *action = &hook_entry.second;
- action_start_callbacks.push_back(action->start);
+ action_start_callbacks.push_back(action->start_callback);
action->started = true;
action->completed = false;
}
@@ -94,11 +89,10 @@
}
}
-bool HookTable::AllActionsComplete() {
+bool HookTable::AllActionsComplete() const {
SLOG(Manager, 2) << __func__;
- for (HookTableMap::const_iterator it = hook_table_.begin();
- it != hook_table_.end(); ++it) {
- const HookAction &action = it->second;
+ for (const auto &hook_entry : hook_table_) {
+ const HookAction &action = hook_entry.second;
if (action.started && !action.completed) {
return false;
}
@@ -107,8 +101,8 @@
}
void HookTable::ActionsTimedOut() {
- done_cb_.Run(Error(Error::kOperationTimeout));
- done_cb_.Reset();
+ done_callback_.Run(Error(Error::kOperationTimeout));
+ done_callback_.Reset();
}
} // namespace shill