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…

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, 9 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

« chrome/browser/search_engines/template_url_parser.h ('K') | « chrome/browser/search_engines/template_url_service.h ('k') | chrome/browser/search_engines/template_url_service_sync_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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);

sky 2012/04/05 03:42:15 How come you're removing this?

Peter Kasting 2012/04/05 19:52:21 Not removing it -- it's moved up a bit, look on li

+ 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());