Consolidate data storage and notifications in the cloud policy subsystem

chrome/browser/policy/cloud_policy_data.h

Index: chrome/browser/policy/cloud_policy_data.h
diff --git a/chrome/browser/policy/cloud_policy_data.h b/chrome/browser/policy/cloud_policy_data.h
new file mode 100644
index 0000000000000000000000000000000000000000..9fe640bbd7a6ca2a3393129cbd8d2d8413ad9e7d
--- /dev/null
+++ b/chrome/browser/policy/cloud_policy_data.h
@@ -0,0 +1,119 @@
+// Copyright (c) 2011 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_POLICY_CLOUD_POLICY_DATA_H_
+#define CHROME_BROWSER_POLICY_CLOUD_POLICY_DATA_H_
+#pragma once
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"

[Joao da Silva, 2011/07/06 16:45:14]

Nit: scoped_ptr.h not used

[gfeher, 2011/07/07 13:51:00]

Done.

+#include "base/observer_list.h"
+#include "chrome/browser/policy/device_management_backend.h"

[pastarmovj, 2011/07/06 12:11:57]

What do you use from the last include?

[gfeher, 2011/07/06 15:14:20]

I am using, em::DeviceRegisterRequest_Type, but that's actually from another
include. Fixed.

+
+namespace policy {
+
+// Stores in memory all the data that is used in the cloud policy subsystem,
+// and manages notification about changes to these fields.
+// TODO(gfeher): The policy data stored in CloudPolicyCacheBase is currently
+// an exception, move that here.
+class CloudPolicyData {
+ public:
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Notifies observers that the effective token for fetching policy
+    // (device_token_, token_cache_loaded_) has changed.
+    virtual void OnDeviceTokenChanged() = 0;
+
+    // Authentication credentials for talking to the device management service
+    // (gaia_token_) changed.
+    virtual void OnCredentialsChanged() = 0;
+
+    // Called from the destructor of CloudPolicyData.
+    virtual void OnPolicyDataGoingAway() = 0;
+  };
+
+  // Create CloudPolicyData with constants initialized for fetching user
+  // policies.
+  static CloudPolicyData* CreateForUserPolicies();
+
+  // Create CloudPolicyData with constants initialized for fetching device
+  // policies.
+  static CloudPolicyData* CreateForDevicePolicies();
+
+  virtual ~CloudPolicyData();
+
+  // Sets the device token, and token_policy_cache_loaded and sends out
+  // notifications. Also ensures that setting the token should first happen
+  // from the cache.
+  void SetDeviceToken(const std::string& device_token,
+                      bool from_cache);
+
+  // Sets the gaia token and sends out notifications.
+  void SetGaiaToken(const std::string& gaia_token);
+
+  // Clears device and user credentials.
+  void Reset();
+
+  // Only used in tests.
+  void SetupForTesting(const std::string& device_token,
+                       const std::string& device_id,
+                       const std::string& user_name,
+                       const std::string& gaia_token,
+                       bool token_cache_loaded);
+
+  void set_device_id(const std::string& device_id);
+  void set_user_name(const std::string& user_name);
+
+  std::string device_id() const;
+  std::string device_token() const;
+  std::string gaia_token() const;
+  std::string machine_id() const;
+  std::string machine_model() const;

[Joao da Silva, 2011/07/06 16:45:14]

What do you think about returning these as const std::string&?

[gfeher, 2011/07/07 13:51:00]

Well, in theory, CloudPolicyData should outlive all its users, so this should be
safe. On the other hand, I'd prefer not to wire this requirement into the
system, because it seems to be hard to be enforced.
And a random example, let's say I was following it: PrefService::GetString(...)
is also returning copies of strings.

[Joao da Silva, 2011/07/07 16:54:16]

Agree, thanks for making that clear.

+  em::DeviceRegisterRequest_Type policy_register_type() const;
+  std::string policy_type() const;
+  bool token_cache_loaded() const;
+  std::string user_name() const;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  void NotifyCredentialsChanged();
+  void NotifyDeviceTokenChanged();
+
+ private:
+  CloudPolicyData(const em::DeviceRegisterRequest_Type policy_register_type,
+                  const std::string& policy_type,
+                  const std::string& machine_model,
+                  const std::string& machine_id);
+
+  // Data necessary for constructing register requests.
+  std::string gaia_token_;
+  std::string user_name_;
+
+  // Data necessary for constructing policy requests.
+  std::string device_token_;
+
+  // Constants that won't change over the life-time of a cloud policy
+  // subsystem.
+  em::DeviceRegisterRequest_Type policy_register_type_;
+  std::string policy_type_;

[pastarmovj, 2011/07/06 12:11:57]

If these are initialized only by the constructor they can be const right?

[gfeher, 2011/07/06 15:14:20]

Done.

+  std::string machine_model_;
+  std::string machine_id_;
+
+  // Data used for constructiong both register and policy requests.
+  std::string device_id_;
+
+  bool token_cache_loaded_;
+
+  ObserverList<Observer, true> observer_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(CloudPolicyData);
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_POLICY_CLOUD_POLICY_DATA_H_