Chromad: Create AuthPolicyCredentialsManager

chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h

+// Copyright 2017 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 CHROME_BROWSER_CHROMEOS_AUTHPOLICY_AUTH_POLICY_CREDENTIALS_MANAGER_H_
+#define CHROME_BROWSER_CHROMEOS_AUTHPOLICY_AUTH_POLICY_CREDENTIALS_MANAGER_H_

[ljusten (tachyonic), 2017/05/03 10:12:17]

Suggest to use 'authpolicy' instead of 'auth_policy' for filenames for
consistency.

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Yes, we don't have consistency now. But it should be auth_policy. So I leave it
as is. And then we'll rename the files later.

[ljusten (tachyonic), 2017/05/19 16:06:13]

Meh. I'd rather rename the classes Authpolicy* than changing everything to
auth_policy.

[Roman Sorokin (ftl), 2017/05/22 12:35:27]

Acknowledged.

+
+#include "base/cancelable_callback.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/dbus/authpolicy/active_directory_info.pb.h"
+#include "chromeos/network/network_state_handler_observer.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/signin/core/account_id/account_id.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+class Profile;
+
+namespace authpolicy {
+class ActiveDirectoryUserStatus;
+}
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}  // namespace base
+

[ljusten (tachyonic), 2017/05/03 10:12:17]

Please add class-level comment.

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Done.

+class AuthPolicyCredentialsManager
+    : public KeyedService,
+      public chromeos::NetworkStateHandlerObserver {
+ public:
+  explicit AuthPolicyCredentialsManager(Profile* profile);
+  ~AuthPolicyCredentialsManager() override;
+
+  // KeyedService overrides.
+  void Shutdown() override;
+
+  // chromeos::NetworkStateHandlerObserver overrides.
+  void DefaultNetworkChanged(const chromeos::NetworkState* network) override;
+  void NetworkConnectionStateChanged(
+      const chromeos::NetworkState* network) override;
+  void OnShuttingDown() override;
+
+ private:
+  // Calls AuthPolicyClient::GetUserStatus method.
+  void GetUserStatus();
+  // See AuthPolicyClient::GetUserStatusCallback.
+  void OnGetUserStatusCallback(
+      authpolicy::ErrorType error,
+      const authpolicy::ActiveDirectoryUserStatus& user_status);
+  // Post delayed task to call GetUserStatus in the future.
+  void ScheduleGetUserStatus();
+
+  // Add itself as network observer.
+  void StartObserveNetwork();
+  // Remove itself as network observer.
+  void StopObserveNetwork();
+
+  // Update display and given name in case it has changed.
+  void UpdateDisplayAndGivenName(
+      const authpolicy::ActiveDirectoryAccountInfo& account_info);
+
+  // Shows user notification to sign out/sign in.
+  void ShowNotification(int message_id) const;
+
+  // Call GetUserStatus if |network_state| is connected and previous call
+  // failed.

[ljusten (tachyonic), 2017/05/03 10:12:17]

The comment contradicts the implementation. It calls GetUserStatus if the
previous call succeeded.

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Done.

+  void CallGetStatusIfConnected(const chromeos::NetworkState* network_state);

[ljusten (tachyonic), 2017/05/03 10:12:17]

CallGetUserStatusIfConnected?

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Done.

+
+  Profile* profile_;
+  AccountId account_id_;
+  std::string display_name_, given_name_;
+  bool should_call_get_status_again_ = false;
+  authpolicy::ErrorType last_error_ = authpolicy::ERROR_UNKNOWN;

[ljusten (tachyonic), 2017/05/03 10:12:17]

ERROR_NONE? ERROR_UNKNOWN is an actual error, default state should be clean.

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Not sure about it. Could be the case when network connected between the first
call and response. It won't schedule another call in case last_error would be
ERROR_NONE

[ljusten (tachyonic), 2017/05/19 16:06:13]

With what we discussed offline, this case will be handled and the default error
can be ERROR_NONE.

[Roman Sorokin (ftl), 2017/05/22 12:35:27]

Done.

+  base::CancelableClosure scheduled_get_user_status_call_;
+
+  base::WeakPtrFactory<AuthPolicyCredentialsManager> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(AuthPolicyCredentialsManager);
+};
+

[ljusten (tachyonic), 2017/05/03 10:12:17]

Please add class-level comment.

[Roman Sorokin (ftl), 2017/05/19 12:13:48]

Done.

+class AuthPolicyCredentialsManagerFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static AuthPolicyCredentialsManagerFactory* GetInstance();
+
+  static void BuildForProfileIfActiveDirectory(Profile* profile);
+
+ private:
+  friend struct base::DefaultSingletonTraits<
+      AuthPolicyCredentialsManagerFactory>;
+
+  AuthPolicyCredentialsManagerFactory();
+  ~AuthPolicyCredentialsManagerFactory() override;
+
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(AuthPolicyCredentialsManagerFactory);
+};
+
+#endif  // CHROME_BROWSER_CHROMEOS_AUTHPOLICY_AUTH_POLICY_CREDENTIALS_MANAGER_H_