Abandon user sign in when policy is retrieved before session started

chromeos/dbus/session_manager_client.h

Index: chromeos/dbus/session_manager_client.h
diff --git a/chromeos/dbus/session_manager_client.h b/chromeos/dbus/session_manager_client.h
index 042c4cc8c4f58491077faaeab951a5a08619e479..666ed46b4f8cc8814cc31f66f18b8377a0e129bf 100644
--- a/chromeos/dbus/session_manager_client.h
+++ b/chromeos/dbus/session_manager_client.h
@@ -28,6 +28,23 @@ namespace chromeos {
 // SessionManagerClient is used to communicate with the session manager.
 class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
  public:
+  // The result type received from session manager on request to retrieve the
+  // policy. Used to define the buckets for an enumerated UMA histogram.
+  // Hence,
+  //   (a) existing enumerated constants should never be deleted or reordered.
+  //   (b) new constants should only be appended at the end of the enumeration.

[Daniel Erat, 2017/04/24 19:50:13]

nit: "new constants should be inserted immediately before COUNT."

[igorcov, 2017/04/25 09:18:42]

Done.

+  enum class RetrievePolicyResponseType {
+    OTHER_ERROR = 0,             // Other type of error while retrieving policy
+                                 // data (e.g. D-Bus timeout).
+    SUCCESS = 1,                 // The policy was retrieved successfully.
+    SESSION_DOES_NOT_EXIST = 2,  // Retrieve policy request issued before
+                                 // session started.
+    POLICY_ENCODE_ERROR = 3,     // Session manager failed to encode the policy
+                                 // data.
+    COUNT                        // Has to be the last value of enumeration.
+                                 // Used for UMA.
+  };
+
   // Interface for observing changes from the session manager.
   class Observer {
    public:
@@ -143,23 +160,26 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
 
   // Used for RetrieveDevicePolicy, RetrievePolicyForUser and
   // RetrieveDeviceLocalAccountPolicy. Takes a serialized protocol buffer as
-  // string.  Upon success, we will pass a protobuf to the callback.  On
-  // failure, we will pass "".
+  // string.  Upon success, we will pass a protobuf and SUCCESS |response_type|
+  // to the callback. On failure, we will pass "" and the details of error type
+  // in |response_type|.
   using RetrievePolicyCallback =
-      base::Callback<void(const std::string& protobuf)>;
+      base::Callback<void(const std::string& protobuf,
+                          RetrievePolicyResponseType response_type)>;
 
   // Fetches the device policy blob stored by the session manager.  Upon
   // completion of the retrieve attempt, we will call the provided callback.
   virtual void RetrieveDevicePolicy(const RetrievePolicyCallback& callback) = 0;
 
   // Same as RetrieveDevicePolicy() but blocks until a reply is received, and
-  // returns the policy synchronously. Returns an empty string if the method
-  // call fails.
+  // populates the policy synchronously. Returns SUCCESS when successful, or
+  // the corresponding error from enum in case of a failure.
   // This may only be called in situations where blocking the UI thread is
   // considered acceptable (e.g. restarting the browser after a crash or after
   // a flag change).
   // TODO: Get rid of blocking calls (crbug.com/160522).
-  virtual std::string BlockingRetrieveDevicePolicy() = 0;
+  virtual RetrievePolicyResponseType BlockingRetrieveDevicePolicy(
+      std::string* policy) = 0;

[Daniel Erat, 2017/04/24 19:50:13]

policy_out

[igorcov, 2017/04/25 09:18:42]

Done.

 
   // Fetches the user policy blob stored by the session manager for the given
   // |cryptohome_id|. Upon completion of the retrieve attempt, we will call the
@@ -169,14 +189,15 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
       const RetrievePolicyCallback& callback) = 0;
 
   // Same as RetrievePolicyForUser() but blocks until a reply is received, and
-  // returns the policy synchronously. Returns an empty string if the method
-  // call fails.
+  // populates the policy synchronously. Returns SUCCESS when successful, or
+  // the corresponding error from enum in case of a failure.
   // This may only be called in situations where blocking the UI thread is
   // considered acceptable (e.g. restarting the browser after a crash or after
   // a flag change).
   // TODO: Get rid of blocking calls (crbug.com/160522).
-  virtual std::string BlockingRetrievePolicyForUser(
-      const cryptohome::Identification& cryptohome_id) = 0;
+  virtual RetrievePolicyResponseType BlockingRetrievePolicyForUser(
+      const cryptohome::Identification& cryptohome_id,
+      std::string* policy) = 0;

[Daniel Erat, 2017/04/24 19:50:13]

policy_out

[igorcov, 2017/04/25 09:18:42]

Done.

 
   // Fetches the policy blob associated with the specified device-local account
   // from session manager.  |callback| is invoked up on completion.
@@ -185,14 +206,15 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
       const RetrievePolicyCallback& callback) = 0;
 
   // Same as RetrieveDeviceLocalAccountPolicy() but blocks until a reply is
-  // received, and returns the policy synchronously.
-  // Returns an empty string if the method call fails.
+  // received, and populates the policy synchronously. Returns SUCCESS when
+  // successful, or the corresponding error from enum in case of a failure.
   // This may only be called in situations where blocking the UI thread is
   // considered acceptable (e.g. restarting the browser after a crash or after
   // a flag change).
   // TODO: Get rid of blocking calls (crbug.com/160522).
-  virtual std::string BlockingRetrieveDeviceLocalAccountPolicy(
-      const std::string& account_id) = 0;
+  virtual RetrievePolicyResponseType BlockingRetrieveDeviceLocalAccountPolicy(
+      const std::string& account_id,
+      std::string* policy) = 0;

[Daniel Erat, 2017/04/24 19:50:13]

policy_out

[igorcov, 2017/04/25 09:18:42]

Done.

 
   // Used for StoreDevicePolicy, StorePolicyForUser and
   // StoreDeviceLocalAccountPolicy. Takes a boolean indicating whether the