Remove old default search preferences.

components/search_engines/default_search_policy_handler.cc

 // Copyright 2014 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 "components/search_engines/default_search_policy_handler.h"
 
 #include <stddef.h>
 
 #include <utility>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/strings/string_util.h"
 #include "components/policy/core/browser/policy_error_map.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_value_map.h"
 #include "components/search_engines/default_search_manager.h"
 #include "components/search_engines/search_engines_pref_names.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
 #include "grit/components_strings.h"
 
@@ skipping 27 matching lines @@
     bool is_string = policy_value->GetAsString(&str);
     DCHECK(is_string);
   }
   dict->SetString(key, str);
 }
 
 }  // namespace
 
 // List of policy types to preference names, for policies affecting the default
 // search provider.
-const PolicyToPreferenceMapEntry kDefaultSearchPolicyMap[] = {
-  { key::kDefaultSearchProviderEnabled,
-    prefs::kDefaultSearchProviderEnabled,
-    base::Value::Type::BOOLEAN },
-  { key::kDefaultSearchProviderName,
-    prefs::kDefaultSearchProviderName,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderKeyword,
-    prefs::kDefaultSearchProviderKeyword,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderSearchURL,
-    prefs::kDefaultSearchProviderSearchURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderSuggestURL,
-    prefs::kDefaultSearchProviderSuggestURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderInstantURL,
-    prefs::kDefaultSearchProviderInstantURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderIconURL,
-    prefs::kDefaultSearchProviderIconURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderEncodings,
-    prefs::kDefaultSearchProviderEncodings,
-    base::Value::Type::LIST },
-  { key::kDefaultSearchProviderAlternateURLs,
-    prefs::kDefaultSearchProviderAlternateURLs,
-    base::Value::Type::LIST },
-  { key::kDefaultSearchProviderSearchTermsReplacementKey,
-    prefs::kDefaultSearchProviderSearchTermsReplacementKey,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderImageURL,
-    prefs::kDefaultSearchProviderImageURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderNewTabURL,
-    prefs::kDefaultSearchProviderNewTabURL,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderSearchURLPostParams,
-    prefs::kDefaultSearchProviderSearchURLPostParams,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderSuggestURLPostParams,
-    prefs::kDefaultSearchProviderSuggestURLPostParams,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderInstantURLPostParams,
-    prefs::kDefaultSearchProviderInstantURLPostParams,
-    base::Value::Type::STRING },
-  { key::kDefaultSearchProviderImageURLPostParams,
-    prefs::kDefaultSearchProviderImageURLPostParams,
-    base::Value::Type::STRING },
-};
-
-// List of policy types to preference names, for policies affecting the default
-// search provider.
 const PolicyToPreferenceMapEntry kDefaultSearchPolicyDataMap[] = {
+    {key::kDefaultSearchProviderEnabled, prefs::kDefaultSearchProviderEnabled,
+     base::Value::Type::BOOLEAN },
     {key::kDefaultSearchProviderName, DefaultSearchManager::kShortName,
      base::Value::Type::STRING},
     {key::kDefaultSearchProviderKeyword, DefaultSearchManager::kKeyword,
      base::Value::Type::STRING},
     {key::kDefaultSearchProviderSearchURL, DefaultSearchManager::kURL,
      base::Value::Type::STRING},
     {key::kDefaultSearchProviderSuggestURL,
      DefaultSearchManager::kSuggestionsURL, base::Value::Type::STRING},
     {key::kDefaultSearchProviderInstantURL, DefaultSearchManager::kInstantURL,
      base::Value::Type::STRING},
@@ skipping 14 matching lines @@
      DefaultSearchManager::kSearchURLPostParams, base::Value::Type::STRING},
     {key::kDefaultSearchProviderSuggestURLPostParams,
      DefaultSearchManager::kSuggestionsURLPostParams,
      base::Value::Type::STRING},
     {key::kDefaultSearchProviderInstantURLPostParams,
      DefaultSearchManager::kInstantURLPostParams, base::Value::Type::STRING},
     {key::kDefaultSearchProviderImageURLPostParams,
      DefaultSearchManager::kImageURLPostParams, base::Value::Type::STRING},
 };
 
-// DefaultSearchEncodingsPolicyHandler implementation --------------------------
-
-DefaultSearchEncodingsPolicyHandler::DefaultSearchEncodingsPolicyHandler()
-    : TypeCheckingPolicyHandler(key::kDefaultSearchProviderEncodings,
-                                base::Value::Type::LIST) {}
-
-DefaultSearchEncodingsPolicyHandler::~DefaultSearchEncodingsPolicyHandler() {
-}
-
-void DefaultSearchEncodingsPolicyHandler::ApplyPolicySettings(
-    const PolicyMap& policies, PrefValueMap* prefs) {
-  // The DefaultSearchProviderEncodings policy has type list, but the related
-  // preference has type string. Convert one into the other here, using
-  // ';' as a separator.
-  const base::Value* value = policies.GetValue(policy_name());
-  const base::ListValue* list;
-  if (!value || !value->GetAsList(&list))
-    return;
-
-  base::ListValue::const_iterator iter(list->begin());
-  base::ListValue::const_iterator end(list->end());
-  std::vector<std::string> string_parts;
-  for (; iter != end; ++iter) {
-    std::string s;
-    if ((*iter)->GetAsString(&s)) {
-      string_parts.push_back(s);
-    }
-  }
-  std::string encodings = base::JoinString(string_parts, ";");
-  prefs->SetString(prefs::kDefaultSearchProviderEncodings, encodings);
-}
-
-
 // DefaultSearchPolicyHandler implementation -----------------------------------
 
-DefaultSearchPolicyHandler::DefaultSearchPolicyHandler() {
-  for (size_t i = 0; i < arraysize(kDefaultSearchPolicyMap); ++i) {
-    const char* policy_name = kDefaultSearchPolicyMap[i].policy_name;
-    if (policy_name == key::kDefaultSearchProviderEncodings) {
-      handlers_.push_back(
-          base::MakeUnique<DefaultSearchEncodingsPolicyHandler>());
-    } else {
-      handlers_.push_back(base::MakeUnique<SimplePolicyHandler>(
-          policy_name, kDefaultSearchPolicyMap[i].preference_path,
-          kDefaultSearchPolicyMap[i].value_type));
-    }
-  }
-}
+DefaultSearchPolicyHandler::DefaultSearchPolicyHandler() {}
 
 DefaultSearchPolicyHandler::~DefaultSearchPolicyHandler() {}
 
 bool DefaultSearchPolicyHandler::CheckPolicySettings(const PolicyMap& policies,
                                                      PolicyErrorMap* errors) {
   if (!CheckIndividualPolicies(policies, errors))
     return false;
 
   if (DefaultSearchProviderIsDisabled(policies)) {
     // Add an error for all specified default search policies except
     // DefaultSearchProviderEnabled.
 
-    for (const auto& handler : handlers_) {
-      const char* policy_name = handler->policy_name();
+    for (const auto& policy_map_entry : kDefaultSearchPolicyDataMap) {
+      const char* policy_name = policy_map_entry.policy_name;
       if (policy_name != key::kDefaultSearchProviderEnabled &&
           HasDefaultSearchPolicy(policies, policy_name)) {
         errors->AddError(policy_name, IDS_POLICY_DEFAULT_SEARCH_DISABLED);
       }
     }
     return true;
   }
 
   const base::Value* url;
   std::string dummy;
@@ skipping 18 matching lines @@
   // sure that they are all specified via policy, so that the regular prefs
   // aren't used.
   const base::Value* dummy;
   std::string url;
   if (!DefaultSearchURLIsValid(policies, &dummy, &url))
     return;
 
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
   for (size_t i = 0; i < arraysize(kDefaultSearchPolicyDataMap); ++i) {
     const char* policy_name = kDefaultSearchPolicyDataMap[i].policy_name;
+    // Skip handling of key::kDefaultSearchProviderEnabled. Its already handled
+    // above.

[Peter Kasting, 2017/01/09 22:00:05]

Nit: Shorter and avoids restating the code: "kDefaultSearchProviderEnabled has
already been handled."

[Alexander Yashkin, 2017/01/10 05:01:43]

Done.

+    if (policy_name == key::kDefaultSearchProviderEnabled) continue;

[Peter Kasting, 2017/01/09 22:00:05]

Nit: Put "continue" on a separate line (Google style guide allows your style,
but Chromium does not generally use it)

[Alexander Yashkin, 2017/01/10 05:01:43]

Done.

+
     switch (kDefaultSearchPolicyDataMap[i].value_type) {
       case base::Value::Type::STRING:
         SetStringInPref(policies,
                         policy_name,
                         kDefaultSearchPolicyDataMap[i].preference_path,
                         dict.get());
         break;
       case base::Value::Type::LIST:
         SetListInPref(policies,
                       policy_name,
@@ skipping 36 matching lines @@
   if (keyword.empty())
     dict->SetString(DefaultSearchManager::kKeyword, host);
 
   DefaultSearchManager::AddPrefValueToMap(std::move(dict), prefs);
 }
 
 bool DefaultSearchPolicyHandler::CheckIndividualPolicies(
     const PolicyMap& policies,
     PolicyErrorMap* errors) {
   bool all_ok = true;
-  for (const auto& handler : handlers_) {
-    // It's important to call CheckPolicySettings() on all handlers and not just
-    // exit on the first error, so we report all policy errors.
-    all_ok &= handler->CheckPolicySettings(policies, errors);
+  for (const auto& policy_map_entry : kDefaultSearchPolicyDataMap) {
+    // It's important to check policy type for all policies and not just exit on
+    // the first error, so we report all policy errors.
+    auto policy_name = policy_map_entry.policy_name;

[Peter Kasting, 2017/01/09 22:00:06]

Nit: Can just inline into the one use below

[Alexander Yashkin, 2017/01/10 05:01:44]

Done.

+    auto value_type = policy_map_entry.value_type;
+    // Check value type for every policy.

[Peter Kasting, 2017/01/09 22:00:05]

Nit: Not sure what this comment buys over the one above

[Alexander Yashkin, 2017/01/10 05:01:44]

Removed

+    const base::Value* value = policies.GetValue(policy_name);
+    if (value && !value->IsType(value_type)) {
+      errors->AddError(policy_name, IDS_POLICY_TYPE_ERROR,
+                     base::Value::GetTypeName(value_type));

[Peter Kasting, 2017/01/09 22:00:05]

Nit: Bad indentation

[Alexander Yashkin, 2017/01/10 05:01:43]

Fixed

+      all_ok &= false;

[Peter Kasting, 2017/01/09 22:00:05]

Nit: Just use =

[Alexander Yashkin, 2017/01/10 05:01:44]

Done.

+    }
   }
   return all_ok;
 }
 
 bool DefaultSearchPolicyHandler::HasDefaultSearchPolicy(
     const PolicyMap& policies,
     const char* policy_name) {
   return policies.Get(policy_name) != NULL;
 }
 
 bool DefaultSearchPolicyHandler::AnyDefaultSearchPoliciesSpecified(
     const PolicyMap& policies) {
-  for (const auto& handler : handlers_) {
-    if (policies.Get(handler->policy_name()))
+  for (const auto& policy_map_entry : kDefaultSearchPolicyDataMap) {
+    if (policies.Get(policy_map_entry.policy_name))
       return true;
   }
   return false;
 }
 
 bool DefaultSearchPolicyHandler::DefaultSearchProviderIsDisabled(
     const PolicyMap& policies) {
   const base::Value* provider_enabled =
       policies.GetValue(key::kDefaultSearchProviderEnabled);
   bool enabled = true;
@@ skipping 26 matching lines @@
 void DefaultSearchPolicyHandler::EnsureListPrefExists(
     PrefValueMap* prefs,
     const std::string& path) {
   base::Value* value;
   base::ListValue* list_value;
   if (!prefs->GetValue(path, &value) || !value->GetAsList(&list_value))
     prefs->SetValue(path, base::MakeUnique<base::ListValue>());
 }
 
 }  // namespace policy