Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(64)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chromeos/dbus/session_manager_client.cc

Issue 2889683006: Misc refactoring of SessionManagerClient. (Closed)
Patch Set: git cl format. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | dbus/object_proxy.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chromeos/dbus/session_manager_client.cc
diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc
index fa1d9ae72a884c3b05a1a47bbff259de530839d0..34afb1806fb36a7724f727ce134f92f8392342f8 100644
--- a/chromeos/dbus/session_manager_client.cc
+++ b/chromeos/dbus/session_manager_client.cc
@@ -151,12 +151,9 @@ void LogPolicyResponseUma(base::StringPiece method_name,
// The SessionManagerClient implementation used in production.
class SessionManagerClientImpl : public SessionManagerClient {
public:
- SessionManagerClientImpl()
- : session_manager_proxy_(NULL),
- screen_is_locked_(false),
- weak_ptr_factory_(this) {}
+ SessionManagerClientImpl() : weak_ptr_factory_(this) {}
- ~SessionManagerClientImpl() override {}
+ ~SessionManagerClientImpl() override = default;
// SessionManagerClient overrides:
void SetStubDelegate(StubDelegate* delegate) override {
@@ -205,10 +202,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
writer.AppendString(cryptohome_id.id());
writer.AppendString(""); // Unique ID is deprecated
session_manager_proxy_->CallMethod(
- &method_call,
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnStartSession,
- weak_ptr_factory_.GetWeakPtr()));
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+ dbus::ObjectProxy::EmptyResponseCallback());
}
void StopSession() override {
@@ -217,20 +212,13 @@ class SessionManagerClientImpl : public SessionManagerClient {
dbus::MessageWriter writer(&method_call);
writer.AppendString(""); // Unique ID is deprecated
session_manager_proxy_->CallMethod(
- &method_call,
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnStopSession,
- weak_ptr_factory_.GetWeakPtr()));
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+ dbus::ObjectProxy::EmptyResponseCallback());
}
void StartDeviceWipe() override {
- dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
- login_manager::kSessionManagerStartDeviceWipe);
- session_manager_proxy_->CallMethod(
- &method_call,
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnDeviceWipe,
- weak_ptr_factory_.GetWeakPtr()));
+ SimpleMethodCallToSessionManager(
+ login_manager::kSessionManagerStartDeviceWipe);
}
void RequestLockScreen() override {
@@ -347,12 +335,9 @@ class SessionManagerClientImpl : public SessionManagerClient {
reinterpret_cast<const uint8_t*>(policy_blob.data()),
policy_blob.size());
session_manager_proxy_->CallMethod(
- &method_call,
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnStorePolicy,
- weak_ptr_factory_.GetWeakPtr(),
- login_manager::kSessionManagerStorePolicy,
- callback));
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
void StorePolicyForUser(const cryptohome::Identification& cryptohome_id,
@@ -440,9 +425,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
login_manager::kSessionManagerStopArcInstance);
session_manager_proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnArcMethod,
- weak_ptr_factory_.GetWeakPtr(),
- login_manager::kSessionManagerStopArcInstance, callback));
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
void SetArcCpuRestriction(
@@ -455,10 +439,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
writer.AppendUint32(restriction_state);
session_manager_proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnArcMethod,
- weak_ptr_factory_.GetWeakPtr(),
- login_manager::kSessionManagerSetArcCpuRestriction,
- callback));
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
void EmitArcBooted(const cryptohome::Identification& cryptohome_id,
@@ -469,9 +451,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
writer.AppendString(cryptohome_id.id());
session_manager_proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnArcMethod,
- weak_ptr_factory_.GetWeakPtr(),
- login_manager::kSessionManagerEmitArcBooted, callback));
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
void GetArcStartTime(const GetArcStartTimeCallback& callback) override {
@@ -493,9 +474,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
writer.AppendString(cryptohome_id.id());
session_manager_proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(&SessionManagerClientImpl::OnArcMethod,
- weak_ptr_factory_.GetWeakPtr(),
- login_manager::kSessionManagerRemoveArcData, callback));
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
protected:
@@ -556,6 +536,15 @@ class SessionManagerClientImpl : public SessionManagerClient {
dbus::ObjectProxy::EmptyResponseCallback());
}
+ // Calls given callback (if non-null), with the |success| boolean
+ // representing the dbus call was successful or not.
+ void OnNoOutputParamResponse(
+ const base::Callback<void(bool success)>& callback,
+ dbus::Response* response) {
+ if (!callback.is_null())
+ callback.Run(response != nullptr);
+ }
+
// Helper for RetrieveDeviceLocalAccountPolicy and RetrievePolicyForUser.
void CallRetrievePolicyByUsername(const std::string& method_name,
const std::string& account_id,
@@ -612,13 +601,9 @@ class SessionManagerClientImpl : public SessionManagerClient {
reinterpret_cast<const uint8_t*>(policy_blob.data()),
policy_blob.size());
session_manager_proxy_->CallMethod(
- &method_call,
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
- base::Bind(
- &SessionManagerClientImpl::OnStorePolicy,
- weak_ptr_factory_.GetWeakPtr(),
- method_name,
- callback));
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+ base::Bind(&SessionManagerClientImpl::OnNoOutputParamResponse,
+ weak_ptr_factory_.GetWeakPtr(), callback));
}
// Called when kSessionManagerRestartJob method is complete.
@@ -631,27 +616,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
: DBUS_METHOD_CALL_FAILURE);
}
- // Called when kSessionManagerStartSession method is complete.
- void OnStartSession(dbus::Response* response) {
- LOG_IF(ERROR, !response)
- << "Failed to call "
- << login_manager::kSessionManagerStartSession;
- }
-
- // Called when kSessionManagerStopSession method is complete.
- void OnStopSession(dbus::Response* response) {
- LOG_IF(ERROR, !response)
- << "Failed to call "
- << login_manager::kSessionManagerStopSession;
- }
-
- // Called when kSessionManagerStopSession method is complete.
- void OnDeviceWipe(dbus::Response* response) {
- LOG_IF(ERROR, !response)
- << "Failed to call "
- << login_manager::kSessionManagerStartDeviceWipe;
- }
-
// Called when kSessionManagerRetrieveActiveSessions method is complete.
void OnRetrieveActiveSessions(const std::string& method_name,
const ActiveSessionsCallback& callback,
@@ -659,20 +623,19 @@ class SessionManagerClientImpl : public SessionManagerClient {
ActiveSessionsMap sessions;
bool success = false;
if (!response) {
- LOG(ERROR) << "Failed to call " << method_name;
callback.Run(sessions, success);
return;
}
dbus::MessageReader reader(response);
- dbus::MessageReader array_reader(NULL);
+ dbus::MessageReader array_reader(nullptr);
if (!reader.PopArray(&array_reader)) {
LOG(ERROR) << method_name << " response is incorrect: "
<< response->ToString();
} else {
while (array_reader.HasMoreData()) {
- dbus::MessageReader dict_entry_reader(NULL);
+ dbus::MessageReader dict_entry_reader(nullptr);
std::string key;
std::string value;
if (!array_reader.PopDictEntry(&dict_entry_reader) ||
@@ -697,7 +660,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
return;
}
dbus::MessageReader reader(response);
- const uint8_t* values = NULL;
+ const uint8_t* values = nullptr;
size_t length = 0;
if (!reader.PopArrayOfBytes(&values, &length)) {
LOG(ERROR) << "Invalid response: " << response->ToString();
@@ -731,16 +694,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
LogPolicyResponseUma(method_name, response_type);
}
- // Called when kSessionManagerStorePolicy or kSessionManagerStorePolicyForUser
- // method is complete.
- void OnStorePolicy(const std::string& method_name,
- const StorePolicyCallback& callback,
- dbus::Response* response) {
- bool success = response != nullptr;
- LOG_IF(ERROR, !success) << "Failed to call " << method_name;
- callback.Run(success);
- }
-
// Called when the owner key set signal is received.
void OwnerKeySetReceived(dbus::Signal* signal) {
dbus::MessageReader reader(signal);
@@ -803,26 +756,22 @@ class SessionManagerClientImpl : public SessionManagerClient {
void OnGetServerBackedStateKeys(const StateKeysCallback& callback,
dbus::Response* response) {
std::vector<std::string> state_keys;
- if (!response) {
- LOG(ERROR) << "Failed to call "
- << login_manager::kSessionManagerGetServerBackedStateKeys;
- } else {
+ if (response) {
dbus::MessageReader reader(response);
- dbus::MessageReader array_reader(NULL);
+ dbus::MessageReader array_reader(nullptr);
if (!reader.PopArray(&array_reader)) {
LOG(ERROR) << "Bad response: " << response->ToString();
} else {
while (array_reader.HasMoreData()) {
- const uint8_t* data = NULL;
+ const uint8_t* data = nullptr;
size_t size = 0;
if (!array_reader.PopArrayOfBytes(&data, &size)) {
LOG(ERROR) << "Bad response: " << response->ToString();
state_keys.clear();
break;
}
- state_keys.push_back(
- std::string(reinterpret_cast<const char*>(data), size));
+ state_keys.emplace_back(reinterpret_cast<const char*>(data), size);
}
}
}
@@ -835,10 +784,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
void OnCheckArcAvailability(const ArcCallback& callback,
dbus::Response* response) {
bool available = false;
- if (!response) {
- LOG(ERROR) << "Failed to call "
- << login_manager::kSessionManagerCheckArcAvailability;
- } else {
+ if (response) {
dbus::MessageReader reader(response);
if (!reader.PopBool(&available))
LOG(ERROR) << "Invalid response: " << response->ToString();
@@ -851,10 +797,7 @@ class SessionManagerClientImpl : public SessionManagerClient {
dbus::Response* response) {
bool success = false;
base::TimeTicks arc_start_time;
- if (!response) {
- LOG(ERROR) << "Failed to call "
- << login_manager::kSessionManagerGetArcStartTimeTicks;
- } else {
+ if (response) {
dbus::MessageReader reader(response);
int64_t ticks = 0;
if (reader.PopInt64(&ticks)) {
@@ -867,22 +810,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
callback.Run(success, arc_start_time);
}
- // Called when kSessionManagerStartArcInstance or
- // kSessionManagerStopArcInstance methods complete.
- void OnArcMethod(const std::string& method_name,
- const ArcCallback& callback,
- dbus::Response* response) {
- bool success = false;
- if (!response) {
- LOG(ERROR) << "Failed to call " << method_name;
- } else {
- success = true;
- }
-
- if (!callback.is_null())
- callback.Run(success);
- }
-
void OnStartArcInstanceSucceeded(const StartArcInstanceCallback& callback,
dbus::Response* response) {
if (!callback.is_null())
@@ -901,12 +828,12 @@ class SessionManagerClientImpl : public SessionManagerClient {
}
}
- dbus::ObjectProxy* session_manager_proxy_;
+ dbus::ObjectProxy* session_manager_proxy_ = nullptr;
std::unique_ptr<BlockingMethodCaller> blocking_method_caller_;
base::ObserverList<Observer> observers_;
// Most recent screen-lock state received from session_manager.
- bool screen_is_locked_;
+ bool screen_is_locked_ = false;
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
@@ -919,8 +846,8 @@ class SessionManagerClientImpl : public SessionManagerClient {
// which does nothing.
class SessionManagerClientStubImpl : public SessionManagerClient {
public:
- SessionManagerClientStubImpl() : delegate_(NULL), screen_is_locked_(false) {}
- ~SessionManagerClientStubImpl() override {}
+ SessionManagerClientStubImpl() = default;
+ ~SessionManagerClientStubImpl() override = default;
// SessionManagerClient overrides
void Init(dbus::Bus* bus) override {}
@@ -1140,10 +1067,10 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
}
private:
- StubDelegate* delegate_; // Weak pointer; may be NULL.
+ StubDelegate* delegate_ = nullptr; // Weak pointer; may be nullptr.
base::ObserverList<Observer> observers_;
std::string device_policy_;
- bool screen_is_locked_;
+ bool screen_is_locked_ = false;
DISALLOW_COPY_AND_ASSIGN(SessionManagerClientStubImpl);
};
« no previous file with comments | « no previous file | dbus/object_proxy.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698