Removed ConfigurationPolicyType and extended PolicyMap.

chrome/browser/policy/policy_map.h

Index: chrome/browser/policy/policy_map.h
diff --git a/chrome/browser/policy/policy_map.h b/chrome/browser/policy/policy_map.h
index ab0e9f82be8a793a8b35fdd00a775bc55cb3848b..007128580b0a5131e11a6d2fe21d576d3668e870 100644
--- a/chrome/browser/policy/policy_map.h
+++ b/chrome/browser/policy/policy_map.h
@@ -7,47 +7,116 @@
 #pragma once
 
 #include <map>
+#include <set>
+#include <string>
 
 #include "base/values.h"
-#include "policy/configuration_policy_type.h"
 
 namespace policy {
 
 struct PolicyDefinitionList;
 
-// Wrapper class around a std::map<ConfigurationPolicyType, Value*> that
-// properly cleans up after itself when going out of scope.
-// Exposes interesting methods of the underlying std::map.
+// The level of a policy determines its enforceability and whether users can
+// override it or not. The values are listed in increasing order of priority.
+enum PolicyLevel {
+  // RECOMMENDED policies can be overridden by users. They are meant as a
+  // default value configured by admins, that users can customize.
+  POLICY_LEVEL_RECOMMENDED = 0,

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

don't need the = 0

[Joao da Silva, 2012/01/17 13:09:29]

Done.

+
+  // MANDATORY policies must be enforced and users can't circumvent them.
+  POLICY_LEVEL_MANDATORY,
+};
+
+// Most Chrome policies apply to the "user", but some can apply to the "device",
+// particularly on ChromeOS. The scope of a policy determines where it should
+// be applied. The values are listed in increasing order of priority.

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

I don't think that description matches what the policy scope really is. The
browser vs. device split is really an intrinsic property of the policy itself
and not variable. I rather think of this as flag saying whether the policy is
being applied to the user or the machine. Of course, the device-level policies
we have on CrOS can never be POLICY_SCOPE_USER, since there is no fixed active
user for the machine.

[Joao da Silva, 2012/01/17 13:09:29]

Rewrote the comments. Renaming SCOPE_DEVICE to SCOPE_MACHINE also helps clearing
this up.

+enum PolicyScope {
+  // USER policies apply to sessions of the current user.
+  POLICY_SCOPE_USER = 0,

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

ditto

[Joao da Silva, 2012/01/17 13:09:29]

Done.

+
+  // DEVICE policies apply to any users of the current device.
+  POLICY_SCOPE_DEVICE,
+

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

stray newline

[Joao da Silva, 2012/01/17 13:09:29]

Done.

+};
+
+// A mapping of policy names to policy values for a given policy namespace.
 class PolicyMap {
  public:
-  typedef std::map<ConfigurationPolicyType, Value*> PolicyMapType;
+  // Each policy maps to an Entry which keeps the policy value as well as other
+  // relevant data about the policy.
+  struct Entry {
+    PolicyLevel level;
+    PolicyScope scope;
+    Value* value;
+
+    // Returns true if |this| has higher priority than |other|.
+    bool has_higher_priority_than(const Entry& other) const;
+
+    // Returns true if |this| equals |other|.
+    bool equals(const Entry& other) const;

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

Should be Equals?

[Joao da Silva, 2012/01/17 13:09:29]

Done.

+  };
+
+  typedef std::map<std::string, Entry> PolicyMapType;
   typedef PolicyMapType::const_iterator const_iterator;
 
   PolicyMap();
   virtual ~PolicyMap();
 
-  // Returns a weak reference to the value currently stored for key |policy|.
-  // Ownership is retained by PolicyMap; callers should use Value::DeepCopy
-  // if they need a copy that they own themselves.
-  // Returns NULL if the map does not contain a value for |policy|.
-  const Value* Get(ConfigurationPolicyType policy) const;
+  // Returns a weak reference to the entry currently stored for key |policy|,
+  // or NULL if not found. Ownership is retained by the PolicyMap.
+  const Entry* Get(const std::string& policy) const;
+
+  // Returns a weak reference to the value currently stored for key |policy|,
+  // or NULL if not found. Ownership is retained by the PolicyMap.
+  // This is equivalent to Get(policy)->value, when it doesn't return NULL.
+  const Value* GetValue(const std::string& policy) const;
+
   // Takes ownership of |value|. Overwrites any existing value stored in the
   // map for the key |policy|.
-  void Set(ConfigurationPolicyType policy, Value* value);
-  void Erase(ConfigurationPolicyType policy);
+  void Set(const std::string& policy,
+           PolicyLevel level,
+           PolicyScope scope,
+           Value* value);
 
+  // Erase the given |policy|, if it exists in this map.
+  void Erase(const std::string& policy);
+
+  // Swaps the internal representation of |this| with |other|.
   void Swap(PolicyMap* other);
+
+  // |this| becomes a copy of |other|. Any existing policies are dropped.
   void CopyFrom(const PolicyMap& other);
 
   // Similar to CopyFrom, but doesn't Clear() |this| before merging, and only
   // merges keys that aren't already contained in |this|.
+
+  // Merges policies from |other| into |this|. Existing policies are only
+  // overridden by those in |other| if they have a higher priority, as defined
+  // by Entry::has_higher_priority_than(). If a policy is contained in both
+  // maps with the same priority, the current value in |this| is preserved.
   void MergeFrom(const PolicyMap& other);
 
   // Loads the values in |policies| into this PolicyMap, mapped to their
   // corresponding policy type. The policies to load, and their types, are
-  // listed in |list|.
+  // listed in |list|. All policies loaded will have |level| and |scope| in
+  // their entries.
   void LoadFrom(const DictionaryValue* policies,
-                const PolicyDefinitionList* list);
+                const PolicyDefinitionList* list,
+                PolicyLevel level,
+                PolicyScope scope);
+
+  // Compares this value map against |other| and stores all key names that have
+  // different values in |differing_keys|. This includes keys that are present
+  // only in one of the maps. |differing_keys| is not cleared before the keys
+  // are added.
+  void GetDifferingKeys(const PolicyMap& other,
+                        std::set<std::string>* differing_keys) const;
+
+  // Removes all policies that don't have the specified |level|. This is a
+  // temporary helper method, until providers aren't split into mandatory and

[Mattias Nissler (ping if slow), 2012/01/17 09:52:45]

had to re-read the sentence. Perhaps: "until mandatory and recommended levels
are served by a single provider"?

[Joao da Silva, 2012/01/17 13:09:29]

Done.

+  // recommended levels.
+  // TODO(joaodasilva): Remove this. http://crbug.com/108999
+  void FilterLevel(PolicyLevel level);
 
   bool Equals(const PolicyMap& other) const;
   bool empty() const;
@@ -58,7 +127,7 @@ class PolicyMap {
   void Clear();
 
  private:
-  // Helper function for Equals(...).
+  // Helper function for Equals().
   static bool MapEntryEquals(const PolicyMapType::value_type& a,
                              const PolicyMapType::value_type& b);