Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Unified Diff: chrome/browser/search_engines/template_url_service.cc

Issue 9982018: Move most TemplateURL data members to a new struct, TemplateURLData. This allows us to eliminate t… (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: Created 8 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/search_engines/template_url_service.cc
===================================================================
--- chrome/browser/search_engines/template_url_service.cc (revision 130759)
+++ chrome/browser/search_engines/template_url_service.cc (working copy)
@@ -315,6 +315,17 @@
NotifyObservers();
}
+void TemplateURLService::AddWithOverrides(const TemplateURL* template_url,
+ const string16& short_name,
+ const string16& keyword,
+ const std::string& url) {
+ TemplateURL* modifiable_url = const_cast<TemplateURL*>(template_url);
+ modifiable_url->data_.short_name = short_name;
+ modifiable_url->data_.SetKeyword(keyword);
+ modifiable_url->SetURL(url);
+ Add(modifiable_url);
+}
+
void TemplateURLService::Remove(const TemplateURL* template_url) {
RemoveNoNotify(template_url);
NotifyObservers();
@@ -367,15 +378,14 @@
const TemplateURL* existing_url = GetTemplateURLForExtension(extension);
string16 keyword = UTF8ToUTF16(extension->omnibox_keyword());
- scoped_ptr<TemplateURL> template_url(new TemplateURL);
- template_url->set_short_name(UTF8ToUTF16(extension->name()));
- template_url->set_keyword(keyword);
+ TemplateURLData data;
+ data.short_name = UTF8ToUTF16(extension->name());
+ data.SetKeyword(UTF8ToUTF16(extension->omnibox_keyword()));
// This URL is not actually used for navigation. It holds the extension's
// ID, as well as forcing the TemplateURL to be treated as a search keyword.
- template_url->SetURL(
- std::string(chrome::kExtensionScheme) + "://" +
- extension->id() + "/?q={searchTerms}");
- template_url->set_safe_for_autoreplace(false);
+ data.SetURL(std::string(chrome::kExtensionScheme) + "://" + extension->id() +
+ "/?q={searchTerms}");
+ scoped_ptr<TemplateURL> template_url(new TemplateURL(data));
if (existing_url) {
// TODO(mpcomplete): only replace if the user hasn't changed the keyword.
@@ -413,25 +423,26 @@
void TemplateURLService::IncrementUsageCount(const TemplateURL* url) {
DCHECK(url && std::find(template_urls_.begin(), template_urls_.end(), url) !=
template_urls_.end());
- const_cast<TemplateURL*>(url)->set_usage_count(url->usage_count() + 1);
+ ++const_cast<TemplateURL*>(url)->data_.usage_count;
if (service_.get())
- service_.get()->UpdateKeyword(*url);
+ service_->UpdateKeyword(*url);
}
void TemplateURLService::ResetTemplateURL(const TemplateURL* url,
const string16& title,
const string16& keyword,
const std::string& search_url) {
- TemplateURL new_url(*url);
- new_url.set_short_name(title);
- new_url.set_keyword(keyword);
- if (new_url.url() != search_url) {
- new_url.SetURL(search_url);
+ TemplateURLData data(url->data());
+ data.short_name = title;
+ data.SetKeyword(keyword);
+ if (search_url != data.url()) {
+ data.SetURL(search_url);
// The urls have changed, reset the favicon url.
- new_url.set_favicon_url(GURL());
+ data.favicon_url = GURL();
}
- new_url.set_safe_for_autoreplace(false);
- new_url.set_last_modified(time_provider_());
+ data.safe_for_autoreplace = false;
+ data.last_modified = time_provider_();
+ TemplateURL new_url(data);
UpdateNoNotify(url, new_url);
NotifyObservers();
}
@@ -567,16 +578,15 @@
// Reuse it.
} else {
// The value from the preferences takes over.
- //
- // AddNoNotify will take ownership of default_from_prefs so it is safe to
- // release. If it's null, there's no ownership to worry about :-)
- TemplateURL* managed_default = default_from_prefs.release();
- if (managed_default) {
- managed_default->set_created_by_policy(true);
- managed_default->set_id(kInvalidTemplateURLID);
+ default_search_provider = NULL;
+ if (default_from_prefs.get()) {
+ TemplateURLData data(default_from_prefs->data());
+ data.created_by_policy = true;
+ data.id = kInvalidTemplateURLID;
+ TemplateURL* managed_default = new TemplateURL(data);
AddNoNotify(managed_default);
+ default_search_provider = managed_default;
}
- default_search_provider = managed_default;
}
// Note that this saves the default search provider to prefs.
SetDefaultSearchProviderNoNotify(default_search_provider);
@@ -588,7 +598,7 @@
default_search_provider = synced_default;
pending_synced_default_search_ = false;
} else if (database_specified_a_default &&
- NULL == default_search_provider &&
+ default_search_provider == NULL &&
!template_urls.empty()) {
default_search_provider = template_urls[0];
}
@@ -750,14 +760,12 @@
iter != change_list.end(); ++iter) {
DCHECK_EQ(syncable::SEARCH_ENGINES, iter->sync_data().GetDataType());
- scoped_ptr<TemplateURL> turl(
- CreateTemplateURLFromSyncData(iter->sync_data()));
- if (!turl.get()) {
- NOTREACHED() << "Failed to read search engine.";
- continue;
- }
+ std::string guid =
+ iter->sync_data().GetSpecifics().search_engine().sync_guid();
+ const TemplateURL* existing_turl = GetTemplateURLForGUID(guid);
+ scoped_ptr<TemplateURL> turl(CreateTemplateURLFromTemplateURLAndSyncData(
+ existing_turl, iter->sync_data()));
- const TemplateURL* existing_turl = GetTemplateURLForGUID(turl->sync_guid());
const TemplateURL* existing_keyword_turl =
GetTemplateURLForKeyword(turl->keyword());
@@ -781,8 +789,9 @@
if (existing_keyword_turl)
ResolveSyncKeywordConflict(turl.get(), &new_changes);
// Force the local ID to kInvalidTemplateURLID so we can add it.
- turl->set_id(kInvalidTemplateURLID);
- Add(turl.release());
+ TemplateURLData data(turl->data());
+ data.id = kInvalidTemplateURLID;
+ Add(new TemplateURL(data));
// Possibly set the newly added |turl| as the default search provider.
SetDefaultSearchProviderIfNewlySynced(guid);
@@ -790,11 +799,9 @@
existing_turl) {
// Possibly resolve a keyword conflict if they have the same keywords but
// are not the same entry.
- TemplateURL updated_turl(*existing_turl);
- UpdateTemplateURLWithSyncData(&updated_turl, iter->sync_data());
if (existing_keyword_turl && existing_keyword_turl != existing_turl)
- ResolveSyncKeywordConflict(&updated_turl, &new_changes);
- UpdateNoNotify(existing_turl, updated_turl);
+ ResolveSyncKeywordConflict(turl.get(), &new_changes);
+ UpdateNoNotify(existing_turl, *turl);
NotifyObservers();
} else {
// Something really unexpected happened. Either we received an
@@ -853,10 +860,9 @@
for (SyncDataMap::const_iterator iter = sync_data_map.begin();
iter != sync_data_map.end(); ++iter) {
+ const TemplateURL* local_turl = GetTemplateURLForGUID(iter->first);
scoped_ptr<TemplateURL> sync_turl(
- CreateTemplateURLFromSyncData(iter->second));
- DCHECK(sync_turl.get());
- const TemplateURL* local_turl = GetTemplateURLForGUID(iter->first);
+ CreateTemplateURLFromTemplateURLAndSyncData(local_turl, iter->second));
if (sync_turl->sync_guid().empty()) {
// Due to a bug, older search engine entries with no sync GUID
@@ -874,9 +880,7 @@
// TemplateURLID and the TemplateURL may have to be reparsed. This
// also makes the local data's last_modified timestamp equal to Sync's,
// avoiding an Update on the next MergeData call.
- TemplateURL updated_turl(*local_turl);
- UpdateTemplateURLWithSyncData(&updated_turl, iter->second);
- UpdateNoNotify(local_turl, updated_turl);
+ UpdateNoNotify(local_turl, *sync_turl);
NotifyObservers();
} else if (sync_turl->last_modified() < local_turl->last_modified()) {
// Otherwise, we know we have newer data, so update Sync with our
@@ -907,8 +911,9 @@
// the cloud.
ResolveSyncKeywordConflict(sync_turl.get(), &new_changes);
// Force the local ID to kInvalidTemplateURLID so we can add it.
- sync_turl->set_id(kInvalidTemplateURLID);
- Add(sync_turl.release());
+ TemplateURLData data(sync_turl->data());
+ data.id = kInvalidTemplateURLID;
+ Add(new TemplateURL(data));
// Possibly set the newly added |turl| as the default search provider.
SetDefaultSearchProviderIfNewlySynced(guid);
@@ -992,11 +997,34 @@
}
// static
-TemplateURL* TemplateURLService::CreateTemplateURLFromSyncData(
+TemplateURL* TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(
+ const TemplateURL* existing_turl,
const SyncData& sync_data) {
- TemplateURL* turl = new TemplateURL();
- UpdateTemplateURLWithSyncData(turl, sync_data);
- return turl;
+ sync_pb::SearchEngineSpecifics specifics =
+ sync_data.GetSpecifics().search_engine();
+
+ TemplateURLData data;
+ data.short_name = UTF8ToUTF16(specifics.short_name());
+ data.originating_url = GURL(specifics.originating_url());
+ data.SetKeyword(UTF8ToUTF16(specifics.keyword()));
+ data.SetAutogenerateKeyword(specifics.autogenerate_keyword());
+ data.SetURL(specifics.url());
+ data.suggestions_url = specifics.suggestions_url();
+ data.instant_url = specifics.instant_url();
+ data.favicon_url = GURL(specifics.favicon_url());
+ data.show_in_default_list = specifics.show_in_default_list();
+ data.safe_for_autoreplace = specifics.safe_for_autoreplace();
+ base::SplitString(specifics.input_encodings(), ';', &data.input_encodings);
+ data.date_created = base::Time::FromInternalValue(specifics.date_created());
+ data.last_modified = base::Time::FromInternalValue(specifics.last_modified());
+ data.prepopulate_id = specifics.prepopulate_id();
+ data.sync_guid = specifics.sync_guid();
+ if (existing_turl) {
+ data.id = existing_turl->id();
+ data.created_by_policy = existing_turl->created_by_policy();
+ data.usage_count = existing_turl->usage_count();
+ }
+ return new TemplateURL(data);
}
// static
@@ -1057,11 +1085,11 @@
// TemplateURLService ends up owning the TemplateURL, don't try and free
// it.
- TemplateURL* template_url = new TemplateURL();
- template_url->set_keyword(UTF8ToUTF16(initializers[i].keyword));
- template_url->set_short_name(UTF8ToUTF16(initializers[i].content));
- template_url->SetURL(osd_url);
- AddNoNotify(template_url);
+ TemplateURLData data;
+ data.short_name = UTF8ToUTF16(initializers[i].content);
+ data.SetKeyword(UTF8ToUTF16(initializers[i].keyword));
+ data.SetURL(osd_url);
+ AddNoNotify(new TemplateURL(data));
}
}
@@ -1242,27 +1270,26 @@
std::string prepopulate_id =
prefs->GetString(prefs::kDefaultSearchProviderPrepopulateID);
- default_provider->reset(new TemplateURL());
- (*default_provider)->set_short_name(name);
- (*default_provider)->SetURL(search_url);
- (*default_provider)->SetSuggestionsURL(suggest_url);
- (*default_provider)->SetInstantURL(instant_url);
- (*default_provider)->set_keyword(keyword);
- (*default_provider)->set_favicon_url(GURL(icon_url));
- std::vector<std::string> encodings_vector;
- base::SplitString(encodings, ';', &encodings_vector);
- (*default_provider)->set_input_encodings(encodings_vector);
+ TemplateURLData data;
+ data.short_name = name;
+ data.SetKeyword(keyword);
+ data.SetURL(search_url);
+ data.suggestions_url = suggest_url;
+ data.instant_url = instant_url;
+ data.favicon_url = GURL(icon_url);
+ data.show_in_default_list = true;
+ base::SplitString(encodings, ';', &data.input_encodings);
if (!id_string.empty() && !*is_managed) {
int64 value;
base::StringToInt64(id_string, &value);
- (*default_provider)->set_id(value);
+ data.id = value;
}
if (!prepopulate_id.empty() && !*is_managed) {
int value;
base::StringToInt(prepopulate_id, &value);
- (*default_provider)->SetPrepopulateId(value);
+ data.prepopulate_id = value;
}
- (*default_provider)->set_show_in_default_list(true);
+ default_provider->reset(new TemplateURL(data));
return true;
}
@@ -1304,9 +1331,13 @@
if (!existing_turl->sync_guid().empty())
guid_to_template_map_.erase(existing_turl->sync_guid());
- // This call handles copying over the values (while retaining the id).
+ provider_map_.Remove(existing_turl);
+ TemplateURLID previous_id = existing_turl->id();
+ TemplateURL* modifiable_turl = const_cast<TemplateURL*>(existing_turl);
+ *modifiable_turl = new_values;
+ modifiable_turl->data_.id = previous_id;
UIThreadSearchTermsData search_terms_data;
- provider_map_.Update(existing_turl, new_values, search_terms_data);
+ provider_map_.Add(existing_turl, search_terms_data);
if (!existing_turl->keyword().empty())
keyword_to_template_map_[existing_turl->keyword()] = existing_turl;
@@ -1511,14 +1542,18 @@
SetDefaultSearchProviderNoNotify(NULL);
RemoveNoNotify(old_default);
} else if (default_search_provider_) {
- new_default_from_prefs->set_created_by_policy(true);
- UpdateNoNotify(default_search_provider_, *new_default_from_prefs.get());
+ TemplateURLData data(new_default_from_prefs->data());
+ data.created_by_policy = true;
+ TemplateURL new_values(data);
+ UpdateNoNotify(default_search_provider_, new_values);
} else {
// AddNoNotify will take ownership of new_template, so it's safe to
// release.
- TemplateURL* new_template = new_default_from_prefs.release();
- if (new_template) {
- new_template->set_created_by_policy(true);
+ TemplateURL* new_template = NULL;
+ if (new_default_from_prefs.get()) {
+ TemplateURLData data(new_default_from_prefs->data());
+ data.created_by_policy = true;
+ new_template = new TemplateURL(data);
AddNoNotify(new_template);
}
SetDefaultSearchProviderNoNotify(new_template);
@@ -1529,9 +1564,11 @@
is_default_search_managed_ = new_is_default_managed;
// AddNoNotify will take ownership of new_template, so it's safe to
// release.
- TemplateURL* new_template = new_default_from_prefs.release();
- if (new_template) {
- new_template->set_created_by_policy(true);
+ TemplateURL* new_template = NULL;
+ if (new_default_from_prefs.get()) {
+ TemplateURLData data(new_default_from_prefs->data());
+ data.created_by_policy = true;
+ new_template = new TemplateURL(data);
AddNoNotify(new_template);
}
SetDefaultSearchProviderNoNotify(new_template);
@@ -1561,17 +1598,17 @@
void TemplateURLService::SetDefaultSearchProviderNoNotify(
const TemplateURL* url) {
- DCHECK(!url || std::find(template_urls_.begin(), template_urls_.end(), url)
- != template_urls_.end());
+ DCHECK(!url || std::find(template_urls_.begin(), template_urls_.end(), url) !=
+ template_urls_.end());
default_search_provider_ = url;
if (url) {
TemplateURL* modifiable_url = const_cast<TemplateURL*>(url);
// Don't mark the url as edited, otherwise we won't be able to rev the
// template urls we ship with.
- modifiable_url->set_show_in_default_list(true);
+ modifiable_url->data_.show_in_default_list = true;
if (service_.get())
- service_.get()->UpdateKeyword(*url);
+ service_->UpdateKeyword(*url);
if (url->url_ref().HasGoogleBaseURLs()) {
GoogleURLTracker::RequestServerCheck();
@@ -1610,7 +1647,7 @@
DCHECK_EQ(kInvalidTemplateURLID, template_url->id());
DCHECK(std::find(template_urls_.begin(), template_urls_.end(), template_url)
== template_urls_.end());
- template_url->set_id(++next_id_);
+ template_url->data_.id = ++next_id_;
template_urls_.push_back(template_url);
AddToMaps(template_url);
@@ -1641,7 +1678,7 @@
template_urls_.erase(i);
if (service_.get())
- service_->RemoveKeyword(*template_url);
+ service_->RemoveKeyword(template_url->id());
// Inform sync of the deletion.
ProcessTemplateURLChange(template_url, SyncChange::ACTION_DELETE);
@@ -1706,7 +1743,7 @@
i = template_urls->erase(i);
if (service_.get())
- service_->RemoveKeyword(*template_url);
+ service_->RemoveKeyword(template_url->id());
delete template_url;
} else {
++i;
@@ -1718,8 +1755,9 @@
const std::string& guid) {
DCHECK(!guid.empty());
- TemplateURL new_url(*url);
- new_url.set_sync_guid(guid);
+ TemplateURLData data(url->data());
+ data.sync_guid = guid;
+ TemplateURL new_url(data);
UpdateNoNotify(url, new_url);
}
@@ -1763,15 +1801,16 @@
existing_turl->created_by_policy()) {
string16 new_keyword = UniquifyKeyword(*sync_turl);
DCHECK(!GetTemplateURLForKeyword(new_keyword));
- sync_turl->set_keyword(new_keyword);
+ sync_turl->data_.SetKeyword(new_keyword);
// If we update the cloud TURL, we need to push an update back to sync
// informing it that something has changed.
SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl);
change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
} else {
string16 new_keyword = UniquifyKeyword(*existing_turl);
- TemplateURL new_turl(*existing_turl);
- new_turl.set_keyword(new_keyword);
+ TemplateURLData data(existing_turl->data());
+ data.SetKeyword(new_keyword);
+ TemplateURL new_turl(data);
UpdateNoNotify(existing_turl, new_turl);
NotifyObservers();
}
@@ -1799,10 +1838,7 @@
DCHECK(sync_turl);
DCHECK(local_turl);
DCHECK(change_list);
-
- scoped_ptr<TemplateURL> scoped_sync_turl(sync_turl);
-
- if (scoped_sync_turl->last_modified() > local_turl->last_modified()) {
+ if (sync_turl->last_modified() > local_turl->last_modified()) {
// Fully replace local_url with Sync's copy. Note that because use Add
// rather than ResetTemplateURL, |sync_url| is added with a fresh
// TemplateURLID. We don't need to sync the new ID back to the server since
@@ -1817,11 +1853,10 @@
Remove(local_turl);
// Force the local ID to kInvalidTemplateURLID so we can add it.
- scoped_sync_turl->set_id(kInvalidTemplateURLID);
- TemplateURL* temp = scoped_sync_turl.release();
- Add(temp);
+ sync_turl->data_.id = kInvalidTemplateURLID;
+ Add(sync_turl);
if (delete_default)
- SetDefaultSearchProvider(temp);
+ SetDefaultSearchProvider(sync_turl);
}
} else {
// Change the local TURL's GUID to the server's GUID and push an update to
@@ -1831,6 +1866,7 @@
ResetTemplateURLGUID(local_turl, sync_turl->sync_guid());
SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
+ delete sync_turl;
}
}
@@ -1870,36 +1906,9 @@
TemplateURL* template_url = *i;
DCHECK(template_url);
if (template_url->sync_guid().empty()) {
- template_url->set_sync_guid(guid::GenerateGUID());
+ template_url->data_.sync_guid = guid::GenerateGUID();
if (service_.get())
service_->UpdateKeyword(*template_url);
}
}
}
-
-// static
-void TemplateURLService::UpdateTemplateURLWithSyncData(
- TemplateURL* dst,
- const SyncData& sync_data) {
- sync_pb::SearchEngineSpecifics specifics =
- sync_data.GetSpecifics().search_engine();
- dst->set_short_name(UTF8ToUTF16(specifics.short_name()));
- dst->set_keyword(UTF8ToUTF16(specifics.keyword()));
- dst->set_favicon_url(GURL(specifics.favicon_url()));
- dst->SetURL(specifics.url());
- dst->set_safe_for_autoreplace(specifics.safe_for_autoreplace());
- dst->set_originating_url(GURL(specifics.originating_url()));
- dst->set_date_created(
- base::Time::FromInternalValue(specifics.date_created()));
- std::vector<std::string> input_encodings;
- base::SplitString(specifics.input_encodings(), ';', &input_encodings);
- dst->set_input_encodings(input_encodings);
- dst->set_show_in_default_list(specifics.show_in_default_list());
- dst->SetSuggestionsURL(specifics.suggestions_url());
- dst->SetPrepopulateId(specifics.prepopulate_id());
- dst->set_autogenerate_keyword(specifics.autogenerate_keyword());
- dst->SetInstantURL(specifics.instant_url());
- dst->set_last_modified(
- base::Time::FromInternalValue(specifics.last_modified()));
- dst->set_sync_guid(specifics.sync_guid());
-}

Powered by Google App Engine
This is Rietveld 408576698