Removed the PolicyDefinitionList.

chrome/browser/policy/policy_prefs_browsertest.cc

Index: chrome/browser/policy/policy_prefs_browsertest.cc
diff --git a/chrome/browser/policy/policy_prefs_browsertest.cc b/chrome/browser/policy/policy_prefs_browsertest.cc
index 8e4f5aa0082cb77b53d688cdfc7c6856af79f03f..08c520773b68aaffd9e51ca7574f4444a2befbb0 100644
--- a/chrome/browser/policy/policy_prefs_browsertest.cc
+++ b/chrome/browser/policy/policy_prefs_browsertest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <algorithm>
+#include <iterator>
 #include <map>
 #include <sstream>
 #include <string>
@@ -30,6 +31,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/policy/core/common/schema.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "policy/policy_constants.h"

[bartfab (slow), 2013/11/05 18:18:33]

Nit: policy/policy_constants.h now implicitly pulls in "callback_forward.h", so
all implementation files using it must #include "base/callback.h"

[Joao da Silva, 2013/11/07 20:27:27]

The interface is not actually used, so forward-decl is enough

@@ -37,7 +39,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
-using testing::AnyNumber;
 using testing::Return;
 using testing::_;
 
@@ -210,12 +211,16 @@ class PolicyTestCases {
       ADD_FAILURE() << "Error parsing policy_test_cases.json: " << error_string;
       return;
     }
-    const PolicyDefinitionList* list = GetChromePolicyDefinitionList();
-    for (const PolicyDefinitionList::Entry* policy = list->begin;
-         policy != list->end; ++policy) {
-      PolicyTestCase* policy_test_case = GetPolicyTestCase(dict, policy->name);
+    Schema chrome_schema = Schema::Wrap(GetChromeSchemaData());
+    if (!chrome_schema.valid()) {
+      ADD_FAILURE();
+      return;
+    }
+    for (Schema::Iterator it = chrome_schema.GetPropertiesIterator();
+         !it.IsAtEnd(); it.Advance()) {
+      PolicyTestCase* policy_test_case = GetPolicyTestCase(dict, it.key());
       if (policy_test_case)
-        (*policy_test_cases_)[policy->name] = policy_test_case;
+        (*policy_test_cases_)[it.key()] = policy_test_case;
     }
   }
 
@@ -380,11 +385,88 @@ void VerifyControlledSettingIndicators(Browser* browser,
 
 }  // namespace
 
+// A forward iterator that iterates over the Chrome policy names, using the
+// Chrome schema data.
+class TestCaseIterator
+    : public std::iterator<std::forward_iterator_tag, const char*> {
+ public:
+  static TestCaseIterator GetBegin() {
+    Schema chrome_schema = Schema::Wrap(GetChromeSchemaData());
+    CHECK(chrome_schema.valid());

[bartfab (slow), 2013/11/05 18:18:33]

Nit: #include "base/logging.h"

[Joao da Silva, 2013/11/07 20:27:27]

Done.

+    return TestCaseIterator(chrome_schema.GetPropertiesIterator());
+  }
+
+  static TestCaseIterator GetEnd() {
+    return TestCaseIterator();
+  }
+
+  TestCaseIterator() {}
+
+  explicit TestCaseIterator(const Schema::Iterator& it)
+      : it_(it.IsAtEnd() ? NULL : new Schema::Iterator(it)) {}
+
+  TestCaseIterator(const TestCaseIterator& other)
+      : it_(other.it_ ? new Schema::Iterator(*other.it_) : NULL) {}
+
+  ~TestCaseIterator() {}
+
+  TestCaseIterator& operator=(const TestCaseIterator& other) {
+    it_.reset(other.it_ ? new Schema::Iterator(*other.it_) : NULL);
+    return *this;
+  }
+
+  bool Equals(const TestCaseIterator& other) const {
+    // Assume that both iterators are working with the same Schema; therefore
+    // the keys() returned are the same.

[bartfab (slow), 2013/11/05 18:18:33]

Nit: s/keys()/key()s/

[Joao da Silva, 2013/11/07 20:27:27]

Done.

+    if (!it_ || !other.it_)
+      return !it_ && !other.it_;
+    return it_->key() == other.it_->key();
+  }
+
+  bool operator==(const TestCaseIterator& other) const {
+    return Equals(other);
+  }
+
+  bool operator !=(const TestCaseIterator& other) const {
+    return !Equals(other);
+  }
+
+  const char* operator*() const {
+    if (!it_) {
+      NOTREACHED();
+      return NULL;
+    }
+    return it_->key();
+  }
+
+  TestCaseIterator& Advance() {
+    if (it_) {
+      it_->Advance();
+      if (it_->IsAtEnd())
+        it_.reset();
+    } else {
+      NOTREACHED();
+    }
+    return *this;
+  }
+
+  TestCaseIterator& operator++() {
+    return Advance();
+  }
+
+  TestCaseIterator& operator++(int) {
+    return Advance();

[bartfab (slow), 2013/11/05 18:18:33]

This is the post-increment operator. Its return value should be the
unincremented iterator.

[Joao da Silva, 2013/11/07 20:27:27]

Ah right, done

+  }
+
+ private:
+  scoped_ptr<Schema::Iterator> it_;
+};
+
 // Base class for tests that change policy and are parameterized with a policy
 // definition.
 class PolicyPrefsTest
     : public InProcessBrowserTest,
-      public testing::WithParamInterface<PolicyDefinitionList::Entry> {
+      public testing::WithParamInterface<const char*> {
  protected:
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     EXPECT_CALL(provider_, IsInitializationComplete(_))
@@ -412,20 +494,22 @@ TEST(PolicyPrefsTestCoverageTest, AllPoliciesHaveATestCase) {
   // This test fails when a policy is added to
   // chrome/app/policy/policy_templates.json but a test case is not added to
   // chrome/test/data/policy/policy_test_cases.json.
+  Schema chrome_schema = Schema::Wrap(GetChromeSchemaData());
+  ASSERT_TRUE(chrome_schema.valid());
+
   PolicyTestCases policy_test_cases;
-  const PolicyDefinitionList* list = GetChromePolicyDefinitionList();
-  for (const PolicyDefinitionList::Entry* policy = list->begin;
-       policy != list->end; ++policy) {
-    EXPECT_TRUE(ContainsKey(policy_test_cases.map(), policy->name))
-        << "Missing policy test case for: " << policy->name;
+  for (Schema::Iterator it = chrome_schema.GetPropertiesIterator();
+       !it.IsAtEnd(); it.Advance()) {
+    EXPECT_TRUE(ContainsKey(policy_test_cases.map(), it.key()))
+        << "Missing policy test case for: " << it.key();
   }
 }
 
 IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, PolicyToPrefsMapping) {
   // Verifies that policies make their corresponding preferences become managed,
   // and that the user can't override that setting.
-  const PolicyTestCase* test_case = policy_test_cases_.Get(GetParam().name);
-  ASSERT_TRUE(test_case) << "PolicyTestCase not found for " << GetParam().name;
+  const PolicyTestCase* test_case = policy_test_cases_.Get(GetParam());
+  ASSERT_TRUE(test_case) << "PolicyTestCase not found for " << GetParam();
   const ScopedVector<PrefMapping>& pref_mappings = test_case->pref_mappings();
   if (!test_case->IsSupported() || pref_mappings.empty())
     return;
@@ -470,9 +554,9 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, CheckPolicyIndicators) {
   // Verifies that controlled setting indicators correctly show whether a pref's
   // value is recommended or enforced by a corresponding policy.
   const PolicyTestCase* policy_test_case =
-      policy_test_cases_.Get(GetParam().name);
+      policy_test_cases_.Get(GetParam());
   ASSERT_TRUE(policy_test_case) << "PolicyTestCase not found for "
-      << GetParam().name;
+      << GetParam();
   const ScopedVector<PrefMapping>& pref_mappings =
       policy_test_case->pref_mappings();
   if (!policy_test_case->IsSupported() || pref_mappings.empty())
@@ -565,10 +649,9 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefsTest, CheckPolicyIndicators) {
   }
 }
 
-INSTANTIATE_TEST_CASE_P(
-    PolicyPrefsTestInstance,
-    PolicyPrefsTest,
-    testing::ValuesIn(GetChromePolicyDefinitionList()->begin,
-                      GetChromePolicyDefinitionList()->end));
+INSTANTIATE_TEST_CASE_P(PolicyPrefsTestInstance,
+                        PolicyPrefsTest,
+                        testing::ValuesIn(TestCaseIterator::GetBegin(),
+                                          TestCaseIterator::GetEnd()));
 
 }  // namespace policy