Removed the PolicyDefinitionList.

chrome/browser/policy/policy_prefs_browsertest.cc

 // 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.
 
 #include <algorithm>
+#include <iterator>
 #include <map>
 #include <sstream>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/policy/mock_configuration_policy_provider.h"
 #include "chrome/browser/policy/policy_map.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #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

 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
-using testing::AnyNumber;
 using testing::Return;
 using testing::_;
 
 namespace policy {
 
 namespace {
 
 const char kMainSettingsPage[] = "chrome://settings-frame";
 
 const char kCrosSettingsPrefix[] = "cros.";
@@ skipping 152 matching lines @@
     }
     int error_code = -1;
     std::string error_string;
     base::DictionaryValue* dict = NULL;
     scoped_ptr<base::Value> value(base::JSONReader::ReadAndReturnError(
         json, base::JSON_PARSE_RFC, &error_code, &error_string));
     if (!value.get() || !value->GetAsDictionary(&dict)) {
       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;
     }
   }
 
   ~PolicyTestCases() {
     STLDeleteValues(policy_test_cases_);
     delete policy_test_cases_;
   }
 
   const PolicyTestCase* Get(const std::string& name) {
     iterator it = policy_test_cases_->find(name);
@@ skipping 144 matching lines @@
   }
   if (!controlled_by.empty()) {
     EXPECT_TRUE(have_visible_indicators)
         << "Expected to find at least one visible controlled setting "
         << "indicator.";
   }
 }
 
 }  // 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(_))
         .WillRepeatedly(Return(true));
     BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
     ui_test_utils::WaitForTemplateURLServiceToLoad(
         TemplateURLServiceFactory::GetForProfile(browser()->profile()));
   }
 
   void UpdateProviderPolicy(const PolicyMap& policy) {
     provider_.UpdateChromePolicy(policy);
     base::RunLoop loop;
     loop.RunUntilIdle();
   }
 
   PolicyTestCases policy_test_cases_;
   MockConfigurationPolicyProvider provider_;
 };
 
 TEST(PolicyPrefsTestCoverageTest, AllPoliciesHaveATestCase) {
   // Verifies that all known policies have a test case in the JSON file.
   // 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;
   LOG(INFO) << "Testing policy: " << test_case->name();
 
   for (ScopedVector<PrefMapping>::const_iterator
            pref_mapping = pref_mappings.begin();
        pref_mapping != pref_mappings.end();
        ++pref_mapping) {
     // Skip Chrome OS preferences that use a different backend and cannot be
@@ skipping 24 matching lines @@
     EXPECT_FALSE(pref->IsUserModifiable());
     EXPECT_FALSE(pref->IsUserControlled());
     EXPECT_TRUE(pref->IsManaged());
   }
 }
 
 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())
     return;
   bool has_indicator_tests = false;
   for (ScopedVector<PrefMapping>::const_iterator
            pref_mapping = pref_mappings.begin();
        pref_mapping != pref_mappings.end();
        ++pref_mapping) {
     if (!(*pref_mapping)->indicator_test_cases().empty()) {
@@ skipping 72 matching lines @@
       prefs->Set((*pref_mapping)->pref().c_str(), *pref->GetValue());
       VerifyControlledSettingIndicators(browser(), indicator_selector,
                                         (*indicator_test_case)->value(),
                                         "hasRecommendation",
                                         (*indicator_test_case)->readonly());
       prefs->ClearPref((*pref_mapping)->pref().c_str());
     }
   }
 }
 
-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