Abandon user sign in when policy is retrieved before session started

chromeos/dbus/session_manager_client.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_DBUS_SESSION_MANAGER_CLIENT_H_
 #define CHROMEOS_DBUS_SESSION_MANAGER_CLIENT_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 10 matching lines @@
 
 namespace cryptohome {
 class Identification;
 }
 
 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 be inserted immediately before COUNT.
+  enum class RetrievePolicyResponseType {
+    OTHER_ERROR = 0,             // Other type of error while retrieving policy
+                                 // data (e.g. D-Bus timeout).

[Alexei Svitkine (slow), 2017/04/25 14:41:41]

Nit: For longer comments like these, just put them above the enums as line
comments.

[igorcov, 2017/04/26 11:34:31]

Done.

+    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:
     virtual ~Observer() {}
 
     // Called when the owner key is set.
     virtual void OwnerKeySet(bool success) {}
 
     // Called when the property change is complete.
     virtual void PropertyChangeComplete(bool success) {}
@@ skipping 95 matching lines @@
   // Enumerates active user sessions. Usually Chrome naturally keeps track of
   // active users when they are added into current session. When Chrome is
   // restarted after crash by session_manager it only receives cryptohome id and
   // user_id_hash for one user. This method is used to retrieve list of all
   // active users.
   virtual void RetrieveActiveSessions(
       const ActiveSessionsCallback& callback) = 0;
 
   // 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_out) = 0;
 
   // 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
   // provided callback.
   virtual void RetrievePolicyForUser(
       const cryptohome::Identification& cryptohome_id,
       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_out) = 0;
 
   // Fetches the policy blob associated with the specified device-local account
   // from session manager.  |callback| is invoked up on completion.
   virtual void RetrieveDeviceLocalAccountPolicy(
       const std::string& account_id,
       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_out) = 0;
 
   // Used for StoreDevicePolicy, StorePolicyForUser and
   // StoreDeviceLocalAccountPolicy. Takes a boolean indicating whether the
   // operation was successful or not.
   using StorePolicyCallback = base::Callback<void(bool success)>;
 
   // Attempts to asynchronously store |policy_blob| as device policy.  Upon
   // completion of the store attempt, we will call callback.
   virtual void StoreDevicePolicy(const std::string& policy_blob,
                                  const StorePolicyCallback& callback) = 0;
@@ skipping 108 matching lines @@
   // Create() should be used instead.
   SessionManagerClient();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SessionManagerClient);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_SESSION_MANAGER_CLIENT_H_