Index: chromeos/dbus/session_manager_client.cc |
diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc |
index 9e87be84e0a371296c29620fa4930b9e484d6049..e890fa1d41e2f7c206071ce352c585357dc93468 100644 |
--- a/chromeos/dbus/session_manager_client.cc |
+++ b/chromeos/dbus/session_manager_client.cc |
@@ -15,6 +15,7 @@ |
#include "base/files/file_util.h" |
#include "base/location.h" |
#include "base/macros.h" |
+#include "base/metrics/histogram_macros.h" |
#include "base/path_service.h" |
#include "base/strings/string_number_conversions.h" |
#include "base/strings/string_util.h" |
@@ -74,6 +75,13 @@ void StoreFile(const base::FilePath& path, const std::string& data) { |
} |
} |
+// Helper to notify the callback with SUCCESS, to be used by the stub. |
+void NotifyOnRetrievePolicySuccess( |
+ const SessionManagerClient::RetrievePolicyCallback& callback, |
+ const std::string& protobuf) { |
+ callback.Run(protobuf, SessionManagerClient::SUCCESS); |
+} |
+ |
} // namespace |
// The SessionManagerClient implementation used in production. |
@@ -202,13 +210,13 @@ class SessionManagerClientImpl : public SessionManagerClient { |
void RetrieveDevicePolicy(const RetrievePolicyCallback& callback) override { |
dbus::MethodCall method_call(login_manager::kSessionManagerInterface, |
login_manager::kSessionManagerRetrievePolicy); |
- session_manager_proxy_->CallMethod( |
- &method_call, |
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, |
- base::Bind(&SessionManagerClientImpl::OnRetrievePolicy, |
+ session_manager_proxy_->CallMethodWithErrorCallback( |
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, |
+ base::Bind(&SessionManagerClientImpl::OnRetrievePolicySuccess, |
weak_ptr_factory_.GetWeakPtr(), |
- login_manager::kSessionManagerRetrievePolicy, |
- callback)); |
+ login_manager::kSessionManagerRetrievePolicy, callback), |
+ base::Bind(&SessionManagerClientImpl::OnRetrievePolicyError, |
+ weak_ptr_factory_.GetWeakPtr(), callback)); |
} |
std::string BlockingRetrieveDevicePolicy() override { |
@@ -250,8 +258,7 @@ class SessionManagerClientImpl : public SessionManagerClient { |
const RetrievePolicyCallback& callback) override { |
CallRetrievePolicyByUsername( |
login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy, |
- account_name, |
- callback); |
+ account_name, callback); |
} |
std::string BlockingRetrieveDeviceLocalAccountPolicy( |
@@ -495,14 +502,12 @@ class SessionManagerClientImpl : public SessionManagerClient { |
method_name); |
dbus::MessageWriter writer(&method_call); |
writer.AppendString(account_id); |
- session_manager_proxy_->CallMethod( |
- &method_call, |
- dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, |
- base::Bind( |
- &SessionManagerClientImpl::OnRetrievePolicy, |
- weak_ptr_factory_.GetWeakPtr(), |
- method_name, |
- callback)); |
+ session_manager_proxy_->CallMethodWithErrorCallback( |
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, |
+ base::Bind(&SessionManagerClientImpl::OnRetrievePolicySuccess, |
+ weak_ptr_factory_.GetWeakPtr(), method_name, callback), |
+ base::Bind(&SessionManagerClientImpl::OnRetrievePolicyError, |
+ weak_ptr_factory_.GetWeakPtr(), callback)); |
} |
void CallStorePolicyByUsername(const std::string& method_name, |
@@ -615,12 +620,29 @@ class SessionManagerClientImpl : public SessionManagerClient { |
// Called when kSessionManagerRetrievePolicy or |
// kSessionManagerRetrievePolicyForUser method is complete. |
emaxx
2017/04/18 15:03:17
nit: Maybe change "complete" to "successfully comp
igorcov
2017/04/20 14:52:29
Done.
|
- void OnRetrievePolicy(const std::string& method_name, |
- const RetrievePolicyCallback& callback, |
- dbus::Response* response) { |
+ void OnRetrievePolicySuccess(const std::string& method_name, |
+ const RetrievePolicyCallback& callback, |
+ dbus::Response* response) { |
std::string serialized_proto; |
ExtractString(method_name, response, &serialized_proto); |
- callback.Run(serialized_proto); |
+ callback.Run(serialized_proto, SUCCESS); |
+ |
+ UMA_HISTOGRAM_ENUMERATION("Enterprise.RetrievePolicyResponse", SUCCESS, |
+ RETRIEVE_POLICY_RESPONSE_TYPE_COUNT); |
+ } |
+ |
+ void OnRetrievePolicyError(const RetrievePolicyCallback& callback, |
emaxx
2017/04/18 15:03:17
nit: Please add a comment for this method.
igorcov
2017/04/20 14:52:29
Done.
|
+ dbus::ErrorResponse* response) { |
+ RetrievePolicyResponseType response_type = OTHER_ERROR; |
+ if (response->GetErrorName() == |
+ login_manager::dbus_error::kSessionDoesNotExist) { |
+ response_type = SESSION_DOES_NOT_EXIST; |
+ } |
+ callback.Run(std::string(), response_type); |
+ |
+ UMA_HISTOGRAM_ENUMERATION("Enterprise.RetrievePolicyResponse", |
+ response_type, |
+ RETRIEVE_POLICY_RESPONSE_TYPE_COUNT); |
} |
// Called when kSessionManagerStorePolicy or kSessionManagerStorePolicyForUser |
@@ -861,17 +883,19 @@ class SessionManagerClientStubImpl : public SessionManagerClient { |
void RetrieveDevicePolicy(const RetrievePolicyCallback& callback) override { |
base::FilePath owner_key_path; |
if (!PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_path)) { |
- callback.Run(""); |
+ callback.Run("", SUCCESS); |
return; |
} |
base::FilePath device_policy_path = |
owner_key_path.DirName().AppendASCII(kStubDevicePolicyFile); |
base::PostTaskWithTraitsAndReplyWithResult( |
- FROM_HERE, base::TaskTraits() |
- .WithShutdownBehavior( |
- base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN) |
- .MayBlock(), |
- base::Bind(&GetFileContent, device_policy_path), callback); |
+ FROM_HERE, |
+ base::TaskTraits() |
+ .WithShutdownBehavior( |
+ base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN) |
+ .MayBlock(), |
+ base::Bind(&GetFileContent, device_policy_path), |
+ base::Bind(&NotifyOnRetrievePolicySuccess, callback)); |
} |
std::string BlockingRetrieveDevicePolicy() override { |
base::FilePath owner_key_path; |
@@ -892,7 +916,7 @@ class SessionManagerClientStubImpl : public SessionManagerClient { |
.MayBlock(), |
base::Bind(&GetFileContent, |
GetUserFilePath(cryptohome_id, kStubPolicyFile)), |
- callback); |
+ base::Bind(&NotifyOnRetrievePolicySuccess, callback)); |
} |
std::string BlockingRetrievePolicyForUser( |
const cryptohome::Identification& cryptohome_id) override { |