Store default search provider data in dictionary pref.

chrome/browser/search_engines/default_search_manager.cc

Index: chrome/browser/search_engines/default_search_manager.cc
diff --git a/chrome/browser/search_engines/default_search_manager.cc b/chrome/browser/search_engines/default_search_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..cc4de8f416998cbda1167acfaddc920947fd65f7
--- /dev/null
+++ b/chrome/browser/search_engines/default_search_manager.cc
@@ -0,0 +1,221 @@
+// 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 "chrome/browser/search_engines/default_search_manager.h"
+
+#include <algorithm>
+#include <utility>
+
+#include "base/compiler_specific.h"
+#include "base/i18n/case_conversion.h"
+#include "base/prefs/pref_service.h"
+#include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/util.h"
+#include "chrome/common/pref_names.h"
+#include "components/user_prefs/pref_registry_syncable.h"
+
+namespace {
+

[Peter Kasting, 2014/04/23 23:36:30]

Nit: If you're going to put a blank line here, put one above the closing brace
as well.

[Cait (Slow), 2014/04/24 14:53:39]

Done.

+// A dictionary to hold all data related to the Default Search Engine.
+// Eventually, this should replace all the data stored in the
+// default_search_provider.* prefs.
+const char kDefaultSearchProviderData[] =
+    "default_search_provider.template_url_data";
+}  // namespace
+
+const char* DefaultSearchManager::kID = "id";
+const char* DefaultSearchManager::kShortName = "short_name";
+const char* DefaultSearchManager::kKeyword = "keyword";
+const char* DefaultSearchManager::kPrepopulateID = "prepopulate_id";
+const char* DefaultSearchManager::kSyncGUID = "synced_guid";
+
+const char* DefaultSearchManager::kURL = "url";
+const char* DefaultSearchManager::kSuggestionsURL = "suggestions_url";
+const char* DefaultSearchManager::kInstantURL = "instant_url";
+const char* DefaultSearchManager::kImageURL = "image_url";
+const char* DefaultSearchManager::kNewTabURL = "new_tab_url";
+const char* DefaultSearchManager::kFaviconURL = "favicon_url";
+const char* DefaultSearchManager::kOriginatingURL = "originating_url";
+
+const char* DefaultSearchManager::kSearchURLPostParams =
+    "search_url_post_params";
+const char* DefaultSearchManager::kSuggestionsURLPostParams =
+    "suggestions_url_post_params";
+const char* DefaultSearchManager::kInstantURLPostParams =
+    "instant_url_post_params";
+const char* DefaultSearchManager::kImageURLPostParams = "image_url_post_params";
+
+const char* DefaultSearchManager::kSafeForAutoReplace = "safe_for_autoreplace";
+const char* DefaultSearchManager::kInputEncodings = "input_encodings";
+
+const char* DefaultSearchManager::kDateCreated = "date_created";
+const char* DefaultSearchManager::kLastModified = "last_modified";
+
+const char* DefaultSearchManager::kUsageCount = "usage_count";
+const char* DefaultSearchManager::kAlternateURLs = "alternate_urls";
+const char* DefaultSearchManager::kSearchTermsReplacementKey =
+    "search_terms_replacement_key";
+const char* DefaultSearchManager::kCreatedByPolicy = "created_by_policy";
+
+DefaultSearchManager::DefaultSearchManager(PrefService* pref_service)
+    : pref_service_(pref_service) {
+  DCHECK(pref_service_);
+}
+
+DefaultSearchManager::~DefaultSearchManager() {
+}
+
+// static
+void DefaultSearchManager::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterDictionaryPref(
+      kDefaultSearchProviderData,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+}
+
+bool DefaultSearchManager::GetDefaultSearchEngine(TemplateURLData* data) {
+  const base::DictionaryValue* url_dict =
+      pref_service_->GetDictionary(kDefaultSearchProviderData);
+
+  if (url_dict->empty())
+    return false;
+
+  std::string search_url;
+  base::string16 keyword;
+  url_dict->GetString(kURL, &search_url);
+  url_dict->GetString(kKeyword, &keyword);
+  DCHECK(!search_url.empty());
+  DCHECK(!keyword.empty());
+
+  if (search_url.empty())

[Peter Kasting, 2014/04/23 23:36:30]

Don't handle DCHECK failure.  Either these can never be empty, or they can.  If
they can't, you don't need to handle it.  If they can, the DCHECKs are wrong.

[Cait (Slow), 2014/04/24 14:53:39]

Done.

[gab, 2014/04/24 15:00:44]

While I agree that DCHECKs shouldn't be used for things that *can* happen, I
think this case is special.

It *should* never happen for prefs that are read as they were written by Chrome
(hence the DCHECK documenting and ensuring that this never happens in regular
Chrome flow); but it's not *impossible* for it to happen if we account for
manual tampering with the Preferences file (which is not supported, but we have
to be prepared to handle IMO -- or we could just say it's really *not* supported
and let Chrome have arbitrary behaviour if it does..?).

[Peter Kasting, 2014/04/24 20:48:46]

Then you must not have DCHECKs.

DCHECKs say that the behavior in question can never occur under any
circumstance, including things outside Chrome's control.  For example, "I ran
out of disk space in the midst of a profile write" is allowed to cause Chrome to
behave badly, but it is NOT allowed to violate any DCHECKs.  A DCHECK must hold
at all times for all reasons.

+    return false;
+  if (keyword.empty())
+    keyword = TemplateURLService::GenerateKeyword(GURL(search_url));
+  data->SetKeyword(keyword);
+  data->SetURL(search_url);
+
+  std::string id;
+  url_dict->GetString(kID, &id);
+  base::StringToInt64(id, &data->id);
+  url_dict->GetString(kShortName, &data->short_name);
+  url_dict->GetInteger(kPrepopulateID, &data->prepopulate_id);
+  url_dict->GetString(kSyncGUID, &data->sync_guid);
+
+  url_dict->GetString(kSuggestionsURL, &data->suggestions_url);
+  url_dict->GetString(kInstantURL, &data->instant_url);
+  url_dict->GetString(kImageURL, &data->image_url);
+  url_dict->GetString(kNewTabURL, &data->new_tab_url);
+
+  std::string favicon_url;
+  std::string originating_url;
+  url_dict->GetString(kFaviconURL, &favicon_url);
+  url_dict->GetString(kOriginatingURL, &originating_url);
+  data->favicon_url = GURL(favicon_url);
+  data->originating_url = GURL(originating_url);
+
+  url_dict->GetString(kSearchURLPostParams, &data->search_url_post_params);
+  url_dict->GetString(kSuggestionsURLPostParams,
+                      &data->suggestions_url_post_params);
+  url_dict->GetString(kInstantURLPostParams, &data->instant_url_post_params);
+  url_dict->GetString(kImageURLPostParams, &data->image_url_post_params);
+
+  url_dict->GetBoolean(kSafeForAutoReplace, &data->safe_for_autoreplace);
+
+  double date_created = 0.0;
+  double last_modified = 0.0;
+  url_dict->GetDouble(kDateCreated, &date_created);
+  url_dict->GetDouble(kLastModified, &last_modified);
+  data->date_created = base::Time::FromInternalValue(date_created);
+  data->last_modified = base::Time::FromInternalValue(last_modified);
+
+  url_dict->GetInteger(kUsageCount, &data->usage_count);
+
+  const base::ListValue* alternate_urls = new base::ListValue;
+  url_dict->GetList(kAlternateURLs, &alternate_urls);
+  data->alternate_urls.clear();
+  for (base::ListValue::const_iterator it = alternate_urls->begin();
+       it != alternate_urls->end();
+       ++it) {

[Peter Kasting, 2014/04/23 23:36:30]

Nit: It's legal either way, but personally I'd put this on the line above.  (2
places)

[Cait (Slow), 2014/04/24 14:53:39]

Done.

[gab, 2014/04/24 15:00:44]

I agree, but this is the format `git cl format` outputs these days and I've
found it annoying to try to go against its will.

[Peter Kasting, 2014/04/24 20:48:46]

There's an easy answer to that: Never run git cl format.  :)

[gab, 2014/04/25 11:23:42]

Hmmm... Then what's the point of it..? IMO it should be used as much as
possible, getting rid of most accidental nits before the review even begins.

[Peter Kasting, 2014/04/25 17:52:13]

See many chromium-dev debate threads about this issue.

+    std::string alternate_url;
+    if ((*it)->GetAsString(&alternate_url))
+      data->alternate_urls.push_back(alternate_url);
+  }
+
+  const base::ListValue* encodings = new base::ListValue;
+  url_dict->GetList(kInputEncodings, &encodings);
+  data->input_encodings.clear();
+  for (base::ListValue::const_iterator it = encodings->begin();
+       it != encodings->end();
+       ++it) {
+    std::string encoding;
+    if ((*it)->GetAsString(&encoding))
+      data->input_encodings.push_back(encoding);
+  }
+
+  url_dict->GetString(kSearchTermsReplacementKey,
+                      &data->search_terms_replacement_key);
+
+  url_dict->GetBoolean(kCreatedByPolicy, &data->created_by_policy);
+
+  data->show_in_default_list = true;
+
+  return true;
+}
+
+void DefaultSearchManager::SetUserSelectedDefaultSearchEngine(
+    const TemplateURLData& data) {
+  base::DictionaryValue* url_dict = new base::DictionaryValue;
+  url_dict->SetString(kID, base::Int64ToString(data.id));
+  url_dict->SetString(kShortName, data.short_name);
+  url_dict->SetString(kKeyword, data.keyword());
+  url_dict->SetInteger(kPrepopulateID, data.prepopulate_id);
+  url_dict->SetString(kSyncGUID, data.sync_guid);
+
+  url_dict->SetString(kURL, data.url());
+  url_dict->SetString(kSuggestionsURL, data.suggestions_url);
+  url_dict->SetString(kInstantURL, data.instant_url);
+  url_dict->SetString(kImageURL, data.image_url);
+  url_dict->SetString(kNewTabURL, data.new_tab_url);
+  url_dict->SetString(kFaviconURL, data.favicon_url.spec());
+  url_dict->SetString(kOriginatingURL, data.originating_url.spec());
+
+  url_dict->SetString(kSearchURLPostParams, data.search_url_post_params);
+  url_dict->SetString(kSuggestionsURLPostParams,
+                      data.suggestions_url_post_params);
+  url_dict->SetString(kInstantURLPostParams, data.instant_url_post_params);
+  url_dict->SetString(kImageURLPostParams, data.image_url_post_params);
+
+  url_dict->SetBoolean(kSafeForAutoReplace, data.safe_for_autoreplace);
+
+  url_dict->SetDouble(kDateCreated, data.date_created.ToInternalValue());
+  url_dict->SetDouble(kLastModified, data.last_modified.ToInternalValue());
+  url_dict->SetInteger(kUsageCount, data.usage_count);
+
+  base::ListValue* alternate_urls = new base::ListValue;
+  std::vector<std::string>::const_iterator it = data.alternate_urls.begin();

[Peter Kasting, 2014/04/23 23:36:30]

Nit: Scope loop variables like this to the loop body, don't declare them above
the loop.  (2 places)

[Cait (Slow), 2014/04/24 14:53:39]

Done.

+  for (; it != data.alternate_urls.end(); ++it)
+    alternate_urls->AppendString(*it);
+  url_dict->Set(kAlternateURLs, alternate_urls);
+
+  base::ListValue* encodings = new base::ListValue;
+  it = data.input_encodings.begin();
+  for (; it != data.input_encodings.end(); ++it)
+    encodings->AppendString(*it);
+  url_dict->Set(kInputEncodings, encodings);
+
+  url_dict->SetString(kSearchTermsReplacementKey,
+                      data.search_terms_replacement_key);
+
+  url_dict->SetBoolean(kCreatedByPolicy, data.created_by_policy);
+
+  pref_service_->Set(kDefaultSearchProviderData, *url_dict);
+}