summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authorpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-05-08 01:34:25 +0000
committerpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-05-08 01:34:25 +0000
commit7b596f69d8211da4a1a642bbdf37ddbcf1546757 (patch)
tree13a9608a191373ac1d4f85fc48631a903dc77030 /chrome/browser
parent80745e361e61c62b38df34f5713f74c4a26a62f4 (diff)
downloadchromium_src-7b596f69d8211da4a1a642bbdf37ddbcf1546757.zip
chromium_src-7b596f69d8211da4a1a642bbdf37ddbcf1546757.tar.gz
chromium_src-7b596f69d8211da4a1a642bbdf37ddbcf1546757.tar.bz2
Remove the "autogenerate keyword" bit on TemplateURL.
This is a large change because a number of other significant pieces were either inherently tied to the above change, or too difficult to separate from it: * Because autogeneration was really only ever supposed to be used for the prepopulated Google entry, replace it with a system that auto-regenerates keywords for TemplateURLs that (a) use {google:baseURL} and (b) have a keyword that's currently "google.<TLD>" whenever the base URL changes. This means that users who manually create such TemplateURLs will automatically get the benefit of "autogenerated" keywords. * Pass information to the KeywordTable as bare TemplateURLData objects instead of TemplateURLs, now that the latter are not needed to perform keyword generation. * Remove the "autogenerate_keyword" column from the KeywordTable. This also takes the opportunity to remove the already-dead "logo_id" column (which I previously asked msw to leave in in order to only have to write the migration code once). This in turn requires adding version numbers to a lot of functions so they know which column set to use, as well as writing migration code to manually generate keywords for previously-autogenerated entries. * Migrate the "autogenerate_keyword" bit in data from prefs and sync as well. For sync this requires a variety of followon changes to send back ACTION_UPDATEs for migrated TemplateURLs and coalesce multiple SyncChanges to the same GUID. * Move various bits of TemplateURLService::GenerateKeyword() that were only used for the "autodetected on a webpage" case to the specific code for that case, in order to make GenerateKeyword() incapable of failing. This is important for the next item. * Remove the possibility for keywords to simply be empty. All TemplateURLs should now have a keyword, whether they were previously marked as "autogenerated" or not. While the UI already tried to guarantee this, the TemplateURL class itself and various pieces of TemplateURLService did not, or didn't deal correctly with exceptions. Enforcing this makes it much easier to reason about keywords and is important for the next item. * Guarantee that all keywords are unique, with one exception noted below. This allows callers to reliably refer to TemplateURLs by keyword; a future change will make AutocompleteMatch do precisely this. It also prevents weird edge cases in the UI and sync. * Exception: explicitly allow extension keywords to overlap with each other and with one non-extension TemplateURL. Previously, the behavior was somewhat random and buggy when we added and removed extensions that defined keywords, especially if we also tried to add/edit/remove keywords from the settings UI. We now define an explicit precedence order: non-replaceable TemplateURL > extension-provided TemplateURLs > replaceable TemplateURL; within the extensions section, the most recently-added extension wins. As we add and remove keywords, the current "TemplateURL for keyword" is always kept up-to-date according to this precedence order (so e.g. removing a later extension will "expose" an earlier one). BUG=none TEST=Adding extensions that specify omnibox keywords which conflict with local keywords should result in predictable behavior as described above; removing the extensions should restore the prior behavior. Review URL: https://chromiumcodereview.appspot.com/10381016 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@135775 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/importer/ie_importer.cc2
-rw-r--r--chrome/browser/importer/profile_import_process_messages.h6
-rw-r--r--chrome/browser/protector/default_search_provider_change.cc9
-rw-r--r--chrome/browser/protector/default_search_provider_change_browsertest.cc5
-rw-r--r--chrome/browser/search_engines/search_host_to_urls_map.cc2
-rw-r--r--chrome/browser/search_engines/search_provider_install_data.cc3
-rw-r--r--chrome/browser/search_engines/template_url.cc54
-rw-r--r--chrome/browser/search_engines/template_url.h57
-rw-r--r--chrome/browser/search_engines/template_url_parser.cc4
-rw-r--r--chrome/browser/search_engines/template_url_prepopulate_data.cc14
-rw-r--r--chrome/browser/search_engines/template_url_service.cc412
-rw-r--r--chrome/browser/search_engines/template_url_service.h59
-rw-r--r--chrome/browser/search_engines/template_url_service_sync_unittest.cc443
-rw-r--r--chrome/browser/search_engines/template_url_service_unittest.cc301
-rw-r--r--chrome/browser/search_engines/template_url_unittest.cc14
-rw-r--r--chrome/browser/search_engines/util.cc7
-rw-r--r--chrome/browser/tab_contents/render_view_context_menu.cc5
-rw-r--r--chrome/browser/ui/search_engines/search_engine_tab_helper.cc12
-rw-r--r--chrome/browser/webdata/keyword_table.cc242
-rw-r--r--chrome/browser/webdata/keyword_table.h26
-rw-r--r--chrome/browser/webdata/keyword_table_unittest.cc106
-rw-r--r--chrome/browser/webdata/web_data_service.cc25
-rw-r--r--chrome/browser/webdata/web_data_service.h8
-rw-r--r--chrome/browser/webdata/web_database.cc12
-rw-r--r--chrome/browser/webdata/web_database_migration_unittest.cc192
25 files changed, 1335 insertions, 685 deletions
diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc
index 93fde8c..052ffdd 100644
--- a/chrome/browser/importer/ie_importer.cc
+++ b/chrome/browser/importer/ie_importer.cc
@@ -586,7 +586,7 @@ void IEImporter::ImportSearchEngines() {
if (gurl.is_valid()) {
TemplateURLData data;
data.short_name = name;
- data.SetKeyword(TemplateURLService::GenerateKeyword(gurl, false));
+ data.SetKeyword(TemplateURLService::GenerateKeyword(gurl));
data.SetURL(url);
data.show_in_default_list = true;
t_iter = search_engines_map.insert(std::make_pair(url,
diff --git a/chrome/browser/importer/profile_import_process_messages.h b/chrome/browser/importer/profile_import_process_messages.h
index 93f62d0..9a6ed03 100644
--- a/chrome/browser/importer/profile_import_process_messages.h
+++ b/chrome/browser/importer/profile_import_process_messages.h
@@ -195,8 +195,7 @@ struct ParamTraits<TemplateURLData> {
typedef TemplateURLData param_type;
static void Write(Message* m, const param_type& p) {
WriteParam(m, p.short_name);
- WriteParam(m, p.raw_keyword());
- WriteParam(m, p.autogenerate_keyword());
+ WriteParam(m, p.keyword());
WriteParam(m, p.url());
WriteParam(m, p.suggestions_url);
WriteParam(m, p.instant_url);
@@ -215,11 +214,9 @@ struct ParamTraits<TemplateURLData> {
}
static bool Read(const Message* m, PickleIterator* iter, param_type* p) {
string16 keyword;
- bool autogenerate_keyword;
std::string url;
if (!ReadParam(m, iter, &p->short_name) ||
!ReadParam(m, iter, &keyword) ||
- !ReadParam(m, iter, &autogenerate_keyword) ||
!ReadParam(m, iter, &url) ||
!ReadParam(m, iter, &p->suggestions_url) ||
!ReadParam(m, iter, &p->instant_url) ||
@@ -237,7 +234,6 @@ struct ParamTraits<TemplateURLData> {
!ReadParam(m, iter, &p->sync_guid))
return false;
p->SetKeyword(keyword);
- p->SetAutogenerateKeyword(autogenerate_keyword);
p->SetURL(url);
return true;
}
diff --git a/chrome/browser/protector/default_search_provider_change.cc b/chrome/browser/protector/default_search_provider_change.cc
index a632f3f..d27e666 100644
--- a/chrome/browser/protector/default_search_provider_change.cc
+++ b/chrome/browser/protector/default_search_provider_change.cc
@@ -53,7 +53,7 @@ class TemplateURLIsSame {
if (!url || !other_)
return false;
return url->short_name() == other_->short_name() &&
- AreKeywordsSame(url, other_) &&
+ url->HasSameKeywordAs(*other_) &&
url->url() == other_->url() &&
url->suggestions_url() == other_->suggestions_url() &&
url->instant_url() == other_->instant_url() &&
@@ -65,13 +65,6 @@ class TemplateURLIsSame {
}
private:
- // Returns true if both |url1| and |url2| have autogenerated keywords
- // or if their keywords are identical.
- bool AreKeywordsSame(const TemplateURL* url1, const TemplateURL* url2) {
- return (url1->autogenerate_keyword() && url2->autogenerate_keyword()) ||
- url1->keyword() == url2->keyword();
- }
-
const TemplateURL* other_;
};
diff --git a/chrome/browser/protector/default_search_provider_change_browsertest.cc b/chrome/browser/protector/default_search_provider_change_browsertest.cc
index 0709478..9f32e9e 100644
--- a/chrome/browser/protector/default_search_provider_change_browsertest.cc
+++ b/chrome/browser/protector/default_search_provider_change_browsertest.cc
@@ -75,10 +75,7 @@ class DefaultSearchProviderChangeTest : public InProcessBrowserTest {
const std::string& url) {
TemplateURLData data;
data.short_name = short_name;
- if (keyword.empty())
- data.SetAutogenerateKeyword(true);
- else
- data.SetKeyword(keyword);
+ data.SetKeyword(keyword);
data.SetURL(url);
return new TemplateURL(browser()->profile(), data);
}
diff --git a/chrome/browser/search_engines/search_host_to_urls_map.cc b/chrome/browser/search_engines/search_host_to_urls_map.cc
index 878d108..1602011 100644
--- a/chrome/browser/search_engines/search_host_to_urls_map.cc
+++ b/chrome/browser/search_engines/search_host_to_urls_map.cc
@@ -27,6 +27,7 @@ void SearchHostToURLsMap::Add(TemplateURL* template_url,
const SearchTermsData& search_terms_data) {
DCHECK(initialized_);
DCHECK(template_url);
+ DCHECK(!template_url->IsExtensionKeyword());
const GURL url(TemplateURLService::GenerateSearchURLUsingTermsData(
template_url, search_terms_data));
@@ -39,6 +40,7 @@ void SearchHostToURLsMap::Add(TemplateURL* template_url,
void SearchHostToURLsMap::Remove(TemplateURL* template_url) {
DCHECK(initialized_);
DCHECK(template_url);
+ DCHECK(!template_url->IsExtensionKeyword());
const GURL url(TemplateURLService::GenerateSearchURL(template_url));
if (!url.is_valid() || !url.has_host())
diff --git a/chrome/browser/search_engines/search_provider_install_data.cc b/chrome/browser/search_engines/search_provider_install_data.cc
index defd639..28d691f 100644
--- a/chrome/browser/search_engines/search_provider_install_data.cc
+++ b/chrome/browser/search_engines/search_provider_install_data.cc
@@ -144,6 +144,7 @@ static bool IsSameOrigin(const GURL& requested_origin,
TemplateURL* template_url,
const SearchTermsData& search_terms_data) {
DCHECK(requested_origin == requested_origin.GetOrigin());
+ DCHECK(!template_url->IsExtensionKeyword());
return requested_origin ==
TemplateURLService::GenerateSearchURLUsingTermsData(template_url,
search_terms_data).GetOrigin();
@@ -264,6 +265,8 @@ void SearchProviderInstallData::SetDefault(const TemplateURL* template_url) {
return;
}
+ DCHECK(!template_url->IsExtensionKeyword());
+
IOThreadSearchTermsData search_terms_data(google_base_url_);
const GURL url(TemplateURLService::GenerateSearchURLUsingTermsData(
template_url, search_terms_data));
diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc
index b29c820..7a67ee6 100644
--- a/chrome/browser/search_engines/template_url.cc
+++ b/chrome/browser/search_engines/template_url.cc
@@ -13,6 +13,7 @@
#include "base/stringprintf.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/autocomplete/autocomplete_field_trial.h"
+#include "chrome/browser/google/google_util.h"
#include "chrome/browser/search_engines/search_terms_data.h"
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/common/guid.h"
@@ -535,41 +536,19 @@ TemplateURLData::TemplateURLData()
usage_count(0),
prepopulate_id(0),
sync_guid(guid::GenerateGUID()),
- url_("x"),
- autogenerate_keyword_(false),
- keyword_generated_(false) {
+ keyword_(ASCIIToUTF16("dummy")),
+ url_("x") {
}
TemplateURLData::~TemplateURLData() {
}
void TemplateURLData::SetKeyword(const string16& keyword) {
+ DCHECK(!keyword.empty());
+
// Case sensitive keyword matching is confusing. As such, we force all
// keywords to be lower case.
keyword_ = base::i18n::ToLower(keyword);
- autogenerate_keyword_ = false;
-}
-
-const string16& TemplateURLData::keyword(TemplateURL* t_url) const {
- EnsureKeyword(t_url);
- return keyword_;
-}
-
-void TemplateURLData::SetAutogenerateKeyword(bool autogenerate_keyword) {
- autogenerate_keyword_ = autogenerate_keyword;
- if (autogenerate_keyword_) {
- keyword_.clear();
- keyword_generated_ = false;
- }
-}
-
-void TemplateURLData::EnsureKeyword(TemplateURL* t_url) const {
- if (autogenerate_keyword_ && !keyword_generated_) {
- // Generate a keyword and cache it.
- keyword_ = base::i18n::ToLower(TemplateURLService::GenerateKeyword(
- TemplateURLService::GenerateSearchURL(t_url).GetWithEmptyPath(), true));
- keyword_generated_ = true;
- }
}
void TemplateURLData::SetURL(const std::string& url) {
@@ -654,6 +633,18 @@ bool TemplateURL::SupportsReplacementUsingTermsData(
return url_ref_.SupportsReplacementUsingTermsData(search_terms_data);
}
+bool TemplateURL::IsGoogleSearchURLWithReplaceableKeyword() const {
+ return !IsExtensionKeyword() && url_ref_.HasGoogleBaseURLs() &&
+ google_util::IsGoogleHostname(UTF16ToUTF8(data_.keyword()),
+ google_util::DISALLOW_SUBDOMAIN);
+}
+
+bool TemplateURL::HasSameKeywordAs(const TemplateURL& other) const {
+ return (data_.keyword() == other.data_.keyword()) ||
+ (IsGoogleSearchURLWithReplaceableKeyword() &&
+ other.IsGoogleSearchURLWithReplaceableKeyword());
+}
+
std::string TemplateURL::GetExtensionId() const {
DCHECK(IsExtensionKeyword());
return GURL(data_.url()).host();
@@ -676,9 +667,18 @@ void TemplateURL::SetPrepopulateId(int id) {
instant_url_ref_.prepopulated_ = prepopulated;
}
+void TemplateURL::ResetKeywordIfNecessary(bool force) {
+ if (IsGoogleSearchURLWithReplaceableKeyword() || force) {
+ DCHECK(!IsExtensionKeyword());
+ GURL url(TemplateURLService::GenerateSearchURL(this));
+ if (url.is_valid())
+ data_.SetKeyword(TemplateURLService::GenerateKeyword(url));
+ }
+}
+
void TemplateURL::InvalidateCachedValues() {
url_ref_.InvalidateCachedValues();
suggestions_url_ref_.InvalidateCachedValues();
instant_url_ref_.InvalidateCachedValues();
- data_.SetAutogenerateKeyword(data_.autogenerate_keyword());
+ ResetKeywordIfNecessary(false);
}
diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h
index 825a60f..f3de598 100644
--- a/chrome/browser/search_engines/template_url.h
+++ b/chrome/browser/search_engines/template_url.h
@@ -238,24 +238,9 @@ struct TemplateURLData {
// shows this when the user selects a substituting match.
string16 short_name;
- // The shortcut for this TemplateURL. May be empty.
+ // The shortcut for this TemplateURL. |keyword| must be non-empty.
void SetKeyword(const string16& keyword);
- const string16& keyword(TemplateURL* t_url) const;
- // TODO(pkasting): This should only be necessary until we eliminate keyword
- // autogeneration.
- const string16& raw_keyword() const { return keyword_; }
-
- // Whether to autogenerate a keyword in TemplateURL::GetKeyword(). Most
- // consumers should not need this.
- // NOTE: Calling SetKeyword() turns this back off. Manual and automatic
- // keywords are mutually exclusive.
- void SetAutogenerateKeyword(bool autogenerate_keyword);
- bool autogenerate_keyword() const { return autogenerate_keyword_; }
-
- // Ensures that the keyword is generated. Most consumers should not need this
- // because it is done automatically. Use this method on the UI thread, so
- // the keyword may be accessed on another thread.
- void EnsureKeyword(TemplateURL* t_url) const;
+ const string16& keyword() const { return keyword_; }
// The raw URL for the TemplateURL, which may not be valid as-is (e.g. because
// it requires substitutions first). This must be non-empty.
@@ -325,15 +310,8 @@ struct TemplateURLData {
private:
// Private so we can enforce using the setters and thus enforce that these
// fields are never empty.
- // TODO(pkasting): |keyword_| is temporarily still allowed to be empty.
- mutable string16 keyword_;
+ string16 keyword_;
std::string url_;
-
- // TODO(pkasting): These fields will go away soon.
- bool autogenerate_keyword_;
- // True if the keyword was generated. This is used to avoid multiple attempts
- // if generating a keyword failed.
- mutable bool keyword_generated_;
};
@@ -372,18 +350,7 @@ class TemplateURL {
// displayed even if it is LTR and the UI is RTL.
string16 AdjustedShortNameForLocaleDirection() const;
- const string16& keyword() const {
- // TODO(pkasting): See comment on EnsureKeyword() below.
- return data_.keyword(const_cast<TemplateURL*>(this));
- }
- bool autogenerate_keyword() const {
- return data_.autogenerate_keyword();
- }
- void EnsureKeyword() const {
- // TODO(pkasting): EnsureKeyword() will die soon so it's not worth jumping
- // through hoops to fix this const_cast<>().
- data_.EnsureKeyword(const_cast<TemplateURL*>(this));
- }
+ const string16& keyword() const { return data_.keyword(); }
const std::string& url() const { return data_.url(); }
const std::string& suggestions_url() const { return data_.suggestions_url; }
@@ -429,6 +396,15 @@ class TemplateURL {
bool SupportsReplacementUsingTermsData(
const SearchTermsData& search_terms_data) const;
+ // Returns true if this TemplateURL uses Google base URLs and has a keyword
+ // of "google.TLD". We use this to decide whether we can automatically
+ // update the keyword to reflect the current Google base URL TLD.
+ bool IsGoogleSearchURLWithReplaceableKeyword() const;
+
+ // Returns true if the keywords match or if
+ // IsGoogleSearchURLWithReplaceableKeyword() is true for both TemplateURLs.
+ bool HasSameKeywordAs(const TemplateURL& other) const;
+
std::string GetExtensionId() const;
bool IsExtensionKeyword() const;
@@ -438,6 +414,13 @@ class TemplateURL {
void SetURL(const std::string& url);
void SetPrepopulateId(int id);
+ // Resets the keyword if IsGoogleSearchURLWithReplaceableKeyword() or |force|.
+ // The |force| parameter is useful when the existing keyword is known to be
+ // a placeholder. The resulting keyword is generated using
+ // TemplateURLService::GenerateSearchURL() and
+ // TemplateURLService::GenerateKeyword().
+ void ResetKeywordIfNecessary(bool force);
+
// Invalidates cached values on this object and its child TemplateURLRefs.
void InvalidateCachedValues();
diff --git a/chrome/browser/search_engines/template_url_parser.cc b/chrome/browser/search_engines/template_url_parser.cc
index a9f6939..f48eb39 100644
--- a/chrome/browser/search_engines/template_url_parser.cc
+++ b/chrome/browser/search_engines/template_url_parser.cc
@@ -309,9 +309,7 @@ TemplateURL* TemplateURLParsingContext::GetTemplateURL(
if (suggestion_method_ == TemplateURLParsingContext::POST)
data_.suggestions_url.clear();
- string16 keyword(TemplateURLService::GenerateKeyword(url, false));
- DCHECK(!keyword.empty());
- data_.SetKeyword(keyword);
+ data_.SetKeyword(TemplateURLService::GenerateKeyword(url));
data_.show_in_default_list = show_in_default_list;
return new TemplateURL(profile, data_);
}
diff --git a/chrome/browser/search_engines/template_url_prepopulate_data.cc b/chrome/browser/search_engines/template_url_prepopulate_data.cc
index a4a893d..3390b01 100644
--- a/chrome/browser/search_engines/template_url_prepopulate_data.cc
+++ b/chrome/browser/search_engines/template_url_prepopulate_data.cc
@@ -44,9 +44,6 @@ namespace {
struct PrepopulatedEngine {
const wchar_t* const name;
- // If empty, we'll autogenerate a keyword based on the search_url every time
- // someone asks. Only entries which need keywords to auto-track a dynamically
- // generated search URL should use this.
const wchar_t* const keyword;
const char* const favicon_url; // If NULL, there is no favicon.
const char* const search_url;
@@ -1085,7 +1082,7 @@ const PrepopulatedEngine goo = {
const PrepopulatedEngine google = {
L"Google",
- L"",
+ L"google.com", // This will be dynamically updated by the TemplateURL system.
"http://www.google.com/favicon.ico",
"{google:baseURL}search?{google:RLZ}{google:acceptedSuggestion}"
"{google:originalQueryForSuggestion}{google:searchFieldtrialParameter}"
@@ -3158,10 +3155,7 @@ TemplateURL* MakePrepopulatedTemplateURL(Profile* profile,
int id) {
TemplateURLData data;
data.short_name = name;
- if (keyword.empty())
- data.SetAutogenerateKeyword(true);
- else
- data.SetKeyword(keyword);
+ data.SetKeyword(keyword);
data.SetURL(search_url.as_string());
data.suggestions_url = suggest_url.as_string();
data.instant_url = instant_url.as_string();
@@ -3208,8 +3202,8 @@ void GetPrepopulatedTemplateFromPrefs(Profile* profile,
engine->Get("encoding", &val) && val->GetAsString(&encoding) &&
engine->Get("id", &val) && val->GetAsInteger(&id)) {
// These next fields are not allowed to be empty.
- if (name.empty() || search_url.empty() || favicon_url.empty() ||
- encoding.empty())
+ if (name.empty() || keyword.empty() || search_url.empty() ||
+ favicon_url.empty() || encoding.empty())
return;
} else {
// Got a parsing error. No big deal.
diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc
index 7bdbe87..fcb6e79 100644
--- a/chrome/browser/search_engines/template_url_service.cc
+++ b/chrome/browser/search_engines/template_url_service.cc
@@ -70,7 +70,7 @@ bool TemplateURLsHaveSamePrefs(const TemplateURL* url1,
return true;
return (url1 != NULL) && (url2 != NULL) &&
(url1->short_name() == url2->short_name()) &&
- (url1->keyword() == url2->keyword()) &&
+ url1->HasSameKeywordAs(*url2) &&
(url1->url() == url2->url()) &&
(url1->suggestions_url() == url2->suggestions_url()) &&
(url1->instant_url() == url2->instant_url()) &&
@@ -90,6 +90,40 @@ TemplateURL* FirstPotentialDefaultEngine(
return NULL;
}
+// If |change_list| contains ACTION_UPDATEs followed by more ACTION_UPDATEs or
+// ACTION_ADDs for the same GUID, remove all but the last one.
+//
+// Removing UPDATE before ADD is important. This can happen if
+// ResolveSyncKeywordConflict() changes a local TemplateURL that hasn't actually
+// been seen by the server yet. In this case sending the UPDATE first might
+// confuse the sync server.
+//
+// Removing UPDATE before UPDATE, OTOH, is not really necessary as the server
+// will coalesce these before other clients see them; however it's easy to do in
+// conjunction with the filtering for UPDATE-before-ADD and saves bandwidth.
+void PreventDuplicateGUIDUpdates(SyncChangeList* change_list) {
+ for (size_t i = change_list->size(); i > 1; ) {
+ --i; // Prevent underflow that could occur if we did this in the loop body.
+ const SyncChange& change_i = (*change_list)[i];
+ if ((change_i.change_type() != SyncChange::ACTION_ADD) &&
+ (change_i.change_type() != SyncChange::ACTION_UPDATE))
+ continue;
+ std::string guid(
+ change_i.sync_data().GetSpecifics().search_engine().sync_guid());
+ for (size_t j = 0; j < i; ) {
+ const SyncChange& change_j = (*change_list)[j];
+ if ((change_j.change_type() == SyncChange::ACTION_UPDATE) &&
+ (change_j.sync_data().GetSpecifics().search_engine().sync_guid() ==
+ guid)) {
+ change_list->erase(change_list->begin() + j);
+ --i;
+ } else {
+ ++j;
+ }
+ }
+ }
+}
+
} // namespace
@@ -160,22 +194,8 @@ TemplateURLService::~TemplateURLService() {
}
// static
-string16 TemplateURLService::GenerateKeyword(const GURL& url,
- bool autodetected) {
- // Don't autogenerate keywords for referrers that are the result of a form
- // submission (TODO: right now we approximate this by checking for the URL
- // having a query, but we should replace this with a call to WebCore to see if
- // the originating page was actually a form submission), anything other than
- // http, or referrers with a path.
- //
- // If we relax the path constraint, we need to be sure to sanitize the path
- // elements and update AutocompletePopup to look for keywords using the path.
- // See http://b/issue?id=863583.
- if (!url.is_valid() ||
- (autodetected && (url.has_query() || !url.SchemeIs(chrome::kHttpScheme) ||
- ((url.path() != "") && (url.path() != "/")))))
- return string16();
-
+string16 TemplateURLService::GenerateKeyword(const GURL& url) {
+ DCHECK(url.is_valid());
// Strip "www." off the front of the keyword; otherwise the keyword won't work
// properly. See http://code.google.com/p/chromium/issues/detail?id=6984 .
// Special case: if the host was exactly "www." (not sure this can happen but
@@ -231,10 +251,10 @@ GURL TemplateURLService::GenerateSearchURLUsingTermsData(
const TemplateURL* t_url,
const SearchTermsData& search_terms_data) {
DCHECK(t_url);
+ DCHECK(!t_url->IsExtensionKeyword());
+
const TemplateURLRef& search_ref = t_url->url_ref();
- // Extension keywords don't have host-based search URLs.
- if (!search_ref.IsValidUsingTermsData(search_terms_data) ||
- t_url->IsExtensionKeyword())
+ if (!search_ref.IsValidUsingTermsData(search_terms_data))
return GURL();
if (!search_ref.SupportsReplacementUsingTermsData(search_terms_data))
@@ -307,18 +327,19 @@ TemplateURL* TemplateURLService::GetTemplateURLForKeyword(
keyword_to_template_map_.find(keyword));
if (elem != keyword_to_template_map_.end())
return elem->second;
- return (initial_default_search_provider_.get() &&
+ return ((!loaded_ || load_failed_) &&
+ initial_default_search_provider_.get() &&
(initial_default_search_provider_->keyword() == keyword)) ?
initial_default_search_provider_.get() : NULL;
}
TemplateURL* TemplateURLService::GetTemplateURLForGUID(
const std::string& sync_guid) {
- GUIDToTemplateMap::const_iterator elem(
- guid_to_template_map_.find(sync_guid));
+ GUIDToTemplateMap::const_iterator elem(guid_to_template_map_.find(sync_guid));
if (elem != guid_to_template_map_.end())
return elem->second;
- return (initial_default_search_provider_.get() &&
+ return ((!loaded_ || load_failed_) &&
+ initial_default_search_provider_.get() &&
(initial_default_search_provider_->sync_guid() == sync_guid)) ?
initial_default_search_provider_.get() : NULL;
}
@@ -328,14 +349,15 @@ TemplateURL* TemplateURLService::GetTemplateURLForHost(
TemplateURL* t_url = provider_map_->GetTemplateURLForHost(host);
if (t_url)
return t_url;
- return (initial_default_search_provider_.get() &&
+ return ((!loaded_ || load_failed_) &&
+ initial_default_search_provider_.get() &&
(GenerateSearchURL(initial_default_search_provider_.get()).host() ==
host)) ? initial_default_search_provider_.get() : NULL;
}
void TemplateURLService::Add(TemplateURL* template_url) {
- AddNoNotify(template_url, true);
- NotifyObservers();
+ if (AddNoNotify(template_url, true))
+ NotifyObservers();
}
void TemplateURLService::AddAndSetProfile(TemplateURL* template_url,
@@ -348,6 +370,7 @@ void TemplateURLService::AddWithOverrides(TemplateURL* template_url,
const string16& short_name,
const string16& keyword,
const std::string& url) {
+ DCHECK(!keyword.empty());
DCHECK(!url.empty());
template_url->data_.short_name = short_name;
template_url->data_.SetKeyword(keyword);
@@ -447,20 +470,25 @@ TemplateURLService::TemplateURLVector TemplateURLService::GetTemplateURLs() {
}
void TemplateURLService::IncrementUsageCount(TemplateURL* url) {
- DCHECK(url && std::find(template_urls_.begin(), template_urls_.end(), url) !=
- template_urls_.end());
+ DCHECK(url);
+ if (std::find(template_urls_.begin(), template_urls_.end(), url) ==
+ template_urls_.end())
+ return;
++url->data_.usage_count;
// Extension keywords are not persisted.
// TODO(mpcomplete): If we allow editing extension keywords, then those should
// be persisted to disk and synced.
if (service_.get() && !url->IsExtensionKeyword())
- service_.get()->UpdateKeyword(*url);
+ service_.get()->UpdateKeyword(url->data());
}
void TemplateURLService::ResetTemplateURL(TemplateURL* url,
const string16& title,
const string16& keyword,
const std::string& search_url) {
+ if (!loaded_)
+ return;
+ DCHECK(!keyword.empty());
DCHECK(!search_url.empty());
TemplateURLData data(url->data());
data.short_name = title;
@@ -473,8 +501,8 @@ void TemplateURLService::ResetTemplateURL(TemplateURL* url,
data.safe_for_autoreplace = false;
data.last_modified = time_provider_();
TemplateURL new_url(url->profile(), data);
- UpdateNoNotify(url, new_url);
- NotifyObservers();
+ if (UpdateNoNotify(url, new_url))
+ NotifyObservers();
}
bool TemplateURLService::CanMakeDefault(const TemplateURL* url) {
@@ -489,8 +517,8 @@ void TemplateURLService::SetDefaultSearchProvider(TemplateURL* url) {
// Always persist the setting in the database, that way if the backup
// signature has changed out from under us it gets reset correctly.
- SetDefaultSearchProviderNoNotify(url);
- NotifyObservers();
+ if (SetDefaultSearchProviderNoNotify(url))
+ NotifyObservers();
}
TemplateURL* TemplateURLService::GetDefaultSearchProvider() {
@@ -611,14 +639,17 @@ void TemplateURLService::OnWebDataServiceRequestDone(
data.created_by_policy = true;
data.id = kInvalidTemplateURLID;
default_search_provider = new TemplateURL(profile_, data);
- AddNoNotify(default_search_provider, true);
+ if (!AddNoNotify(default_search_provider, true))
+ default_search_provider = NULL;
}
}
// Note that this saves the default search provider to prefs.
if (!default_search_provider ||
(!default_search_provider->IsExtensionKeyword() &&
- default_search_provider->SupportsReplacement()))
- SetDefaultSearchProviderNoNotify(default_search_provider);
+ default_search_provider->SupportsReplacement())) {
+ bool success = SetDefaultSearchProviderNoNotify(default_search_provider);
+ DCHECK(success);
+ }
} else {
// If we had a managed default, replace it with the synced default if
// applicable, or the first provider of the list.
@@ -699,8 +730,11 @@ void TemplateURLService::OnWebDataServiceRequestDone(
UMA_HISTOGRAM_BOOLEAN("Search.HasDefaultSearchProvider",
has_default_search_provider);
// Ensure that default search provider exists. See http://crbug.com/116952.
- if (!has_default_search_provider)
- SetDefaultSearchProviderNoNotify(FindNewDefaultSearchProvider());
+ if (!has_default_search_provider) {
+ bool success =
+ SetDefaultSearchProviderNoNotify(FindNewDefaultSearchProvider());
+ DCHECK(success);
+ }
}
NotifyObservers();
@@ -726,7 +760,7 @@ void TemplateURLService::Observe(int type,
const content::NotificationDetails& details) {
if (type == chrome::NOTIFICATION_HISTORY_URL_VISITED) {
content::Details<history::URLVisitedDetails> visit_details(details);
- if (!loaded())
+ if (!loaded_)
visits_to_add_.push_back(*visit_details.ptr());
else
UpdateKeywordSearchTermsForURL(*visit_details.ptr());
@@ -792,6 +826,7 @@ SyncError TemplateURLService::ProcessSyncChanges(
syncable::SEARCH_ENGINES);
return error;
}
+ DCHECK(loaded_);
AutoReset<bool> processing_changes(&processing_syncer_changes_, true);
@@ -809,9 +844,13 @@ SyncError TemplateURLService::ProcessSyncChanges(
if (!turl.get())
continue;
+ // Explicitly don't check for conflicts against extension keywords; in this
+ // case the functions which modify the keyword map know how to handle the
+ // conflicts.
+ // TODO(mpcomplete): If we allow editing extension keywords, then those will
+ // need to undergo conflict resolution.
TemplateURL* existing_keyword_turl =
- GetTemplateURLForKeyword(turl->keyword());
-
+ FindNonExtensionTemplateURLForKeyword(turl->keyword());
if (iter->change_type() == SyncChange::ACTION_DELETE && existing_turl) {
bool delete_default = (existing_turl == GetDefaultSearchProvider());
@@ -829,8 +868,10 @@ SyncError TemplateURLService::ProcessSyncChanges(
} else if (iter->change_type() == SyncChange::ACTION_ADD &&
!existing_turl) {
std::string guid = turl->sync_guid();
- if (existing_keyword_turl)
- ResolveSyncKeywordConflict(turl.get(), &new_changes);
+ if (existing_keyword_turl) {
+ ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl,
+ &new_changes);
+ }
// Force the local ID to kInvalidTemplateURLID so we can add it.
TemplateURLData data(turl->data());
data.id = kInvalidTemplateURLID;
@@ -842,10 +883,12 @@ SyncError TemplateURLService::ProcessSyncChanges(
existing_turl) {
// Possibly resolve a keyword conflict if they have the same keywords but
// are not the same entry.
- if (existing_keyword_turl && existing_keyword_turl != existing_turl)
- ResolveSyncKeywordConflict(turl.get(), &new_changes);
- UpdateNoNotify(existing_turl, *turl);
- NotifyObservers();
+ if (existing_keyword_turl && existing_keyword_turl != existing_turl) {
+ ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl,
+ &new_changes);
+ }
+ if (UpdateNoNotify(existing_turl, *turl))
+ NotifyObservers();
} else {
// Something really unexpected happened. Either we received an
// ACTION_INVALID, or Sync is in a crazy state:
@@ -858,6 +901,7 @@ SyncError TemplateURLService::ProcessSyncChanges(
SyncChange::ChangeTypeToString(iter->change_type()));
}
}
+ PreventDuplicateGUIDUpdates(&new_changes);
// If something went wrong, we want to prematurely exit to avoid pushing
// inconsistent data to Sync. We return the last error we received.
@@ -874,7 +918,7 @@ SyncError TemplateURLService::MergeDataAndStartSyncing(
const SyncDataList& initial_sync_data,
scoped_ptr<SyncChangeProcessor> sync_processor,
scoped_ptr<SyncErrorFactory> sync_error_factory) {
- DCHECK(loaded());
+ DCHECK(loaded_);
DCHECK_EQ(type, syncable::SEARCH_ENGINES);
DCHECK(!sync_processor_.get());
DCHECK(sync_processor.get());
@@ -924,8 +968,8 @@ SyncError TemplateURLService::MergeDataAndStartSyncing(
// 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.
- UpdateNoNotify(local_turl, *sync_turl);
- NotifyObservers();
+ if (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
// data fields.
@@ -949,8 +993,14 @@ SyncError TemplateURLService::MergeDataAndStartSyncing(
// Keyword conflict is possible in this case. Resolve it first before
// adding the new TemplateURL. Note that we don't remove the local TURL
// from local_data_map in this case as it may still need to be pushed to
- // the cloud.
- ResolveSyncKeywordConflict(sync_turl.get(), &new_changes);
+ // the cloud. We also explicitly don't resolve conflicts against
+ // extension keywords; see comments in ProcessSyncChanges().
+ TemplateURL* existing_keyword_turl =
+ FindNonExtensionTemplateURLForKeyword(sync_turl->keyword());
+ if (existing_keyword_turl) {
+ ResolveSyncKeywordConflict(sync_turl.get(), existing_keyword_turl,
+ &new_changes);
+ }
// Force the local ID to kInvalidTemplateURLID so we can add it.
TemplateURLData data(sync_turl->data());
data.id = kInvalidTemplateURLID;
@@ -969,6 +1019,8 @@ SyncError TemplateURLService::MergeDataAndStartSyncing(
new_changes.push_back(SyncChange(SyncChange::ACTION_ADD, iter->second));
}
+ PreventDuplicateGUIDUpdates(&new_changes);
+
SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes);
if (error.IsSet())
return error;
@@ -1031,7 +1083,6 @@ SyncData TemplateURLService::CreateSyncDataFromTemplateURL(
se_specifics->set_show_in_default_list(turl.show_in_default_list());
se_specifics->set_suggestions_url(turl.suggestions_url());
se_specifics->set_prepopulate_id(turl.prepopulate_id());
- se_specifics->set_autogenerate_keyword(turl.autogenerate_keyword());
se_specifics->set_instant_url(turl.instant_url());
se_specifics->set_last_modified(turl.last_modified().ToInternalValue());
se_specifics->set_sync_guid(turl.sync_guid());
@@ -1061,8 +1112,17 @@ TemplateURL* TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(
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());
+ string16 keyword(UTF8ToUTF16(specifics.keyword()));
+ // NOTE: Once this code has shipped in a couple of stable releases, we can
+ // probably remove the migration portion, comment out the
+ // "autogenerate_keyword" field entirely in the .proto file, and fold the
+ // empty keyword case into the "delete data" block above.
+ bool reset_keyword =
+ specifics.autogenerate_keyword() || specifics.keyword().empty();
+ if (reset_keyword)
+ keyword = ASCIIToUTF16("dummy"); // Will be replaced below.
+ DCHECK(!keyword.empty());
+ data.SetKeyword(keyword);
data.SetURL(specifics.url());
data.suggestions_url = specifics.suggestions_url();
data.instant_url = specifics.instant_url();
@@ -1082,6 +1142,11 @@ TemplateURL* TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(
TemplateURL* turl = new TemplateURL(profile, data);
DCHECK(!turl->IsExtensionKeyword());
+ if (reset_keyword) {
+ turl->ResetKeywordIfNecessary(true);
+ SyncData sync_data = CreateSyncDataFromTemplateURL(*turl);
+ change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
+ }
return turl;
}
@@ -1167,8 +1232,31 @@ void TemplateURLService::Init(const Initializer* initializers,
}
void TemplateURLService::RemoveFromMaps(TemplateURL* template_url) {
- if (!template_url->keyword().empty())
- keyword_to_template_map_.erase(template_url->keyword());
+ const string16& keyword = template_url->keyword();
+ DCHECK_NE(0U, keyword_to_template_map_.count(keyword));
+ if (keyword_to_template_map_[keyword] == template_url) {
+ // We need to check whether the keyword can now be provided by another
+ // TemplateURL. See the comments in AddToMaps() for more information on
+ // extension keywords and how they can coexist with non-extension keywords.
+ // In the case of more than one extension, we use the most recently
+ // installed (which will be the most recently added, which will have the
+ // highest ID).
+ TemplateURL* best_fallback = NULL;
+ for (TemplateURLVector::const_iterator i(template_urls_.begin());
+ i != template_urls_.end(); ++i) {
+ TemplateURL* turl = *i;
+ // This next statement relies on the fact that there can only be one
+ // non-extension TemplateURL with a given keyword.
+ if ((turl != template_url) && (turl->keyword() == keyword) &&
+ (!best_fallback || !best_fallback->IsExtensionKeyword() ||
+ (turl->IsExtensionKeyword() && (turl->id() > best_fallback->id()))))
+ best_fallback = turl;
+ }
+ if (best_fallback)
+ keyword_to_template_map_[keyword] = best_fallback;
+ else
+ keyword_to_template_map_.erase(keyword);
+ }
// If the keyword we're removing is from an extension, we're now done, since
// it won't be synced or stored in the provider map.
@@ -1198,15 +1286,31 @@ void TemplateURLService::RemoveFromKeywordMapByPointer(
}
void TemplateURLService::AddToMaps(TemplateURL* template_url) {
- if (!template_url->keyword().empty())
- keyword_to_template_map_[template_url->keyword()] = template_url;
+ bool template_extension = template_url->IsExtensionKeyword();
+ const string16& keyword = template_url->keyword();
+ KeywordToTemplateMap::const_iterator i =
+ keyword_to_template_map_.find(keyword);
+ if (i == keyword_to_template_map_.end()) {
+ keyword_to_template_map_[keyword] = template_url;
+ } else {
+ const TemplateURL* existing_url = i->second;
+ // We should only have overlapping keywords when at least one comes from
+ // an extension. In that case, the ranking order is:
+ // Manually-modified keywords > extension keywords > replaceable keywords
+ // When there are multiple extensions, the last-added wins.
+ bool existing_extension = existing_url->IsExtensionKeyword();
+ DCHECK(existing_extension || template_extension);
+ if (existing_extension ?
+ !CanReplace(template_url) : CanReplace(existing_url))
+ keyword_to_template_map_[keyword] = template_url;
+ }
// Extension keywords are not synced, so they don't go into the GUID map,
// and do not use host-based search URLs, so they don't go into the provider
// map, so at this point we're done.
// TODO(mpcomplete): If we allow editing extension keywords, then those should
// be persisted to disk and synced.
- if (template_url->IsExtensionKeyword())
+ if (template_extension)
return;
if (!template_url->sync_guid().empty())
@@ -1326,6 +1430,13 @@ bool TemplateURLService::LoadDefaultSearchProviderFromPrefs(
UTF8ToUTF16(prefs->GetString(prefs::kDefaultSearchProviderName));
string16 keyword =
UTF8ToUTF16(prefs->GetString(prefs::kDefaultSearchProviderKeyword));
+ // Force keyword to be non-empty.
+ // TODO(pkasting): This is only necessary as long as we're potentially loading
+ // older prefs where empty keywords are theoretically possible. Eventually
+ // this code can be replaced with a DCHECK(!keyword.empty());.
+ bool update_keyword = keyword.empty();
+ if (update_keyword)
+ keyword = ASCIIToUTF16("dummy");
std::string search_url =
prefs->GetString(prefs::kDefaultSearchProviderSearchURL);
// Force URL to be non-empty. We've never supported this case, but past bugs
@@ -1366,6 +1477,8 @@ bool TemplateURLService::LoadDefaultSearchProviderFromPrefs(
}
default_provider->reset(new TemplateURL(profile_, data));
DCHECK(!(*default_provider)->IsExtensionKeyword());
+ if (update_keyword)
+ (*default_provider)->ResetKeywordIfNecessary(true);
return true;
}
@@ -1391,12 +1504,28 @@ bool TemplateURLService::CanReplace(const TemplateURL* t_url) {
t_url->safe_for_autoreplace());
}
-void TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
+TemplateURL* TemplateURLService::FindNonExtensionTemplateURLForKeyword(
+ const string16& keyword) {
+ TemplateURL* keyword_turl = GetTemplateURLForKeyword(keyword);
+ if (!keyword_turl || !keyword_turl->IsExtensionKeyword())
+ return keyword_turl;
+ // The extension keyword in the model may be hiding a replaceable
+ // non-extension keyword. Look for it.
+ for (TemplateURLVector::const_iterator i(template_urls_.begin());
+ i != template_urls_.end(); ++i) {
+ if (!(*i)->IsExtensionKeyword() && ((*i)->keyword() == keyword))
+ return *i;
+ }
+ return NULL;
+}
+
+bool TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
const TemplateURL& new_values) {
DCHECK(loaded_);
DCHECK(existing_turl);
- DCHECK(std::find(template_urls_.begin(), template_urls_.end(),
- existing_turl) != template_urls_.end());
+ if (std::find(template_urls_.begin(), template_urls_.end(), existing_turl) ==
+ template_urls_.end())
+ return false;
// TODO(mpcomplete): If we allow editing extension keywords, then those should
// be persisted to disk and synced. In this case this DCHECK should be
@@ -1404,8 +1533,7 @@ void TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
DCHECK(!existing_turl->IsExtensionKeyword());
string16 old_keyword(existing_turl->keyword());
- if (!old_keyword.empty())
- keyword_to_template_map_.erase(old_keyword);
+ keyword_to_template_map_.erase(old_keyword);
if (!existing_turl->sync_guid().empty())
guid_to_template_map_.erase(existing_turl->sync_guid());
@@ -1417,19 +1545,47 @@ void TemplateURLService::UpdateNoNotify(TemplateURL* existing_turl,
provider_map_->Add(existing_turl, search_terms_data);
const string16& keyword = existing_turl->keyword();
- if (!keyword.empty())
+ KeywordToTemplateMap::const_iterator i =
+ keyword_to_template_map_.find(keyword);
+ if (i == keyword_to_template_map_.end()) {
keyword_to_template_map_[keyword] = existing_turl;
+ } else {
+ // We can theoretically reach here in two cases:
+ // * There is an existing extension keyword and sync brings in a rename of
+ // a non-extension keyword to match. In this case we just need to pick
+ // which keyword has priority to update the keyword map.
+ // * Autogeneration of the keyword for a Google default search provider
+ // at load time causes it to conflict with an existing keyword. In this
+ // case we delete the existing keyword if it's replaceable, or else undo
+ // the change in keyword for |existing_turl|.
+ DCHECK(!existing_turl->IsExtensionKeyword());
+ TemplateURL* existing_keyword_turl = i->second;
+ if (existing_keyword_turl->IsExtensionKeyword()) {
+ if (!CanReplace(existing_turl))
+ keyword_to_template_map_[keyword] = existing_turl;
+ } else {
+ if (CanReplace(existing_keyword_turl)) {
+ RemoveNoNotify(existing_keyword_turl);
+ } else {
+ existing_turl->data_.SetKeyword(old_keyword);
+ keyword_to_template_map_[old_keyword] = existing_turl;
+ }
+ }
+ }
if (!existing_turl->sync_guid().empty())
guid_to_template_map_[existing_turl->sync_guid()] = existing_turl;
if (service_.get())
- service_->UpdateKeyword(*existing_turl);
+ service_->UpdateKeyword(existing_turl->data());
// Inform sync of the update.
ProcessTemplateURLChange(existing_turl, SyncChange::ACTION_UPDATE);
- if (default_search_provider_ == existing_turl)
- SetDefaultSearchProviderNoNotify(existing_turl);
+ if (default_search_provider_ == existing_turl) {
+ bool success = SetDefaultSearchProviderNoNotify(existing_turl);
+ DCHECK(success);
+ }
+ return true;
}
PrefService* TemplateURLService::GetPrefs() {
@@ -1564,9 +1720,21 @@ void TemplateURLService::GoogleBaseURLChanged() {
string16 original_keyword(t_url->keyword());
t_url->InvalidateCachedValues();
const string16& new_keyword(t_url->keyword());
+ KeywordToTemplateMap::const_iterator i =
+ keyword_to_template_map_.find(new_keyword);
+ if ((i != keyword_to_template_map_.end()) && (i->second != t_url)) {
+ // The new autogenerated keyword conflicts with another TemplateURL.
+ // Overwrite it if it's replaceable; otherwise just reset |t_url|'s
+ // keyword. (This will not prevent |t_url| from auto-updating the
+ // keyword in the future if the conflicting TemplateURL disappears.)
+ if (!CanReplace(i->second)) {
+ t_url->data_.SetKeyword(original_keyword);
+ continue;
+ }
+ RemoveNoNotify(i->second);
+ }
RemoveFromKeywordMapByPointer(t_url);
- if (!new_keyword.empty())
- keyword_to_template_map_[new_keyword] = t_url;
+ keyword_to_template_map_[new_keyword] = t_url;
}
}
@@ -1618,7 +1786,8 @@ void TemplateURLService::UpdateDefaultSearch() {
// TemplateURLsHaveSamePrefs would have returned true. Remove this now
// invalid value.
TemplateURL* old_default = default_search_provider_;
- SetDefaultSearchProviderNoNotify(NULL);
+ bool success = SetDefaultSearchProviderNoNotify(NULL);
+ DCHECK(success);
RemoveNoNotify(old_default);
} else if (default_search_provider_) {
TemplateURLData data(new_default_from_prefs->data());
@@ -1631,9 +1800,11 @@ void TemplateURLService::UpdateDefaultSearch() {
TemplateURLData data(new_default_from_prefs->data());
data.created_by_policy = true;
new_template = new TemplateURL(profile_, data);
- AddNoNotify(new_template, true);
+ if (!AddNoNotify(new_template, true))
+ return;
}
- SetDefaultSearchProviderNoNotify(new_template);
+ bool success = SetDefaultSearchProviderNoNotify(new_template);
+ DCHECK(success);
}
} else if (!is_default_search_managed_ && new_is_default_managed) {
// The default used to be unmanaged and is now managed. Add the new
@@ -1644,9 +1815,11 @@ void TemplateURLService::UpdateDefaultSearch() {
TemplateURLData data(new_default_from_prefs->data());
data.created_by_policy = true;
new_template = new TemplateURL(profile_, data);
- AddNoNotify(new_template, true);
+ if (!AddNoNotify(new_template, true))
+ return;
}
- SetDefaultSearchProviderNoNotify(new_template);
+ bool success = SetDefaultSearchProviderNoNotify(new_template);
+ DCHECK(success);
} else {
// The default was managed and is no longer.
DCHECK(is_default_search_managed_ && !new_is_default_managed);
@@ -1671,10 +1844,11 @@ void TemplateURLService::UpdateDefaultSearch() {
NotifyObservers();
}
-void TemplateURLService::SetDefaultSearchProviderNoNotify(TemplateURL* url) {
+bool TemplateURLService::SetDefaultSearchProviderNoNotify(TemplateURL* url) {
if (url) {
- DCHECK(std::find(template_urls_.begin(), template_urls_.end(), url) !=
- template_urls_.end());
+ if (std::find(template_urls_.begin(), template_urls_.end(), url) ==
+ template_urls_.end())
+ return false;
// Extension keywords cannot be made default, as they're inherently async.
DCHECK(!url->IsExtensionKeyword());
}
@@ -1686,7 +1860,7 @@ void TemplateURLService::SetDefaultSearchProviderNoNotify(TemplateURL* url) {
// template urls we ship with.
url->data_.show_in_default_list = true;
if (service_.get())
- service_->UpdateKeyword(*url);
+ service_->UpdateKeyword(url->data());
if (url->url_ref().HasGoogleBaseURLs()) {
GoogleURLTracker::RequestServerCheck();
@@ -1718,9 +1892,10 @@ void TemplateURLService::SetDefaultSearchProviderNoNotify(TemplateURL* url) {
// Inform sync the change to the show_in_default_list flag.
if (url)
ProcessTemplateURLChange(url, SyncChange::ACTION_UPDATE);
+ return true;
}
-void TemplateURLService::AddNoNotify(TemplateURL* template_url,
+bool TemplateURLService::AddNoNotify(TemplateURL* template_url,
bool newly_adding) {
DCHECK(template_url);
@@ -1731,6 +1906,27 @@ void TemplateURLService::AddNoNotify(TemplateURL* template_url,
template_url->data_.id = ++next_id_;
}
+ template_url->ResetKeywordIfNecessary(false);
+ if (!template_url->IsExtensionKeyword()) {
+ // Check whether |template_url|'s keyword conflicts with any already in the
+ // model.
+ TemplateURL* existing_keyword_turl =
+ FindNonExtensionTemplateURLForKeyword(template_url->keyword());
+ if (existing_keyword_turl != NULL) {
+ DCHECK_NE(existing_keyword_turl, template_url);
+ if (CanReplace(existing_keyword_turl)) {
+ RemoveNoNotify(existing_keyword_turl);
+ } else if (CanReplace(template_url)) {
+ delete template_url;
+ return false;
+ } else {
+ string16 new_keyword = UniquifyKeyword(*existing_keyword_turl);
+ ResetTemplateURL(existing_keyword_turl,
+ existing_keyword_turl->short_name(), new_keyword,
+ existing_keyword_turl->url());
+ }
+ }
+ }
template_urls_.push_back(template_url);
AddToMaps(template_url);
@@ -1740,12 +1936,14 @@ void TemplateURLService::AddNoNotify(TemplateURL* template_url,
// TODO(mpcomplete): If we allow editing extension keywords, then those
// should be persisted to disk and synced.
if (service_.get() && !template_url->IsExtensionKeyword())
- service_->AddKeyword(*template_url);
+ service_->AddKeyword(template_url->data());
// Inform sync of the addition. Note that this will assign a GUID to
// template_url and add it to the guid_to_template_map_.
ProcessTemplateURLChange(template_url, SyncChange::ACTION_ADD);
}
+
+ return true;
}
void TemplateURLService::RemoveNoNotify(TemplateURL* template_url) {
@@ -1847,6 +2045,7 @@ void TemplateURLService::RemoveProvidersCreatedByPolicy(
void TemplateURLService::ResetTemplateURLGUID(TemplateURL* url,
const std::string& guid) {
+ DCHECK(loaded_);
DCHECK(!guid.empty());
TemplateURLData data(url->data());
@@ -1863,9 +2062,8 @@ string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl) {
// First, try to return the generated keyword for the TemplateURL.
GURL gurl(turl.url());
if (gurl.is_valid()) {
- string16 keyword_candidate = GenerateKeyword(gurl, true);
- if (!GetTemplateURLForKeyword(keyword_candidate) &&
- !keyword_candidate.empty())
+ string16 keyword_candidate = GenerateKeyword(gurl);
+ if (!GetTemplateURLForKeyword(keyword_candidate))
return keyword_candidate;
}
@@ -1880,20 +2078,19 @@ string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl) {
return keyword_candidate;
}
-bool TemplateURLService::ResolveSyncKeywordConflict(
+void TemplateURLService::ResolveSyncKeywordConflict(
TemplateURL* sync_turl,
+ TemplateURL* local_turl,
SyncChangeList* change_list) {
+ DCHECK(loaded_);
DCHECK(sync_turl);
+ DCHECK(local_turl);
+ DCHECK(sync_turl->sync_guid() != local_turl->sync_guid());
+ DCHECK(!local_turl->IsExtensionKeyword());
DCHECK(change_list);
- TemplateURL* existing_turl =
- GetTemplateURLForKeyword(sync_turl->keyword());
- // If there is no conflict, or it's just conflicting with itself, return.
- if (!existing_turl || existing_turl->sync_guid() == sync_turl->sync_guid())
- return false;
-
- if (existing_turl->last_modified() > sync_turl->last_modified() ||
- existing_turl->created_by_policy()) {
+ if (local_turl->last_modified() > sync_turl->last_modified() ||
+ local_turl->created_by_policy()) {
string16 new_keyword = UniquifyKeyword(*sync_turl);
DCHECK(!GetTemplateURLForKeyword(new_keyword));
sync_turl->data_.SetKeyword(new_keyword);
@@ -1902,14 +2099,24 @@ bool TemplateURLService::ResolveSyncKeywordConflict(
SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl);
change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
} else {
- string16 new_keyword = UniquifyKeyword(*existing_turl);
- TemplateURLData data(existing_turl->data());
+ string16 new_keyword = UniquifyKeyword(*local_turl);
+ TemplateURLData data(local_turl->data());
data.SetKeyword(new_keyword);
- TemplateURL new_turl(existing_turl->profile(), data);
- UpdateNoNotify(existing_turl, new_turl);
- NotifyObservers();
+ TemplateURL new_turl(local_turl->profile(), data);
+ if (UpdateNoNotify(local_turl, new_turl))
+ NotifyObservers();
+ if (!models_associated_) {
+ // We're doing our initial sync, so UpdateNoNotify() won't have generated
+ // an ACTION_UPDATE. If this local URL is one that was just newly brought
+ // down from the sync server, we need to go ahead and generate an update
+ // for it. If it was pre-existing, then this is unnecessary (and in fact
+ // wrong) because MergeDataAndStartSyncing() will later add an ACTION_ADD
+ // for this URL; but in this case, PreventDuplicateGUIDUpdates() will
+ // prune out the ACTION_UPDATE we create here.
+ SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
+ change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data));
+ }
}
- return true;
}
TemplateURL* TemplateURLService::FindDuplicateOfSyncTemplateURL(
@@ -1923,6 +2130,7 @@ void TemplateURLService::MergeSyncAndLocalURLDuplicates(
TemplateURL* sync_turl,
TemplateURL* local_turl,
SyncChangeList* change_list) {
+ DCHECK(loaded_);
DCHECK(sync_turl);
DCHECK(local_turl);
DCHECK(change_list);
@@ -1997,7 +2205,7 @@ void TemplateURLService::PatchMissingSyncGUIDs(
!template_url->IsExtensionKeyword()) {
template_url->data_.sync_guid = guid::GenerateGUID();
if (service_.get())
- service_->UpdateKeyword(*template_url);
+ service_->UpdateKeyword(template_url->data());
}
}
}
diff --git a/chrome/browser/search_engines/template_url_service.h b/chrome/browser/search_engines/template_url_service.h
index 60618e204..cb571f6 100644
--- a/chrome/browser/search_engines/template_url_service.h
+++ b/chrome/browser/search_engines/template_url_service.h
@@ -83,12 +83,10 @@ class TemplateURLService : public WebDataServiceConsumer,
TemplateURLService(const Initializer* initializers, const int count);
virtual ~TemplateURLService();
- // Generates a suitable keyword for the specified url. Returns an empty
- // string if a keyword couldn't be generated. If |autodetected| is true, we
- // don't generate keywords for a variety of situations where we would probably
- // not want to auto-add keywords, such as keywords for searches on pages that
- // themselves come from form submissions.
- static string16 GenerateKeyword(const GURL& url, bool autodetected);
+ // Generates a suitable keyword for the specified url, which must be valid.
+ // This is guaranteed not to return an empty string, since TemplateURLs should
+ // never have an empty keyword.
+ static string16 GenerateKeyword(const GURL& url);
// Removes any unnecessary characters from a user input keyword.
// This removes the leading scheme, "www." and any trailing slash.
@@ -101,8 +99,9 @@ class TemplateURLService : public WebDataServiceConsumer,
static GURL GenerateSearchURL(TemplateURL* t_url);
// Just like GenerateSearchURL except that it takes SearchTermsData to supply
- // the data for some search terms. Most of the time GenerateSearchURL should
- // be called.
+ // the data for some search terms, e.g. so this can be used on threads other
+ // than the UI thread. See the various TemplateURLRef::XXXUsingTermsData()
+ // functions.
static GURL GenerateSearchURLUsingTermsData(
const TemplateURL* t_url,
const SearchTermsData& search_terms_data);
@@ -140,7 +139,8 @@ class TemplateURLService : public WebDataServiceConsumer,
// or NULL if there are no such TemplateURLs
TemplateURL* GetTemplateURLForHost(const std::string& host);
- // Takes ownership of |template_url| and adds it to this model.
+ // Takes ownership of |template_url| and adds it to this model. For obvious
+ // reasons, it is illegal to Add() the same |template_url| pointer twice.
void Add(TemplateURL* template_url);
// Like Add(), but overwrites the |template_url|'s values with the provided
@@ -413,12 +413,17 @@ class TemplateURLService : public WebDataServiceConsumer,
// in the default list and is marked as safe_for_autoreplace.
bool CanReplace(const TemplateURL* t_url);
+ // Like GetTemplateURLForKeyword(), but ignores extension-provided keywords.
+ TemplateURL* FindNonExtensionTemplateURLForKeyword(const string16& keyword);
+
// Updates the information in |existing_turl| using the information from
- // |new_values|, but the ID for |existing_turl| is retained.
- // Notifying observers is the responsibility of the caller.
+ // |new_values|, but the ID for |existing_turl| is retained. Notifying
+ // observers is the responsibility of the caller. Returns whether
+ // |existing_turl| was found in |template_urls_| and thus could be updated.
+ //
// NOTE: This should not be called with an extension keyword as there are no
// updates needed in that case.
- void UpdateNoNotify(TemplateURL* existing_turl,
+ bool UpdateNoNotify(TemplateURL* existing_turl,
const TemplateURL& new_values);
// Returns the preferences we use.
@@ -451,16 +456,21 @@ class TemplateURLService : public WebDataServiceConsumer,
void UpdateDefaultSearch();
// Set the default search provider even if it is managed. |url| may be null.
- // Caller is responsible for notifying observers.
- void SetDefaultSearchProviderNoNotify(TemplateURL* url);
+ // Caller is responsible for notifying observers. Returns whether |url| was
+ // found in |template_urls_| and thus could be made default.
+ bool SetDefaultSearchProviderNoNotify(TemplateURL* url);
// Adds a new TemplateURL to this model. TemplateURLService will own the
// reference, and delete it when the TemplateURL is removed.
// If |newly_adding| is false, we assume that this TemplateURL was already
// part of the model in the past, and therefore we don't need to do things
// like assign it an ID or notify sync.
- // Caller is responsible for notifying observers.
- void AddNoNotify(TemplateURL* template_url, bool newly_adding);
+ // This function guarantees that on return the model will not have two
+ // non-extension TemplateURLs with the same keyword. If that means that it
+ // cannot add the provided argument, it will delete it and return false.
+ // Caller is responsible for notifying observers if this function returns
+ // true.
+ bool AddNoNotify(TemplateURL* template_url, bool newly_adding);
// Removes the keyword from the model. This deletes the supplied TemplateURL.
// This fails if the supplied template_url is the default search provider.
@@ -491,15 +501,14 @@ class TemplateURLService : public WebDataServiceConsumer,
// it is unique to the Service.
string16 UniquifyKeyword(const TemplateURL& turl);
- // Given a TemplateURL from Sync (cloud), resolves any keyword conflicts by
- // checking the local keywords and uniquifying either the cloud keyword or a
- // conflicting local keyword (whichever is older). If the cloud TURL is
- // changed, then an appropriate SyncChange is appended to |change_list|. If
- // a local TURL is changed, the service is updated with the new keyword. If
- // there was no conflict to begin with, this does nothing. In the case of tied
- // last_modified dates, |sync_turl| wins. Returns true iff there was a
- // conflict.
- bool ResolveSyncKeywordConflict(TemplateURL* sync_turl,
+ // Given a TemplateURL from Sync (cloud) and a local TemplateURL with the same
+ // keyword, resolves the conflict by uniquifying either the cloud keyword or
+ // the local keyword (whichever is older). If the cloud TURL is changed, then
+ // an appropriate SyncChange is appended to |change_list|. If a local TURL is
+ // changed, the service is updated with the new keyword. In the case of tied
+ // last_modified dates, |sync_turl| wins.
+ void ResolveSyncKeywordConflict(TemplateURL* sync_turl,
+ TemplateURL* local_turl,
SyncChangeList* change_list);
// Returns a TemplateURL from the service that has the same keyword and search
diff --git a/chrome/browser/search_engines/template_url_service_sync_unittest.cc b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
index 451538e..5378817 100644
--- a/chrome/browser/search_engines/template_url_service_sync_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
@@ -6,6 +6,7 @@
#include "base/string_util.h"
#include "base/time.h"
#include "base/utf_string_conversions.h"
+#include "chrome/browser/search_engines/search_terms_data.h"
#include "chrome/browser/search_engines/template_url.h"
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -14,9 +15,11 @@
#include "chrome/browser/sync/api/sync_error_factory_mock.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
#include "chrome/test/base/testing_pref_service.h"
#include "chrome/test/base/testing_profile.h"
#include "content/public/browser/notification_service.h"
+#include "net/base/net_util.h"
#include "sync/protocol/search_engine_specifics.pb.h"
#include "sync/protocol/sync.pb.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -41,17 +44,19 @@ std::string GetKeyword(const SyncData& sync_data) {
}
// Much like TemplateURLService::CreateSyncDataFromTemplateURL(), but allows the
-// caller to override the URL or GUID fields with empty strings, in order to
-// create illegal data that should be DELETEd when we try to sync it to a
+// caller to override the keyword, URL, or GUID fields with empty strings, in
+// order to create custom data that should be handled specially when synced to a
// client.
-SyncData CreateBogusSyncData(const TemplateURL& turl,
- const std::string& url,
- const std::string& sync_guid) {
+SyncData CreateCustomSyncData(const TemplateURL& turl,
+ bool autogenerate_keyword,
+ const std::string& url,
+ const std::string& sync_guid) {
sync_pb::EntitySpecifics specifics;
sync_pb::SearchEngineSpecifics* se_specifics =
specifics.mutable_search_engine();
se_specifics->set_short_name(UTF16ToUTF8(turl.short_name()));
- se_specifics->set_keyword(UTF16ToUTF8(turl.keyword()));
+ se_specifics->set_keyword(
+ autogenerate_keyword ? std::string() : UTF16ToUTF8(turl.keyword()));
se_specifics->set_favicon_url(turl.favicon_url().spec());
se_specifics->set_url(url);
se_specifics->set_safe_for_autoreplace(turl.safe_for_autoreplace());
@@ -61,7 +66,7 @@ SyncData CreateBogusSyncData(const TemplateURL& turl,
se_specifics->set_show_in_default_list(turl.show_in_default_list());
se_specifics->set_suggestions_url(turl.suggestions_url());
se_specifics->set_prepopulate_id(turl.prepopulate_id());
- se_specifics->set_autogenerate_keyword(turl.autogenerate_keyword());
+ se_specifics->set_autogenerate_keyword(autogenerate_keyword);
se_specifics->set_instant_url(turl.instant_url());
se_specifics->set_last_modified(turl.last_modified().ToInternalValue());
se_specifics->set_sync_guid(sync_guid);
@@ -189,7 +194,8 @@ class TemplateURLServiceSyncTest : public testing::Test {
const std::string& url,
const std::string& guid = std::string(),
time_t last_mod = 100,
- bool created_by_policy = false) const;
+ bool created_by_policy = false,
+ bool safe_for_autoreplace = true) const;
// Verifies the two TemplateURLs are equal.
// TODO(stevet): Share this with TemplateURLServiceTest.
@@ -264,13 +270,14 @@ TemplateURL* TemplateURLServiceSyncTest::CreateTestTemplateURL(
const std::string& url,
const std::string& guid,
time_t last_mod,
- bool created_by_policy) const {
+ bool created_by_policy,
+ bool safe_for_autoreplace) const {
TemplateURLData data;
data.short_name = ASCIIToUTF16("unittest");
data.SetKeyword(keyword);
data.SetURL(url);
data.favicon_url = GURL("http://favicon.url");
- data.safe_for_autoreplace = true;
+ data.safe_for_autoreplace = safe_for_autoreplace;
data.date_created = Time::FromTimeT(100);
data.last_modified = Time::FromTimeT(last_mod);
data.created_by_policy = created_by_policy;
@@ -381,7 +388,7 @@ TEST_F(TemplateURLServiceSyncTest, GetAllSyncDataNoExtensions) {
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com"));
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key2"), "http://key2.com"));
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key3"),
- "chrome-extension://blahblahblah"));
+ std::string(chrome::kExtensionScheme) + "://blahblahblah"));
SyncDataList all_sync_data =
model()->GetAllSyncData(syncable::SEARCH_ENGINES);
@@ -446,65 +453,58 @@ TEST_F(TemplateURLServiceSyncTest, UniquifyKeyword) {
}
TEST_F(TemplateURLServiceSyncTest, ResolveSyncKeywordConflict) {
+ // Create a keyword that conflicts, and make it older.
+ // Conflict, sync keyword is uniquified, and a SyncChange is added.
string16 original_turl_keyword = ASCIIToUTF16("key1");
TemplateURL* original_turl = CreateTestTemplateURL(original_turl_keyword,
"http://key1.com", std::string(), 9000);
model()->Add(original_turl);
-
- // Create a key that does not conflict with something in the model.
- scoped_ptr<TemplateURL> sync_turl(
- CreateTestTemplateURL(ASCIIToUTF16("unique"), "http://new.com"));
- string16 sync_keyword = sync_turl->keyword();
+ scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword,
+ "http://new.com", "remote", 8999));
SyncChangeList changes;
-
- // No conflict, no TURLs changed, no changes.
- EXPECT_FALSE(model()->ResolveSyncKeywordConflict(sync_turl.get(), &changes));
- EXPECT_EQ(original_turl_keyword, original_turl->keyword());
- EXPECT_EQ(sync_keyword, sync_turl->keyword());
- EXPECT_EQ(0U, changes.size());
-
- // Change sync keyword to something that conflicts, and make it older.
- // Conflict, sync keyword is uniquified, and a SyncChange is added.
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 8999));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), &changes));
+ model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
EXPECT_NE(original_turl_keyword, sync_turl->keyword());
EXPECT_EQ(original_turl_keyword, original_turl->keyword());
- EXPECT_EQ(1U, changes.size());
+ ASSERT_EQ(1U, changes.size());
+ EXPECT_EQ("remote", GetGUID(changes[0].sync_data()));
changes.clear();
- // Sync is newer. Original TemplateURL keyword is uniquified, no SyncChange
- // is added. Also ensure that this does not change the safe_for_autoreplace
- // flag or the TemplateURLID in the original.
+ // Sync is newer. Original TemplateURL keyword is uniquified. A SyncChange is
+ // added (which in a normal run would be deleted by
+ // PreventDuplicateGUIDUpdates() after we subsequently add an ACTION_ADD
+ // change for the local URL). Also ensure that this does not change the
+ // safe_for_autoreplace flag or the TemplateURLID in the original.
model()->Remove(original_turl);
original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000);
+ "http://key1.com", "local", 9000);
model()->Add(original_turl);
TemplateURLID original_id = original_turl->id();
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
std::string(), 9001));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), &changes));
+ model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
EXPECT_NE(original_turl_keyword, original_turl->keyword());
EXPECT_TRUE(original_turl->safe_for_autoreplace());
EXPECT_EQ(original_id, original_turl->id());
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword));
- EXPECT_EQ(0U, changes.size());
+ ASSERT_EQ(1U, changes.size());
+ EXPECT_EQ("local", GetGUID(changes[0].sync_data()));
changes.clear();
// Equal times. Same result as above. Sync left alone, original uniquified so
// sync_turl can fit.
model()->Remove(original_turl);
original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000);
+ "http://key1.com", "local2", 9000);
model()->Add(original_turl);
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
std::string(), 9000));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), &changes));
+ model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
EXPECT_NE(original_turl_keyword, original_turl->keyword());
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword));
- EXPECT_EQ(0U, changes.size());
+ ASSERT_EQ(1U, changes.size());
+ EXPECT_EQ("local2", GetGUID(changes[0].sync_data()));
changes.clear();
// Sync is newer, but original TemplateURL is created by policy, so it wins.
@@ -514,12 +514,13 @@ TEST_F(TemplateURLServiceSyncTest, ResolveSyncKeywordConflict) {
"http://key1.com", std::string(), 9000, true);
model()->Add(original_turl);
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 9999));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), &changes));
+ "remote2", 9999));
+ model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
EXPECT_NE(original_turl_keyword, sync_turl->keyword());
EXPECT_EQ(original_turl_keyword, original_turl->keyword());
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(sync_turl->keyword()));
- EXPECT_EQ(1U, changes.size());
+ ASSERT_EQ(1U, changes.size());
+ EXPECT_EQ("remote2", GetGUID(changes[0].sync_data()));
changes.clear();
}
@@ -581,10 +582,8 @@ TEST_F(TemplateURLServiceSyncTest, MergeSyncAndLocalURLDuplicates) {
}
TEST_F(TemplateURLServiceSyncTest, StartSyncEmpty) {
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, SyncDataList(),
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, SyncDataList(),
+ PassProcessor(), CreateAndPassSyncErrorFactory());
EXPECT_EQ(0U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
EXPECT_EQ(0U, processor()->change_list_size());
@@ -593,10 +592,8 @@ TEST_F(TemplateURLServiceSyncTest, StartSyncEmpty) {
TEST_F(TemplateURLServiceSyncTest, MergeIntoEmpty) {
SyncDataList initial_data = CreateInitialSyncData();
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
// We expect the model to have accepted all of the initial sync data. Search
@@ -611,18 +608,16 @@ TEST_F(TemplateURLServiceSyncTest, MergeIntoEmpty) {
}
TEST_F(TemplateURLServiceSyncTest, MergeInAllNewData) {
- model()->Add(CreateTestTemplateURL(ASCIIToUTF16("google.com"),
- "http://google.com", "abc"));
- model()->Add(CreateTestTemplateURL(ASCIIToUTF16("yahoo.com"),
- "http://yahoo.com", "def"));
- model()->Add(CreateTestTemplateURL(ASCIIToUTF16("bing.com"),
- "http://bing.com", "xyz"));
+ model()->Add(CreateTestTemplateURL(ASCIIToUTF16("abc.com"), "http://abc.com",
+ "abc"));
+ model()->Add(CreateTestTemplateURL(ASCIIToUTF16("def.com"), "http://def.com",
+ "def"));
+ model()->Add(CreateTestTemplateURL(ASCIIToUTF16("xyz.com"), "http://xyz.com",
+ "xyz"));
SyncDataList initial_data = CreateInitialSyncData();
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
EXPECT_EQ(6U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
// We expect the model to have accepted all of the initial sync data. Search
@@ -633,9 +628,9 @@ TEST_F(TemplateURLServiceSyncTest, MergeInAllNewData) {
EXPECT_TRUE(model()->GetTemplateURLForGUID(guid));
}
// All the original TemplateURLs should also remain in the model.
- EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("google.com")));
- EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("yahoo.com")));
- EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("bing.com")));
+ EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("abc.com")));
+ EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("def.com")));
+ EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("xyz.com")));
// Ensure that Sync received the expected changes.
EXPECT_EQ(3U, processor()->change_list_size());
EXPECT_TRUE(processor()->contains_guid("abc"));
@@ -654,10 +649,8 @@ TEST_F(TemplateURLServiceSyncTest, MergeSyncIsTheSame) {
model()->Add(converted);
}
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
for (SyncDataList::const_iterator iter = initial_data.begin();
@@ -672,41 +665,39 @@ TEST_F(TemplateURLServiceSyncTest, MergeUpdateFromSync) {
// The local data is the same as the sync data merged in, but timestamps have
// changed. Ensure the right fields are merged in.
SyncDataList initial_data;
- TemplateURL* turl1 = CreateTestTemplateURL(ASCIIToUTF16("google.com"),
- "http://google.com", "abc", 9000);
+ TemplateURL* turl1 = CreateTestTemplateURL(ASCIIToUTF16("abc.com"),
+ "http://abc.com", "abc", 9000);
model()->Add(turl1);
- TemplateURL* turl2 = CreateTestTemplateURL(ASCIIToUTF16("bing.com"),
- "http://bing.com", "xyz", 9000);
+ TemplateURL* turl2 = CreateTestTemplateURL(ASCIIToUTF16("xyz.com"),
+ "http://xyz.com", "xyz", 9000);
model()->Add(turl2);
scoped_ptr<TemplateURL> turl1_newer(CreateTestTemplateURL(
- ASCIIToUTF16("google.com"), "http://google.ca", "abc", 9999));
+ ASCIIToUTF16("abc.com"), "http://abc.ca", "abc", 9999));
initial_data.push_back(
TemplateURLService::CreateSyncDataFromTemplateURL(*turl1_newer));
scoped_ptr<TemplateURL> turl2_older(CreateTestTemplateURL(
- ASCIIToUTF16("bing.com"), "http://bing.ca", "xyz", 8888));
+ ASCIIToUTF16("xyz.com"), "http://xyz.ca", "xyz", 8888));
initial_data.push_back(
TemplateURLService::CreateSyncDataFromTemplateURL(*turl2_older));
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
// Both were local updates, so we expect the same count.
EXPECT_EQ(2U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
- // Check that the first replaced the initial Google TemplateURL.
+ // Check that the first replaced the initial abc TemplateURL.
EXPECT_EQ(turl1, model()->GetTemplateURLForGUID("abc"));
- EXPECT_EQ("http://google.ca", turl1->url());
+ EXPECT_EQ("http://abc.ca", turl1->url());
- // Check that the second produced an upstream update to the Bing TemplateURL.
+ // Check that the second produced an upstream update to the xyz TemplateURL.
EXPECT_EQ(1U, processor()->change_list_size());
ASSERT_TRUE(processor()->contains_guid("xyz"));
SyncChange change = processor()->change_for_guid("xyz");
EXPECT_TRUE(change.change_type() == SyncChange::ACTION_UPDATE);
- EXPECT_EQ("http://bing.com", GetURL(change.sync_data()));
+ EXPECT_EQ("http://xyz.com", GetURL(change.sync_data()));
}
TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
@@ -722,10 +713,9 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("unique"),
"http://unique.com", "ccc")); // add
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, CreateInitialSyncData(),
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// The dupe results in a merge. The other two should be added to the model.
EXPECT_EQ(5U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -782,11 +772,9 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("unique"),
"http://unique.com", "ccc", 10)); // add
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(),
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// The dupe results in a merge. The other two should be added to the model.
EXPECT_EQ(5U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -824,10 +812,8 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
TEST_F(TemplateURLServiceSyncTest, ProcessChangesEmptyModel) {
// We initially have no data.
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, SyncDataList(),
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, SyncDataList(),
+ PassProcessor(), CreateAndPassSyncErrorFactory());
// Set up a bunch of ADDs.
SyncChangeList changes;
@@ -848,10 +834,9 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesEmptyModel) {
}
TEST_F(TemplateURLServiceSyncTest, ProcessChangesNoConflicts) {
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Process different types of changes, without conflicts.
SyncChangeList changes;
@@ -879,10 +864,9 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesNoConflicts) {
}
TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsSyncWins) {
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Process different types of changes, with conflicts. Note that all this data
// has a newer timestamp, so Sync will win in these scenarios.
@@ -917,10 +901,9 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsSyncWins) {
}
TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) {
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Process different types of changes, with conflicts. Note that all this data
// has an older timestamp, so the local data will win in these scenarios.
@@ -966,10 +949,9 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) {
TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {
// Ensure that ProcessTemplateURLChange is called and pushes the correct
// changes to Sync whenever local changes are made to TemplateURLs.
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Add a new search engine.
TemplateURL* new_turl =
@@ -1001,12 +983,179 @@ TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {
EXPECT_EQ(SyncChange::ACTION_DELETE, change.change_type());
}
+TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithLocalExtensions) {
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
+
+ // Add some extension keywords locally. These shouldn't be synced.
+ TemplateURL* extension1 = CreateTestTemplateURL(ASCIIToUTF16("keyword1"),
+ std::string(chrome::kExtensionScheme) + "://extension1");
+ model()->Add(extension1);
+ TemplateURL* extension2 = CreateTestTemplateURL(ASCIIToUTF16("keyword2"),
+ std::string(chrome::kExtensionScheme) + "://extension2");
+ model()->Add(extension2);
+ EXPECT_EQ(0U, processor()->change_list_size());
+
+ // Create some sync changes that will conflict with the extension keywords.
+ SyncChangeList changes;
+ changes.push_back(CreateTestSyncChange(SyncChange::ACTION_ADD,
+ CreateTestTemplateURL(ASCIIToUTF16("keyword1"), "http://aaa.com")));
+ changes.push_back(CreateTestSyncChange(SyncChange::ACTION_ADD,
+ CreateTestTemplateURL(ASCIIToUTF16("keyword2"), "http://bbb.com",
+ std::string(), 100, false, false)));
+ model()->ProcessSyncChanges(FROM_HERE, changes);
+
+ // The synced keywords should be added unchanged, and the result of
+ // GetTemplateURLForKeyword() for each keyword should depend on whether the
+ // synced keyword is replaceable or not.
+ EXPECT_EQ(extension1,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1")));
+ EXPECT_FALSE(model()->GetTemplateURLForHost("aaa.com") == NULL);
+ TemplateURL* url_for_keyword2 =
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2"));
+ EXPECT_NE(extension2, url_for_keyword2);
+ EXPECT_EQ("http://bbb.com", url_for_keyword2->url());
+ // Note that extensions don't use host-based keywords, so we can't check that
+ // |extension2| is still in the model using GetTemplateURLForHost(); and we
+ // have to create a full-fledged Extension to use
+ // GetTemplateURLForExtension(), which is annoying, so just grab all the URLs
+ // from the model and search for |extension2| within them.
+ TemplateURLService::TemplateURLVector template_urls(
+ model()->GetTemplateURLs());
+ EXPECT_FALSE(std::find(template_urls.begin(), template_urls.end(),
+ extension2) == template_urls.end());
+}
+
+TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordMigrated) {
+ // Create a couple of sync entries with autogenerated keywords.
+ SyncDataList initial_data;
+ scoped_ptr<TemplateURL> turl(
+ CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com", "key1"));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key2"),
+ "{google:baseURL}search?q={searchTerms}", "key2"));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+
+ // Now try to sync the data locally.
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
+
+ // Both entries should have been added, with explicit keywords.
+ TemplateURL* key1 = model()->GetTemplateURLForHost("key1.com");
+ ASSERT_FALSE(key1 == NULL);
+ EXPECT_EQ(ASCIIToUTF16("key1.com"), key1->keyword());
+ std::string google_hostname(
+ GURL(UIThreadSearchTermsData().GoogleBaseURLValue()).host());
+ TemplateURL* key2 = model()->GetTemplateURLForHost(google_hostname);
+ ASSERT_FALSE(key2 == NULL);
+ string16 google_keyword(net::StripWWW(ASCIIToUTF16(google_hostname)));
+ EXPECT_EQ(google_keyword, key2->keyword());
+
+ // We should also have gotten some corresponding UPDATEs pushed upstream.
+ EXPECT_GE(processor()->change_list_size(), 2U);
+ ASSERT_TRUE(processor()->contains_guid("key1"));
+ SyncChange key1_change = processor()->change_for_guid("key1");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, key1_change.change_type());
+ EXPECT_EQ("key1.com", GetKeyword(key1_change.sync_data()));
+ ASSERT_TRUE(processor()->contains_guid("key2"));
+ SyncChange key2_change = processor()->change_for_guid("key2");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, key2_change.change_type());
+ EXPECT_EQ(google_keyword, UTF8ToUTF16(GetKeyword(key2_change.sync_data())));
+}
+
+TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) {
+ // Sync brings in some autogenerated keywords, but the generated keywords we
+ // try to create conflict with ones in the model.
+ string16 google_keyword(net::StripWWW(ASCIIToUTF16(GURL(
+ UIThreadSearchTermsData().GoogleBaseURLValue()).host())));
+ TemplateURL* google = CreateTestTemplateURL(google_keyword,
+ "{google:baseURL}1/search?q={searchTerms}");
+ model()->Add(google);
+ TemplateURL* other =
+ CreateTestTemplateURL(ASCIIToUTF16("other.com"), "http://other.com/foo");
+ model()->Add(other);
+ SyncDataList initial_data;
+ scoped_ptr<TemplateURL> turl(CreateTestTemplateURL(ASCIIToUTF16("sync1"),
+ "{google:baseURL}2/search?q={searchTerms}", "sync1", 50));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ turl.reset(CreateTestTemplateURL(ASCIIToUTF16("sync2"),
+ "http://other.com/search?q={searchTerms}", "sync2", 150));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
+
+ // In this case, the conflicts should be handled just like any other keyword
+ // conflicts -- the later-modified TemplateURL is assumed to be authoritative.
+ EXPECT_EQ(google_keyword, google->keyword());
+ EXPECT_EQ(google_keyword + ASCIIToUTF16("_"),
+ model()->GetTemplateURLForGUID("sync1")->keyword());
+ EXPECT_EQ(ASCIIToUTF16("other.com_"), other->keyword());
+ EXPECT_EQ(ASCIIToUTF16("other.com"),
+ model()->GetTemplateURLForGUID("sync2")->keyword());
+
+ // Both synced URLs should have associated UPDATEs, since both needed their
+ // keywords to be generated (and sync1 needed conflict resolution as well).
+ EXPECT_GE(processor()->change_list_size(), 2U);
+ ASSERT_TRUE(processor()->contains_guid("sync1"));
+ SyncChange sync1_change = processor()->change_for_guid("sync1");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, sync1_change.change_type());
+ EXPECT_EQ(google_keyword + ASCIIToUTF16("_"),
+ UTF8ToUTF16(GetKeyword(sync1_change.sync_data())));
+ ASSERT_TRUE(processor()->contains_guid("sync2"));
+ SyncChange sync2_change = processor()->change_for_guid("sync2");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, sync2_change.change_type());
+ EXPECT_EQ("other.com", GetKeyword(sync2_change.sync_data()));
+}
+
+TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) {
+ // Sync brings in two autogenerated keywords and both use Google base URLs.
+ // We make the first older so that it will get renamed once before the second
+ // and then again once after (when we resolve conflicts for the second).
+ SyncDataList initial_data;
+ scoped_ptr<TemplateURL> turl(CreateTestTemplateURL(ASCIIToUTF16("key1"),
+ "{google:baseURL}1/search?q={searchTerms}", "key1", 50));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key2"),
+ "{google:baseURL}2/search?q={searchTerms}", "key2"));
+ initial_data.push_back(
+ CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
+
+ // We should still have coalesced the updates to one each.
+ string16 google_keyword(net::StripWWW(ASCIIToUTF16(GURL(
+ UIThreadSearchTermsData().GoogleBaseURLValue()).host())));
+ TemplateURL* keyword1 =
+ model()->GetTemplateURLForKeyword(google_keyword + ASCIIToUTF16("_"));
+ ASSERT_FALSE(keyword1 == NULL);
+ EXPECT_EQ("key1", keyword1->sync_guid());
+ TemplateURL* keyword2 = model()->GetTemplateURLForKeyword(google_keyword);
+ ASSERT_FALSE(keyword2 == NULL);
+ EXPECT_EQ("key2", keyword2->sync_guid());
+ EXPECT_GE(processor()->change_list_size(), 2U);
+ ASSERT_TRUE(processor()->contains_guid("key1"));
+ SyncChange key1_change = processor()->change_for_guid("key1");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, key1_change.change_type());
+ EXPECT_EQ(keyword1->keyword(),
+ UTF8ToUTF16(GetKeyword(key1_change.sync_data())));
+ ASSERT_TRUE(processor()->contains_guid("key2"));
+ SyncChange key2_change = processor()->change_for_guid("key2");
+ EXPECT_EQ(SyncChange::ACTION_UPDATE, key2_change.change_type());
+ EXPECT_EQ(keyword2->keyword(),
+ UTF8ToUTF16(GetKeyword(key2_change.sync_data())));
+}
+
TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsBasic) {
// Start off B with some empty data.
- model_b()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model_b()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Merge A and B. All of B's data should transfer over to A, which initially
// has no data.
@@ -1024,10 +1173,9 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsBasic) {
TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {
// Start off B with some empty data.
- model_b()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- CreateInitialSyncData(), PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model_b()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ CreateInitialSyncData(), PassProcessor(),
+ CreateAndPassSyncErrorFactory());
// Set up A so we have some interesting duplicates and conflicts.
model_a()->Add(CreateTestTemplateURL(ASCIIToUTF16("key4"), "http://key4.com",
@@ -1042,8 +1190,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {
// Merge A and B.
scoped_ptr<SyncChangeProcessorDelegate> delegate_b(
new SyncChangeProcessorDelegate(model_b()));
- model_a()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
+ model_a()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
model_b()->GetAllSyncData(syncable::SEARCH_ENGINES),
delegate_b.PassAs<SyncChangeProcessor>(),
CreateAndPassSyncErrorFactory());
@@ -1054,8 +1201,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {
}
TEST_F(TemplateURLServiceSyncTest, StopSyncing) {
- SyncError error = model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
+ SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory());
ASSERT_FALSE(error.IsSet());
@@ -1075,8 +1221,7 @@ TEST_F(TemplateURLServiceSyncTest, StopSyncing) {
TEST_F(TemplateURLServiceSyncTest, SyncErrorOnInitialSync) {
processor()->set_erroneous(true);
- SyncError error = model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
+ SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory());
EXPECT_TRUE(error.IsSet());
@@ -1100,8 +1245,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncErrorOnInitialSync) {
TEST_F(TemplateURLServiceSyncTest, SyncErrorOnLaterSync) {
// Ensure that if the SyncProcessor succeeds in the initial merge, but fails
// in future ProcessSyncChanges, we still return an error.
- SyncError error = model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
+ SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory());
ASSERT_FALSE(error.IsSet());
@@ -1124,10 +1268,8 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) {
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com",
"key1", 10)); // earlier
- SyncError error = model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- initial_data, PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ initial_data, PassProcessor(), CreateAndPassSyncErrorFactory());
ASSERT_FALSE(error.IsSet());
// We should have updated the original TemplateURL with Sync's version.
@@ -1153,10 +1295,8 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) {
model()->StopSyncing(syncable::SEARCH_ENGINES);
sync_processor_delegate_.reset(new SyncChangeProcessorDelegate(
sync_processor_.get()));
- error = model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- initial_data, PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,
+ initial_data, PassProcessor(), CreateAndPassSyncErrorFactory());
ASSERT_FALSE(error.IsSet());
// Check that the TemplateURL was not modified.
@@ -1171,10 +1311,8 @@ TEST_F(TemplateURLServiceSyncTest, SyncedDefaultGUIDArrivesFirst) {
scoped_ptr<TemplateURL> turl(CreateTestTemplateURL(ASCIIToUTF16("key2"),
"http://key2.com/{searchTerms}", "key2", 90));
initial_data[1] = TemplateURLService::CreateSyncDataFromTemplateURL(*turl);
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
model()->SetDefaultSearchProvider(model()->GetTemplateURLForGUID("key2"));
EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -1240,10 +1378,8 @@ TEST_F(TemplateURLServiceSyncTest, SyncedDefaultArrivesAfterStartup) {
"http://key2.com/{searchTerms}", "key2", 90));
initial_data[1] = TemplateURLService::CreateSyncDataFromTemplateURL(*turl);
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
// Ensure that the new default has been set.
EXPECT_EQ(4U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -1271,10 +1407,8 @@ TEST_F(TemplateURLServiceSyncTest, SyncedDefaultAlreadySetOnStartup) {
// Now sync the initial data.
SyncDataList initial_data = CreateInitialSyncData();
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
// Ensure that the new entries were added and the default has not changed.
EXPECT_EQ(4U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -1285,10 +1419,8 @@ TEST_F(TemplateURLServiceSyncTest, SyncWithManagedDefaultSearch) {
// First start off with a few entries and make sure we can set an unmanaged
// default search provider.
SyncDataList initial_data = CreateInitialSyncData();
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
model()->SetDefaultSearchProvider(model()->GetTemplateURLForGUID("key2"));
EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
@@ -1349,10 +1481,8 @@ TEST_F(TemplateURLServiceSyncTest, SyncMergeDeletesDefault) {
"http://key1.com/{searchTerms}", "key1", 90));
initial_data[0] = TemplateURLService::CreateSyncDataFromTemplateURL(*turl);
- model()->MergeDataAndStartSyncing(
- syncable::SEARCH_ENGINES,
- initial_data, PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
+ PassProcessor(), CreateAndPassSyncErrorFactory());
EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());
EXPECT_FALSE(model()->GetTemplateURLForGUID("whateverguid"));
@@ -1366,15 +1496,14 @@ TEST_F(TemplateURLServiceSyncTest, DeleteBogusData) {
scoped_ptr<TemplateURL> turl(
CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com", "key1"));
initial_data.push_back(
- CreateBogusSyncData(*turl, std::string(), turl->sync_guid()));
+ CreateCustomSyncData(*turl, false, std::string(), turl->sync_guid()));
turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key2"), "http://key2.com"));
initial_data.push_back(
- CreateBogusSyncData(*turl, turl->url(), std::string()));
+ CreateCustomSyncData(*turl, false, turl->url(), std::string()));
// Now try to sync the data locally.
model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,
- PassProcessor(),
- CreateAndPassSyncErrorFactory());
+ PassProcessor(), CreateAndPassSyncErrorFactory());
// Nothing should have been added, and both bogus entries should be marked for
// deletion.
diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc
index d214f15..9bf29c6 100644
--- a/chrome/browser/search_engines/template_url_service_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_unittest.cc
@@ -21,6 +21,7 @@
#include "chrome/browser/search_engines/template_url_service.h"
#include "chrome/browser/search_engines/template_url_service_test_util.h"
#include "chrome/browser/webdata/web_database.h"
+#include "chrome/common/url_constants.h"
#include "chrome/test/base/testing_profile.h"
#include "content/test/test_browser_thread.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -137,7 +138,6 @@ class TemplateURLServiceTest : public testing::Test {
TemplateURL* AddKeywordWithDate(const std::string& short_name,
const std::string& keyword,
- bool autogenerate_keyword,
const std::string& url,
const std::string& suggest_url,
const std::string& favicon_url,
@@ -198,7 +198,6 @@ void TemplateURLServiceTest::TearDown() {
TemplateURL* TemplateURLServiceTest::AddKeywordWithDate(
const std::string& short_name,
const std::string& keyword,
- bool autogenerate_keyword,
const std::string& url,
const std::string& suggest_url,
const std::string& favicon_url,
@@ -209,7 +208,6 @@ TemplateURL* TemplateURLServiceTest::AddKeywordWithDate(
TemplateURLData data;
data.short_name = UTF8ToUTF16(short_name);
data.SetKeyword(UTF8ToUTF16(keyword));
- data.SetAutogenerateKeyword(autogenerate_keyword);
data.SetURL(url);
data.suggestions_url = suggest_url;
data.favicon_url = GURL(favicon_url);
@@ -413,31 +411,167 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) {
EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("b")) == NULL);
}
+TEST_F(TemplateURLServiceTest, AddSameKeyword) {
+ test_util_.VerifyLoad();
+
+ AddKeywordWithDate("first", "keyword", "http://test1", std::string(),
+ std::string(), true, "UTF-8", Time(), Time());
+ VerifyObserverCount(1);
+
+ // Test what happens when we try to add a TemplateURL with the same keyword as
+ // one in the model.
+ TemplateURLData data;
+ data.short_name = ASCIIToUTF16("second");
+ data.SetKeyword(ASCIIToUTF16("keyword"));
+ data.SetURL("http://test2");
+ data.safe_for_autoreplace = false;
+ TemplateURL* t_url = new TemplateURL(test_util_.profile(), data);
+ model()->Add(t_url);
+
+ // Because the old TemplateURL was replaceable and the new one wasn't, the new
+ // one should have replaced the old.
+ VerifyObserverCount(1);
+ EXPECT_EQ(t_url, model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")));
+ EXPECT_EQ(ASCIIToUTF16("second"), t_url->short_name());
+ EXPECT_EQ(ASCIIToUTF16("keyword"), t_url->keyword());
+ EXPECT_FALSE(t_url->safe_for_autoreplace());
+
+ // Now try adding a replaceable TemplateURL. This should just delete the
+ // passed-in URL.
+ data.short_name = ASCIIToUTF16("third");
+ data.SetURL("http://test3");
+ data.safe_for_autoreplace = true;
+ model()->Add(new TemplateURL(test_util_.profile(), data));
+ VerifyObserverCount(0);
+ EXPECT_EQ(t_url, model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")));
+ EXPECT_EQ(ASCIIToUTF16("second"), t_url->short_name());
+ EXPECT_EQ(ASCIIToUTF16("keyword"), t_url->keyword());
+ EXPECT_FALSE(t_url->safe_for_autoreplace());
+
+ // Now try adding a non-replaceable TemplateURL again. This should uniquify
+ // the existing entry's keyword.
+ data.short_name = ASCIIToUTF16("fourth");
+ data.SetURL("http://test4");
+ data.safe_for_autoreplace = false;
+ TemplateURL* t_url2 = new TemplateURL(test_util_.profile(), data);
+ model()->Add(t_url2);
+ VerifyObserverCount(2);
+ EXPECT_EQ(t_url2, model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")));
+ EXPECT_EQ(ASCIIToUTF16("fourth"), t_url2->short_name());
+ EXPECT_EQ(ASCIIToUTF16("keyword"), t_url2->keyword());
+ EXPECT_EQ(ASCIIToUTF16("second"), t_url->short_name());
+ EXPECT_EQ(ASCIIToUTF16("test2"), t_url->keyword());
+}
+
+TEST_F(TemplateURLServiceTest, AddExtensionKeyword) {
+ TemplateURL* original1 = AddKeywordWithDate("replaceable", "keyword1",
+ "http://test1", std::string(), std::string(), true, "UTF-8", Time(),
+ Time());
+ TemplateURL* original2 = AddKeywordWithDate("nonreplaceable", "keyword2",
+ "http://test2", std::string(), std::string(), false, "UTF-8", Time(),
+ Time());
+ TemplateURL* original3 = AddKeywordWithDate("extension", "keyword3",
+ std::string(chrome::kExtensionScheme) + "://test3", std::string(),
+ std::string(), false, "UTF-8", Time(), Time());
+
+ // Add an extension keyword that conflicts with each of the above three
+ // keywords.
+ TemplateURLData data;
+ data.short_name = ASCIIToUTF16("test");
+ data.SetKeyword(ASCIIToUTF16("keyword1"));
+ data.SetURL(std::string(chrome::kExtensionScheme) + "://test4");
+ data.safe_for_autoreplace = false;
+
+ // Extension keywords should override replaceable keywords.
+ TemplateURL* extension1 = new TemplateURL(test_util_.profile(), data);
+ model()->Add(extension1);
+ ASSERT_EQ(extension1,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1")));
+ model()->Remove(extension1);
+ EXPECT_EQ(original1,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword1")));
+
+ // They should not override non-replaceable keywords.
+ data.SetKeyword(ASCIIToUTF16("keyword2"));
+ TemplateURL* extension2 = new TemplateURL(test_util_.profile(), data);
+ model()->Add(extension2);
+ ASSERT_EQ(original2,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2")));
+ model()->Remove(original2);
+ EXPECT_EQ(extension2,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword2")));
+
+ // They should override extension keywords added earlier.
+ data.SetKeyword(ASCIIToUTF16("keyword3"));
+ TemplateURL* extension3 = new TemplateURL(test_util_.profile(), data);
+ model()->Add(extension3);
+ ASSERT_EQ(extension3,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3")));
+ model()->Remove(extension3);
+ EXPECT_EQ(original3,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3")));
+}
+
+TEST_F(TemplateURLServiceTest, AddSameKeywordWithExtensionPresent) {
+ test_util_.VerifyLoad();
+
+ // Similar to the AddSameKeyword test, but with an extension keyword masking a
+ // replaceable TemplateURL. We should still do correct conflict resolution
+ // between the non-template URLs.
+ AddKeywordWithDate("replaceable", "keyword", "http://test1", std::string(),
+ std::string(), true, "UTF-8", Time(), Time());
+ TemplateURL* extension = AddKeywordWithDate("extension", "keyword",
+ std::string(chrome::kExtensionScheme) + "://test2", std::string(),
+ std::string(), false, "UTF-8", Time(), Time());
+
+ // Adding another replaceable keyword should remove the existing one, but
+ // leave the extension as the associated URL for this keyword.
+ TemplateURLData data;
+ data.short_name = ASCIIToUTF16("name1");
+ data.SetKeyword(ASCIIToUTF16("keyword"));
+ data.SetURL("http://test3");
+ data.safe_for_autoreplace = true;
+ TemplateURL* t_url = new TemplateURL(test_util_.profile(), data);
+ model()->Add(t_url);
+ EXPECT_EQ(extension,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")));
+ EXPECT_TRUE(model()->GetTemplateURLForHost("test1") == NULL);
+ EXPECT_EQ(t_url, model()->GetTemplateURLForHost("test3"));
+
+ // Adding a nonreplaceable keyword should remove the existing replaceable
+ // keyword and replace the extension as the associated URL for this keyword,
+ // but not evict the extension from the service entirely.
+ data.short_name = ASCIIToUTF16("name2");
+ data.SetURL("http://test4");
+ data.safe_for_autoreplace = false;
+ TemplateURL* t_url2 = new TemplateURL(test_util_.profile(), data);
+ model()->Add(t_url2);
+ EXPECT_EQ(t_url2,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")));
+ EXPECT_TRUE(model()->GetTemplateURLForHost("test3") == NULL);
+ // Note that extensions don't use host-based keywords, so we can't check that
+ // the extension is still in the model using GetTemplateURLForHost(); and we
+ // have to create a full-fledged Extension to use
+ // GetTemplateURLForExtension(), which is annoying, so just grab all the URLs
+ // from the model and search for |extension| within them.
+ TemplateURLService::TemplateURLVector template_urls(
+ model()->GetTemplateURLs());
+ EXPECT_FALSE(std::find(template_urls.begin(), template_urls.end(),
+ extension) == template_urls.end());
+}
+
TEST_F(TemplateURLServiceTest, GenerateKeyword) {
- ASSERT_EQ(string16(), TemplateURLService::GenerateKeyword(GURL(), true));
- // Shouldn't generate keywords for https.
- ASSERT_EQ(string16(),
- TemplateURLService::GenerateKeyword(GURL("https://blah"), true));
ASSERT_EQ(ASCIIToUTF16("foo"),
- TemplateURLService::GenerateKeyword(GURL("http://foo"), true));
+ TemplateURLService::GenerateKeyword(GURL("http://foo")));
// www. should be stripped.
ASSERT_EQ(ASCIIToUTF16("foo"),
- TemplateURLService::GenerateKeyword(GURL("http://www.foo"), true));
- // Shouldn't generate keywords with paths, if autodetected.
- ASSERT_EQ(string16(),
- TemplateURLService::GenerateKeyword(GURL("http://blah/foo"), true));
+ TemplateURLService::GenerateKeyword(GURL("http://www.foo")));
+ // Make sure we don't get a trailing '/'.
ASSERT_EQ(ASCIIToUTF16("blah"),
- TemplateURLService::GenerateKeyword(GURL("http://blah/foo"),
- false));
- // FTP shouldn't generate a keyword.
- ASSERT_EQ(string16(),
- TemplateURLService::GenerateKeyword(GURL("ftp://blah/"), true));
- // Make sure we don't get a trailing /
- ASSERT_EQ(ASCIIToUTF16("blah"),
- TemplateURLService::GenerateKeyword(GURL("http://blah/"), true));
+ TemplateURLService::GenerateKeyword(GURL("http://blah/")));
// Don't generate the empty string.
ASSERT_EQ(ASCIIToUTF16("www"),
- TemplateURLService::GenerateKeyword(GURL("http://www."), false));
+ TemplateURLService::GenerateKeyword(GURL("http://www.")));
}
TEST_F(TemplateURLServiceTest, GenerateSearchURL) {
@@ -471,19 +605,19 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_Keywords) {
EXPECT_EQ(0U, model()->GetTemplateURLs().size());
// Create one with a 0 time.
- AddKeywordWithDate("name1", "key1", false, "http://foo1", "http://suggest1",
+ AddKeywordWithDate("name1", "key1", "http://foo1", "http://suggest1",
"http://icon1", true, "UTF-8;UTF-16", Time(), Time());
// Create one for now and +/- 1 day.
- AddKeywordWithDate("name2", "key2", false, "http://foo2", "http://suggest2",
+ AddKeywordWithDate("name2", "key2", "http://foo2", "http://suggest2",
"http://icon2", true, "UTF-8;UTF-16", now - one_day, Time());
- AddKeywordWithDate("name3", "key3", false, "http://foo3", std::string(),
+ AddKeywordWithDate("name3", "key3", "http://foo3", std::string(),
std::string(), true, std::string(), now, Time());
- AddKeywordWithDate("name4", "key4", false, "http://foo4", std::string(),
+ AddKeywordWithDate("name4", "key4", "http://foo4", std::string(),
std::string(), true, std::string(), now + one_day, Time());
// Try the other three states.
- AddKeywordWithDate("name5", "key5", false, "http://foo5", "http://suggest5",
+ AddKeywordWithDate("name5", "key5", "http://foo5", "http://suggest5",
"http://icon5", false, "UTF-8;UTF-16", now, Time());
- AddKeywordWithDate("name6", "key6", false, "http://foo6", "http://suggest6",
+ AddKeywordWithDate("name6", "key6", "http://foo6", "http://suggest6",
"http://icon6", false, "UTF-8;UTF-16", month_ago, Time());
// We just added a few items, validate them.
@@ -529,11 +663,11 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_KeywordsForOrigin) {
EXPECT_EQ(0U, model()->GetTemplateURLs().size());
// Create one for now and +/- 1 day.
- AddKeywordWithDate("name1", "key1", false, "http://foo1", "http://suggest1",
+ AddKeywordWithDate("name1", "key1", "http://foo1", "http://suggest1",
"http://icon2", true, "UTF-8;UTF-16", now - one_day, Time());
- AddKeywordWithDate("name2", "key2", false, "http://foo2", std::string(),
+ AddKeywordWithDate("name2", "key2", "http://foo2", std::string(),
std::string(), true, std::string(), now, Time());
- AddKeywordWithDate("name3", "key3", false, "http://foo3", std::string(),
+ AddKeywordWithDate("name3", "key3", "http://foo3", std::string(),
std::string(), true, std::string(), now + one_day, Time());
// We just added a few items, validate them.
@@ -618,7 +752,7 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProvider) {
// Add a new TemplateURL.
test_util_.VerifyLoad();
const size_t initial_count = model()->GetTemplateURLs().size();
- TemplateURL* t_url = AddKeywordWithDate("name1", "key1", false,
+ TemplateURL* t_url = AddKeywordWithDate("name1", "key1",
"http://foo1/{searchTerms}", "http://sugg1", "http://icon1", true,
"UTF-8;UTF-16", Time(), Time());
test_util_.ResetObserverCount();
@@ -642,36 +776,10 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProvider) {
AssertEquals(*cloned_url, *model()->GetDefaultSearchProvider());
}
-TEST_F(TemplateURLServiceTest, TemplateURLWithNoKeyword) {
- test_util_.VerifyLoad();
-
- const size_t initial_count = model()->GetTemplateURLs().size();
-
- AddKeywordWithDate("name1", std::string(), false, "http://foo1",
- "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time());
-
- // We just added a few items, validate them.
- ASSERT_EQ(initial_count + 1, model()->GetTemplateURLs().size());
-
- // Reload the model from the database and make sure we get the url back.
- test_util_.ResetModel(true);
-
- ASSERT_EQ(1 + initial_count, model()->GetTemplateURLs().size());
-
- bool found_keyword = false;
- for (size_t i = 0; i < initial_count + 1; ++i) {
- if (model()->GetTemplateURLs()[i]->keyword().empty()) {
- found_keyword = true;
- break;
- }
- }
- ASSERT_TRUE(found_keyword);
-}
-
TEST_F(TemplateURLServiceTest, CantReplaceWithSameKeyword) {
test_util_.ChangeModelToLoadState();
ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("foo"), GURL(), NULL));
- TemplateURL* t_url = AddKeywordWithDate("name1", "foo", false, "http://foo1",
+ TemplateURL* t_url = AddKeywordWithDate("name1", "foo", "http://foo1",
"http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time());
// Can still replace, newly added template url is marked safe to replace.
@@ -691,9 +799,8 @@ TEST_F(TemplateURLServiceTest, CantReplaceWithSameHosts) {
test_util_.ChangeModelToLoadState();
ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("foo"),
GURL("http://foo.com"), NULL));
- TemplateURL* t_url = AddKeywordWithDate("name1", "foo", false,
- "http://foo.com", "http://sugg1", "http://icon1", true, "UTF-8;UTF-16",
- Time(), Time());
+ TemplateURL* t_url = AddKeywordWithDate("name1", "foo", "http://foo.com",
+ "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time());
// Can still replace, newly added template url is marked safe to replace.
ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("bar"),
@@ -820,7 +927,7 @@ TEST_F(TemplateURLServiceTest, UpdateKeywordSearchTermsForURL) {
};
test_util_.ChangeModelToLoadState();
- AddKeywordWithDate("name", "x", false, "http://x/foo?q={searchTerms}",
+ AddKeywordWithDate("name", "x", "http://x/foo?q={searchTerms}",
"http://sugg1", "http://icon1", false, "UTF-8;UTF-16", Time(), Time());
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) {
@@ -842,7 +949,7 @@ TEST_F(TemplateURLServiceTest, DontUpdateKeywordSearchForNonReplaceable) {
};
test_util_.ChangeModelToLoadState();
- AddKeywordWithDate("name", "x", false, "http://x/foo", "http://sugg1",
+ AddKeywordWithDate("name", "x", "http://x/foo", "http://sugg1",
"http://icon1", false, "UTF-8;UTF-16", Time(), Time());
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) {
@@ -860,7 +967,7 @@ TEST_F(TemplateURLServiceTest, ChangeGoogleBaseValue) {
// test.
test_util_.ChangeModelToLoadState();
test_util_.SetGoogleBaseURL(GURL("http://google.com/"));
- const TemplateURL* t_url = AddKeywordWithDate("name", "google.com", true,
+ const TemplateURL* t_url = AddKeywordWithDate("name", "google.com",
"{google:baseURL}?q={searchTerms}", "http://sugg1", "http://icon1",
false, "UTF-8;UTF-16", Time(), Time());
ASSERT_EQ(t_url, model()->GetTemplateURLForHost("google.com"));
@@ -879,6 +986,35 @@ TEST_F(TemplateURLServiceTest, ChangeGoogleBaseValue) {
EXPECT_EQ(ASCIIToUTF16("google.co.uk"), t_url->keyword());
EXPECT_EQ("http://google.co.uk/?q=x", t_url->url_ref().ReplaceSearchTerms(
ASCIIToUTF16("x"), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16()));
+
+ // Now add a manual entry and then change the Google base URL such that the
+ // autogenerated Google search keyword would conflict.
+ TemplateURL* manual = AddKeywordWithDate("manual", "google.de",
+ "http://google.de/search?q={searchTerms}", std::string(), std::string(),
+ false, "UTF-8", Time(), Time());
+ test_util_.SetGoogleBaseURL(GURL("http://google.de"));
+
+ // Verify that the manual entry is untouched, and the autogenerated keyword
+ // has not changed.
+ ASSERT_EQ(manual,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("google.de")));
+ EXPECT_EQ("google.de", manual->url_ref().GetHost());
+ ASSERT_EQ(t_url,
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("google.co.uk")));
+ EXPECT_EQ("google.de", t_url->url_ref().GetHost());
+ EXPECT_EQ(ASCIIToUTF16("google.co.uk"), t_url->keyword());
+
+ // Change the base URL again and verify that the autogenerated keyword follows
+ // even though it didn't match the base URL, while the manual entry is still
+ // untouched.
+ test_util_.SetGoogleBaseURL(GURL("http://google.fr/"));
+ ASSERT_EQ(manual, model()->GetTemplateURLForHost("google.de"));
+ EXPECT_EQ("google.de", manual->url_ref().GetHost());
+ EXPECT_EQ(ASCIIToUTF16("google.de"), manual->keyword());
+ ASSERT_EQ(t_url, model()->GetTemplateURLForHost("google.fr"));
+ EXPECT_TRUE(model()->GetTemplateURLForHost("google.co.uk") == NULL);
+ EXPECT_EQ("google.fr", t_url->url_ref().GetHost());
+ EXPECT_EQ(ASCIIToUTF16("google.fr"), t_url->keyword());
}
struct QueryHistoryCallbackImpl {
@@ -907,7 +1043,7 @@ TEST_F(TemplateURLServiceTest, GenerateVisitOnKeyword) {
test_util_.profile()->CreateHistoryService(true, false);
// Create a keyword.
- TemplateURL* t_url = AddKeywordWithDate("keyword", "keyword", false,
+ TemplateURL* t_url = AddKeywordWithDate("keyword", "keyword",
"http://foo.com/foo?query={searchTerms}", "http://sugg1", "http://icon1",
true, "UTF-8;UTF-16", base::Time::Now(), base::Time::Now());
@@ -1114,29 +1250,6 @@ TEST_F(TemplateURLServiceTest, LoadEnsuresDefaultSearchProviderExists) {
EXPECT_TRUE(model()->GetDefaultSearchProvider()->SupportsReplacement());
}
-// Make sure that the load does update of auto-keywords correctly.
-// This test basically verifies that no asserts or crashes occur
-// during this operation.
-TEST_F(TemplateURLServiceTest, LoadDoesAutoKeywordUpdate) {
- string16 prepopulated_url;
- scoped_ptr<TemplateURL> t_url(
- CreateReplaceablePreloadedTemplateURL(false, 0, &prepopulated_url));
- TemplateURLData data(t_url->data());
- data.SetAutogenerateKeyword(true);
- data.SetURL("{google:baseURL}?q={searchTerms}");
-
- // Then add it to the model and save it all.
- test_util_.ChangeModelToLoadState();
- model()->Add(new TemplateURL(test_util_.profile(), data));
- test_util_.BlockTillServiceProcessesRequests();
-
- // Now reload the model and verify that the merge updates the url.
- test_util_.ResetModel(true);
-
- // Wait for any saves to finish.
- test_util_.BlockTillServiceProcessesRequests();
-}
-
// Simulates failing to load the webdb and makes sure the default search
// provider is valid.
TEST_F(TemplateURLServiceTest, FailedInit) {
@@ -1164,7 +1277,7 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) {
test_util_.ResetObserverCount();
// Set a regular default search provider.
- TemplateURL* regular_default = AddKeywordWithDate("name1", "key1", false,
+ TemplateURL* regular_default = AddKeywordWithDate("name1", "key1",
"http://foo1/{searchTerms}", "http://sugg1", "http://icon1", true,
"UTF-8;UTF-16", Time(), Time());
VerifyObserverCount(1);
@@ -1267,11 +1380,11 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) {
// First, remove the preferences, reset the model, and set a default.
test_util_.RemoveManagedDefaultSearchPreferences();
test_util_.ResetModel(true);
- TemplateURL* t_url = AddKeywordWithDate("name1", "key1", false,
- "http://foo1/{searchTerms}", "http://sugg1", "http://icon1", true,
- "UTF-8;UTF-16", Time(), Time());
- model()->SetDefaultSearchProvider(t_url);
- EXPECT_EQ(t_url, model()->GetDefaultSearchProvider());
+ TemplateURL* new_default =
+ model()->GetTemplateURLForKeyword(ASCIIToUTF16("key1"));
+ ASSERT_FALSE(new_default == NULL);
+ model()->SetDefaultSearchProvider(new_default);
+ EXPECT_EQ(new_default, model()->GetDefaultSearchProvider());
// Now reset the model again but load it after setting the preferences.
test_util_.ResetModel(false);
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index dbc5c39..708650b 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -491,20 +491,6 @@ TEST_F(TemplateURLTest, GoogleBaseSuggestURL) {
CheckSuggestBaseURL(data[i].base_url, data[i].base_suggest_url);
}
-TEST_F(TemplateURLTest, Keyword) {
- TemplateURLData data;
- data.SetURL("http://www.google.com/search");
- EXPECT_FALSE(data.autogenerate_keyword());
- data.SetKeyword(ASCIIToUTF16("foo"));
- EXPECT_EQ(ASCIIToUTF16("foo"), TemplateURL(NULL, data).keyword());
- data.SetAutogenerateKeyword(true);
- EXPECT_TRUE(data.autogenerate_keyword());
- EXPECT_EQ(ASCIIToUTF16("google.com"), TemplateURL(NULL, data).keyword());
- data.SetKeyword(ASCIIToUTF16("foo"));
- EXPECT_FALSE(data.autogenerate_keyword());
- EXPECT_EQ(ASCIIToUTF16("foo"), TemplateURL(NULL, data).keyword());
-}
-
TEST_F(TemplateURLTest, ParseParameterKnown) {
std::string parsed_url("{searchTerms}");
TemplateURLData data;
diff --git a/chrome/browser/search_engines/util.cc b/chrome/browser/search_engines/util.cc
index c8bd3c6..216d8f3 100644
--- a/chrome/browser/search_engines/util.cc
+++ b/chrome/browser/search_engines/util.cc
@@ -130,18 +130,17 @@ void MergeEnginesFromPrepopulateData(
if (!existing_url->safe_for_autoreplace()) {
data.safe_for_autoreplace = false;
data.SetKeyword(existing_url->keyword());
- data.SetAutogenerateKeyword(existing_url->autogenerate_keyword());
data.short_name = existing_url->short_name();
}
data.id = existing_url->id();
- url_in_vector = new TemplateURL(profile, data);
if (service)
- service->UpdateKeyword(*url_in_vector);
+ service->UpdateKeyword(data);
// Replace the entry in |template_urls| with the updated one.
std::vector<TemplateURL*>::iterator j = std::find(template_urls->begin(),
template_urls->end(), existing_url.get());
- *j = url_in_vector;
+ *j = new TemplateURL(profile, data);
+ url_in_vector = *j;
if (*default_search_provider == existing_url.get())
*default_search_provider = url_in_vector;
} else {
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index de05fcf..7214e0b 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -1831,8 +1831,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
if (tab_contents_wrapper &&
tab_contents_wrapper->search_engine_tab_helper() &&
tab_contents_wrapper->search_engine_tab_helper()->delegate()) {
- string16 keyword(TemplateURLService::GenerateKeyword(params_.page_url,
- false));
+ string16 keyword(TemplateURLService::GenerateKeyword(params_.page_url));
TemplateURLData data;
data.short_name = keyword;
data.SetKeyword(keyword);
@@ -1871,7 +1870,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
}
ProtocolHandlerRegistry::ProtocolHandlerList
-RenderViewContextMenu::GetHandlersForLinkUrl() {
+ RenderViewContextMenu::GetHandlersForLinkUrl() {
ProtocolHandlerRegistry::ProtocolHandlerList handlers =
protocol_handler_registry_->GetHandlersFor(params_.link_url.scheme());
std::sort(handlers.begin(), handlers.end());
diff --git a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
index 0a93c5b..d256a40 100644
--- a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
+++ b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc
@@ -12,6 +12,7 @@
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/search_engines/template_url_fetcher_ui_callbacks.h"
#include "chrome/common/render_messages.h"
+#include "chrome/common/url_constants.h"
#include "content/public/browser/favicon_status.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
@@ -45,7 +46,16 @@ string16 GenerateKeywordFromNavigationEntry(const NavigationEntry* entry) {
return string16();
}
- return TemplateURLService::GenerateKeyword(url, true);
+ // Don't autogenerate keywords for referrers that are anything other than HTTP
+ // or have a path.
+ //
+ // If we relax the path constraint, we need to be sure to sanitize the path
+ // elements and update AutocompletePopup to look for keywords using the path.
+ // See http://b/issue?id=863583.
+ if (!url.SchemeIs(chrome::kHttpScheme) || (url.path().length() > 1))
+ return string16();
+
+ return TemplateURLService::GenerateKeyword(url);
}
} // namespace
diff --git a/chrome/browser/webdata/keyword_table.cc b/chrome/browser/webdata/keyword_table.cc
index 8df072d..c08a6f7 100644
--- a/chrome/browser/webdata/keyword_table.cc
+++ b/chrome/browser/webdata/keyword_table.cc
@@ -18,7 +18,10 @@
#include "chrome/browser/history/history_database.h"
#include "chrome/browser/protector/histograms.h"
#include "chrome/browser/protector/protector_utils.h"
+#include "chrome/browser/search_engines/search_terms_data.h"
#include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/webdata/web_database.h"
#include "googleurl/src/gurl.h"
#include "sql/statement.h"
#include "sql/transaction.h"
@@ -35,34 +38,49 @@ const char KeywordTable::kBackupSignatureKey[] =
const char KeywordTable::kKeywordColumns[] = "id, short_name, keyword, "
"favicon_url, url, safe_for_autoreplace, originating_url, date_created, "
"usage_count, input_encodings, show_in_default_list, suggest_url, "
- "prepopulate_id, autogenerate_keyword, logo_id, created_by_policy, "
- "instant_url, last_modified, sync_guid";
+ "prepopulate_id, created_by_policy, instant_url, last_modified, sync_guid";
namespace {
// Keys used in the meta table.
const char kBuiltinKeywordVersion[] = "Builtin Keyword Version";
+// The set of columns up through version 44. (There were different columns
+// below version 29 but none of the code below needs to worry about that case.)
+const char kKeywordColumnsVersion44Concatenated[] = "id || short_name || "
+ "keyword || favicon_url || url || safe_for_autoreplace || "
+ "originating_url || date_created || usage_count || input_encodings || "
+ "show_in_default_list || suggest_url || prepopulate_id || "
+ "autogenerate_keyword || logo_id || created_by_policy || instant_url || "
+ "last_modified || sync_guid";
+const char kKeywordColumnsVersion44[] = "id, short_name, keyword, favicon_url, "
+ "url, safe_for_autoreplace, originating_url, date_created, usage_count, "
+ "input_encodings, show_in_default_list, suggest_url, prepopulate_id, "
+ "autogenerate_keyword, logo_id, created_by_policy, instant_url, "
+ "last_modified, sync_guid";
+// NOTE: Remember to change what |kKeywordColumnsVersion45| says if the column
+// set in |kKeywordColumns| changes, and update any code that needs to switch
+// column sets based on a version number!
+const char* const kKeywordColumnsVersion45 = KeywordTable::kKeywordColumns;
+
+// The current columns.
const char kKeywordColumnsConcatenated[] = "id || short_name || keyword || "
"favicon_url || url || safe_for_autoreplace || originating_url || "
"date_created || usage_count || input_encodings || show_in_default_list || "
- "suggest_url || prepopulate_id || autogenerate_keyword || logo_id || "
- "created_by_policy || instant_url || last_modified || sync_guid";
+ "suggest_url || prepopulate_id || created_by_policy || instant_url || "
+ "last_modified || sync_guid";
// Inserts the data from |data| into |s|. |s| is assumed to have slots for all
// the columns in the keyword table. |id_column| is the slot number to bind
-// |data|'s id() to; |starting_column| is the slot number of the first of a
+// |data|'s |id| to; |starting_column| is the slot number of the first of a
// contiguous set of slots to bind all the other fields to.
-void BindURLToStatement(const TemplateURL& url,
+void BindURLToStatement(const TemplateURLData& data,
sql::Statement* s,
int id_column,
int starting_column) {
- const TemplateURLData& data = url.data();
s->BindInt64(id_column, data.id);
s->BindString16(starting_column, data.short_name);
- // TODO(pkasting): See comment on TempalteURL::EnsureKeyword().
- s->BindString16(starting_column + 1,
- data.keyword(const_cast<TemplateURL*>(&url)));
+ s->BindString16(starting_column + 1, data.keyword());
s->BindString(starting_column + 2, data.favicon_url.is_valid() ?
history::HistoryDatabase::GURLToDatabaseURL(data.favicon_url) :
std::string());
@@ -77,12 +95,10 @@ void BindURLToStatement(const TemplateURL& url,
s->BindBool(starting_column + 9, data.show_in_default_list);
s->BindString(starting_column + 10, data.suggestions_url);
s->BindInt(starting_column + 11, data.prepopulate_id);
- s->BindBool(starting_column + 12, data.autogenerate_keyword());
- s->BindInt(starting_column + 13, 0);
- s->BindBool(starting_column + 14, data.created_by_policy);
- s->BindString(starting_column + 15, data.instant_url);
- s->BindInt64(starting_column + 16, data.last_modified.ToTimeT());
- s->BindString(starting_column + 17, data.sync_guid);
+ s->BindBool(starting_column + 12, data.created_by_policy);
+ s->BindString(starting_column + 13, data.instant_url);
+ s->BindInt64(starting_column + 14, data.last_modified.ToTimeT());
+ s->BindString(starting_column + 15, data.sync_guid);
}
// Signs search provider id and returns its signature.
@@ -116,27 +132,25 @@ bool KeywordTable::Init() {
"show_in_default_list INTEGER,"
"suggest_url VARCHAR,"
"prepopulate_id INTEGER DEFAULT 0,"
- "autogenerate_keyword INTEGER DEFAULT 0,"
- "logo_id INTEGER DEFAULT 0,"
"created_by_policy INTEGER DEFAULT 0,"
"instant_url VARCHAR,"
"last_modified INTEGER DEFAULT 0,"
"sync_guid VARCHAR)") &&
- UpdateBackupSignature());
+ UpdateBackupSignature(WebDatabase::kCurrentVersionNumber));
}
bool KeywordTable::IsSyncable() {
return true;
}
-bool KeywordTable::AddKeyword(const TemplateURL& url) {
- DCHECK(url.id());
+bool KeywordTable::AddKeyword(const TemplateURLData& data) {
+ DCHECK(data.id);
std::string query("INSERT INTO keywords (" + std::string(kKeywordColumns) +
- ") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)");
+ ") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)");
sql::Statement s(db_->GetUniqueStatement(query.c_str()));
- BindURLToStatement(url, &s, 0, 1);
+ BindURLToStatement(data, &s, 0, 1);
- return s.Run() && UpdateBackupSignature();
+ return s.Run() && UpdateBackupSignature(WebDatabase::kCurrentVersionNumber);
}
bool KeywordTable::RemoveKeyword(TemplateURLID id) {
@@ -145,7 +159,7 @@ bool KeywordTable::RemoveKeyword(TemplateURLID id) {
db_->GetUniqueStatement("DELETE FROM keywords WHERE id = ?"));
s.BindInt64(0, id);
- return s.Run() && UpdateBackupSignature();
+ return s.Run() && UpdateBackupSignature(WebDatabase::kCurrentVersionNumber);
}
bool KeywordTable::GetKeywords(Keywords* keywords) {
@@ -168,17 +182,17 @@ bool KeywordTable::GetKeywords(Keywords* keywords) {
return succeeded;
}
-bool KeywordTable::UpdateKeyword(const TemplateURL& url) {
- DCHECK(url.id());
+bool KeywordTable::UpdateKeyword(const TemplateURLData& data) {
+ DCHECK(data.id);
sql::Statement s(db_->GetUniqueStatement("UPDATE keywords SET short_name=?, "
"keyword=?, favicon_url=?, url=?, safe_for_autoreplace=?, "
"originating_url=?, date_created=?, usage_count=?, input_encodings=?, "
"show_in_default_list=?, suggest_url=?, prepopulate_id=?, "
- "autogenerate_keyword=?, logo_id=?, created_by_policy=?, instant_url=?, "
- "last_modified=?, sync_guid=? WHERE id=?"));
- BindURLToStatement(url, &s, 18, 0); // "18" binds id() as the last item.
+ "created_by_policy=?, instant_url=?, last_modified=?, sync_guid=? WHERE "
+ "id=?"));
+ BindURLToStatement(data, &s, 16, 0); // "16" binds id() as the last item.
- return s.Run() && UpdateBackupSignature();
+ return s.Run() && UpdateBackupSignature(WebDatabase::kCurrentVersionNumber);
}
bool KeywordTable::SetDefaultSearchProviderID(int64 id) {
@@ -186,7 +200,7 @@ bool KeywordTable::SetDefaultSearchProviderID(int64 id) {
UMA_HISTOGRAM_COUNTS_100("Search.DefaultSearchProviderID",
static_cast<int32>(id));
return meta_table_->SetValue(kDefaultSearchProviderKey, id) &&
- UpdateBackupSignature();
+ UpdateBackupSignature(WebDatabase::kCurrentVersionNumber);
}
int64 KeywordTable::GetDefaultSearchProviderID() {
@@ -196,7 +210,7 @@ int64 KeywordTable::GetDefaultSearchProviderID() {
}
bool KeywordTable::GetDefaultSearchProviderBackup(TemplateURLData* backup) {
- if (!IsBackupSignatureValid())
+ if (!IsBackupSignatureValid(WebDatabase::kCurrentVersionNumber))
return false;
int64 backup_id = kInvalidTemplateURLID;
@@ -212,7 +226,7 @@ bool KeywordTable::GetDefaultSearchProviderBackup(TemplateURLData* backup) {
if (!s.Step()) {
LOG_IF(ERROR, s.Succeeded())
<< "No default search provider with backup id.";
- return NULL;
+ return false;
}
if (!GetKeywordDataFromStatement(s, backup))
@@ -225,7 +239,7 @@ bool KeywordTable::GetDefaultSearchProviderBackup(TemplateURLData* backup) {
}
bool KeywordTable::DidDefaultSearchProviderChange() {
- if (!IsBackupSignatureValid()) {
+ if (!IsBackupSignatureValid(WebDatabase::kCurrentVersionNumber)) {
UMA_HISTOGRAM_ENUMERATION(
protector::kProtectorHistogramDefaultSearchProvider,
protector::kProtectorErrorBackupInvalid,
@@ -340,7 +354,35 @@ bool KeywordTable::MigrateToVersion39AddSyncGUIDColumn() {
}
bool KeywordTable::MigrateToVersion44AddDefaultSearchProviderBackup() {
- return IsBackupSignatureValid() || UpdateBackupSignature();
+ return IsBackupSignatureValid(44) || UpdateBackupSignature(44);
+}
+
+bool KeywordTable::MigrateToVersion45RemoveLogoIDAndAutogenerateColumns() {
+ sql::Transaction transaction(db_);
+ if (!transaction.Begin())
+ return false;
+
+ // The version 43 migration should have been written to do this, but since it
+ // wasn't, we'll do it now. Unfortunately a previous change deleted this for
+ // some users, so we can't be sure this will succeed (so don't bail on error).
+ meta_table_->DeleteKey("Default Search Provider Backup");
+
+ if (!MigrateKeywordsTableForVersion45("keywords"))
+ return false;
+
+ if (IsBackupSignatureValid(44)) {
+ // Migrate the keywords backup table as well.
+ if (!MigrateKeywordsTableForVersion45("keywords_backup") || !SignBackup(45))
+ return false;
+ } else {
+ // Old backup was invalid; drop the table entirely, which will trigger the
+ // protector code to prompt the user and recreate the table.
+ if (db_->DoesTableExist("keywords_backup") &&
+ !db_->Execute("DROP TABLE keywords_backup"))
+ return false;
+ }
+
+ return transaction.Commit();
}
// static
@@ -349,7 +391,6 @@ bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s,
DCHECK(data);
data->short_name = s.ColumnString16(1);
data->SetKeyword(s.ColumnString16(2));
- data->SetAutogenerateKeyword(s.ColumnBool(13));
// Due to past bugs, we might have persisted entries with empty URLs. Avoid
// reading these out. (GetKeywords() will delete these entries on return.)
// NOTE: This code should only be needed as long as we might be reading such
@@ -358,7 +399,7 @@ bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s,
return false;
data->SetURL(s.ColumnString(4));
data->suggestions_url = s.ColumnString(11);
- data->instant_url = s.ColumnString(16);
+ data->instant_url = s.ColumnString(14);
data->favicon_url = GURL(s.ColumnString(3));
data->originating_url = GURL(s.ColumnString(6));
data->show_in_default_list = s.ColumnBool(10);
@@ -366,15 +407,15 @@ bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s,
base::SplitString(s.ColumnString(9), ';', &data->input_encodings);
data->id = s.ColumnInt64(0);
data->date_created = Time::FromTimeT(s.ColumnInt64(7));
- data->last_modified = Time::FromTimeT(s.ColumnInt64(17));
- data->created_by_policy = s.ColumnBool(15);
+ data->last_modified = Time::FromTimeT(s.ColumnInt64(15));
+ data->created_by_policy = s.ColumnBool(13);
data->usage_count = s.ColumnInt(8);
data->prepopulate_id = s.ColumnInt(12);
- data->sync_guid = s.ColumnString(18);
+ data->sync_guid = s.ColumnString(16);
return true;
}
-bool KeywordTable::GetSignatureData(std::string* backup) {
+bool KeywordTable::GetSignatureData(int table_version, std::string* backup) {
DCHECK(backup);
int64 backup_value = kInvalidTemplateURLID;
@@ -384,7 +425,8 @@ bool KeywordTable::GetSignatureData(std::string* backup) {
}
std::string keywords_backup_data;
- if (!GetTableContents("keywords_backup", &keywords_backup_data)) {
+ if (!GetTableContents("keywords_backup", table_version,
+ &keywords_backup_data)) {
LOG(ERROR) << "Can't get keywords backup data";
return false;
}
@@ -393,6 +435,7 @@ bool KeywordTable::GetSignatureData(std::string* backup) {
}
bool KeywordTable::GetTableContents(const char* table_name,
+ int table_version,
std::string* contents) {
DCHECK(contents);
@@ -400,16 +443,19 @@ bool KeywordTable::GetTableContents(const char* table_name,
return false;
contents->clear();
- std::string query("SELECT " + std::string(kKeywordColumnsConcatenated) +
+ std::string query("SELECT " +
+ std::string((table_version <= 44) ?
+ kKeywordColumnsVersion44Concatenated : kKeywordColumnsConcatenated) +
" FROM " + std::string(table_name) + " ORDER BY id ASC");
- sql::Statement s(db_->GetCachedStatement(sql::StatementID(table_name),
- query.c_str()));
+ sql::Statement s((table_version == WebDatabase::kCurrentVersionNumber) ?
+ db_->GetCachedStatement(sql::StatementID(table_name), query.c_str()) :
+ db_->GetUniqueStatement(query.c_str()));
while (s.Step())
*contents += s.ColumnString(0);
return s.Succeeded();
}
-bool KeywordTable::UpdateBackupSignature() {
+bool KeywordTable::UpdateBackupSignature(int table_version) {
sql::Transaction transaction(db_);
if (!transaction.Begin())
return false;
@@ -426,14 +472,20 @@ bool KeywordTable::UpdateBackupSignature() {
return false;
std::string query("CREATE TABLE keywords_backup AS SELECT " +
- std::string(kKeywordColumns) + " FROM keywords ORDER BY id ASC");
+ std::string((table_version <= 44) ?
+ kKeywordColumnsVersion44 : kKeywordColumns) +
+ " FROM keywords ORDER BY id ASC");
if (!db_->Execute(query.c_str())) {
LOG(ERROR) << "Failed to create keywords_backup table.";
return false;
}
+ return SignBackup(table_version) && transaction.Commit();
+}
+
+bool KeywordTable::SignBackup(int table_version) {
std::string data_to_sign;
- if (!GetSignatureData(&data_to_sign)) {
+ if (!GetSignatureData(table_version, &data_to_sign)) {
LOG(ERROR) << "No data to sign.";
return false;
}
@@ -444,15 +496,14 @@ bool KeywordTable::UpdateBackupSignature() {
return false;
}
- return meta_table_->SetValue(kBackupSignatureKey, signature) &&
- transaction.Commit();
+ return meta_table_->SetValue(kBackupSignatureKey, signature);
}
-bool KeywordTable::IsBackupSignatureValid() {
+bool KeywordTable::IsBackupSignatureValid(int table_version) {
std::string signature;
std::string signature_data;
return meta_table_->GetValue(kBackupSignatureKey, &signature) &&
- GetSignatureData(&signature_data) &&
+ GetSignatureData(table_version, &signature_data) &&
protector::IsSettingValid(signature_data, signature);
}
@@ -489,3 +540,88 @@ bool KeywordTable::UpdateDefaultSearchProviderIDBackup(TemplateURLID* id) {
*id = default_search_id;
return true;
}
+
+bool KeywordTable::MigrateKeywordsTableForVersion45(const std::string& name) {
+ // Create a new table without the columns we're dropping.
+ if (!db_->Execute("CREATE TABLE keywords_temp ("
+ "id INTEGER PRIMARY KEY,"
+ "short_name VARCHAR NOT NULL,"
+ "keyword VARCHAR NOT NULL,"
+ "favicon_url VARCHAR NOT NULL,"
+ "url VARCHAR NOT NULL,"
+ "safe_for_autoreplace INTEGER,"
+ "originating_url VARCHAR,"
+ "date_created INTEGER DEFAULT 0,"
+ "usage_count INTEGER DEFAULT 0,"
+ "input_encodings VARCHAR,"
+ "show_in_default_list INTEGER,"
+ "suggest_url VARCHAR,"
+ "prepopulate_id INTEGER DEFAULT 0,"
+ "created_by_policy INTEGER DEFAULT 0,"
+ "instant_url VARCHAR,"
+ "last_modified INTEGER DEFAULT 0,"
+ "sync_guid VARCHAR)"))
+ return false;
+ std::string sql("INSERT INTO keywords_temp SELECT " +
+ std::string(kKeywordColumnsVersion45) + " FROM " + name);
+ if (!db_->Execute(sql.c_str()))
+ return false;
+
+ // NOTE: The ORDER BY here ensures that the uniquing process for keywords will
+ // happen identically on both the normal and backup tables.
+ sql = "SELECT id, keyword, url, autogenerate_keyword FROM " + name +
+ " ORDER BY id ASC";
+ sql::Statement s(db_->GetUniqueStatement(sql.c_str()));
+ string16 placeholder_keyword(ASCIIToUTF16("dummy"));
+ std::set<string16> keywords;
+ while (s.Step()) {
+ string16 keyword(s.ColumnString16(1));
+ bool generate_keyword = keyword.empty() || s.ColumnBool(3);
+ if (generate_keyword)
+ keyword = placeholder_keyword;
+ TemplateURLData data;
+ data.SetKeyword(keyword);
+ data.SetURL(s.ColumnString(2));
+ TemplateURL turl(NULL, data);
+ // Don't persist extension keywords to disk. These will get added to the
+ // TemplateURLService as the extensions are loaded.
+ bool delete_entry = turl.IsExtensionKeyword();
+ if (!delete_entry && generate_keyword) {
+ // Explicitly generate keywords for all rows with the autogenerate bit set
+ // or where the keyword is empty.
+ SearchTermsData terms_data;
+ GURL url(TemplateURLService::GenerateSearchURLUsingTermsData(&turl,
+ terms_data));
+ if (!url.is_valid()) {
+ delete_entry = true;
+ } else {
+ // Ensure autogenerated keywords are unique.
+ keyword = TemplateURLService::GenerateKeyword(url);
+ while (keywords.count(keyword))
+ keyword.append(ASCIIToUTF16("_"));
+ sql::Statement u(db_->GetUniqueStatement(
+ "UPDATE keywords_temp SET keyword=? WHERE id=?"));
+ u.BindString16(0, keyword);
+ u.BindInt64(1, s.ColumnInt64(0));
+ if (!u.Run())
+ return false;
+ }
+ }
+ if (delete_entry) {
+ sql::Statement u(db_->GetUniqueStatement(
+ "DELETE FROM keywords_temp WHERE id=?"));
+ u.BindInt64(0, s.ColumnInt64(0));
+ if (!u.Run())
+ return false;
+ } else {
+ keywords.insert(keyword);
+ }
+ }
+
+ // Replace the old table with the new one.
+ sql = "DROP TABLE " + name;
+ if (!db_->Execute(sql.c_str()))
+ return false;
+ sql = "ALTER TABLE keywords_temp RENAME TO " + name;
+ return db_->Execute(sql.c_str());
+}
diff --git a/chrome/browser/webdata/keyword_table.h b/chrome/browser/webdata/keyword_table.h
index a5285e7..b5d147f 100644
--- a/chrome/browser/webdata/keyword_table.h
+++ b/chrome/browser/webdata/keyword_table.h
@@ -15,7 +15,6 @@
#include "chrome/browser/webdata/web_database_table.h"
#include "chrome/browser/search_engines/template_url_id.h"
-class TemplateURL;
struct TemplateURLData;
namespace sql {
@@ -46,7 +45,6 @@ class Statement;
// suggest_url
// prepopulate_id See TemplateURLData::prepopulate_id.
// autogenerate_keyword
-// logo_id Deprecated, to be removed; see crbug.com/113248.
// created_by_policy See TemplateURLData::created_by_policy. This was
// added in version 26.
// instant_url See TemplateURLData::instant_url. This was added in
@@ -104,7 +102,7 @@ class KeywordTable : public WebDatabaseTable {
// Adds a new keyword, updating the id field on success.
// Returns true if successful.
- bool AddKeyword(const TemplateURL& url);
+ bool AddKeyword(const TemplateURLData& data);
// Removes the specified keyword.
// Returns true if successful.
@@ -117,7 +115,7 @@ class KeywordTable : public WebDatabaseTable {
// Updates the database values for the specified url.
// Returns true on success.
- bool UpdateKeyword(const TemplateURL& url);
+ bool UpdateKeyword(const TemplateURLData& data);
// ID (TemplateURLData->id) of the default search provider.
bool SetDefaultSearchProviderID(int64 id);
@@ -145,12 +143,14 @@ class KeywordTable : public WebDatabaseTable {
bool MigrateToVersion38AddLastModifiedColumn();
bool MigrateToVersion39AddSyncGUIDColumn();
bool MigrateToVersion44AddDefaultSearchProviderBackup();
+ bool MigrateToVersion45RemoveLogoIDAndAutogenerateColumns();
private:
FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, DefaultSearchProviderBackup);
FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, GetTableContents);
FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, GetTableContentsOrdering);
FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, SanitizeURLs);
+ FRIEND_TEST_ALL_PREFIXES(WebDatabaseMigrationTest, MigrateVersion44ToCurrent);
// NOTE: Since the table columns have changed in different versions, many
// functions below take a |table_version| argument which dictates which
@@ -165,19 +165,25 @@ class KeywordTable : public WebDatabaseTable {
// Returns contents of |keywords_backup| table and default search provider
// id backup as a string through |data|. Return value is true on success,
// false otherwise.
- bool GetSignatureData(std::string* data);
+ bool GetSignatureData(int table_version, std::string* data);
// Returns contents of selected table as a string in |contents| parameter.
// Returns true on success, false otherwise.
- bool GetTableContents(const char* table_name, std::string* contents);
+ bool GetTableContents(const char* table_name,
+ int table_version,
+ std::string* contents);
// Updates settings backup, signs it and stores the signature in the
// database. Returns true on success.
- bool UpdateBackupSignature();
+ bool UpdateBackupSignature(int table_version);
+
+ // Signs the backup table. This is a subset of what UpdateBackupSignature()
+ // does.
+ bool SignBackup(int table_version);
// Checks the signature for the current settings backup. Returns true
// if signature is valid, false otherwise.
- bool IsBackupSignatureValid();
+ bool IsBackupSignatureValid(int table_version);
// Gets a string representation for keyword with id specified.
// Used to store its result in |meta| table or to compare with another
@@ -190,6 +196,10 @@ class KeywordTable : public WebDatabaseTable {
// true on success. The id is returned back via |id| parameter.
bool UpdateDefaultSearchProviderIDBackup(TemplateURLID* id);
+ // Migrates table |name| (which should be either "keywords" or
+ // "keywords_backup" from version 44 to version 45.
+ bool MigrateKeywordsTableForVersion45(const std::string& name);
+
DISALLOW_COPY_AND_ASSIGN(KeywordTable);
};
diff --git a/chrome/browser/webdata/keyword_table_unittest.cc b/chrome/browser/webdata/keyword_table_unittest.cc
index 5af4343..9dd5613 100644
--- a/chrome/browser/webdata/keyword_table_unittest.cc
+++ b/chrome/browser/webdata/keyword_table_unittest.cc
@@ -69,8 +69,7 @@ TEST_F(KeywordTableTest, Keywords) {
keyword.created_by_policy = true;
keyword.usage_count = 32;
keyword.prepopulate_id = 10;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
KeywordTable::Keywords keywords;
EXPECT_TRUE(keyword_table->GetKeywords(&keywords));
@@ -78,9 +77,7 @@ TEST_F(KeywordTableTest, Keywords) {
const TemplateURLData& restored_keyword = keywords.front();
EXPECT_EQ(keyword.short_name, restored_keyword.short_name);
- EXPECT_EQ(url.keyword(), TemplateURL(NULL, restored_keyword).keyword());
- EXPECT_EQ(keyword.autogenerate_keyword(),
- restored_keyword.autogenerate_keyword());
+ EXPECT_EQ(keyword.keyword(), restored_keyword.keyword());
EXPECT_EQ(keyword.url(), restored_keyword.url());
EXPECT_EQ(keyword.suggestions_url, restored_keyword.suggestions_url);
EXPECT_EQ(keyword.instant_url, restored_keyword.instant_url);
@@ -133,8 +130,7 @@ TEST_F(KeywordTableTest, KeywordMisc) {
keyword.created_by_policy = true;
keyword.usage_count = 32;
keyword.prepopulate_id = 10;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
EXPECT_TRUE(keyword_table->SetDefaultSearchProviderID(10));
EXPECT_TRUE(keyword_table->SetBuiltinKeywordVersion(11));
@@ -160,11 +156,11 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
keyword.show_in_default_list = true;
keyword.safe_for_autoreplace = true;
keyword.id = 1;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
EXPECT_TRUE(keyword_table->SetDefaultSearchProviderID(1));
- EXPECT_TRUE(keyword_table->IsBackupSignatureValid());
+ EXPECT_TRUE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID());
TemplateURLData backup;
@@ -172,7 +168,7 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
// Backup URL should have an invalid ID.
EXPECT_EQ(kInvalidTemplateURLID, backup.id);
EXPECT_EQ(keyword.short_name, backup.short_name);
- EXPECT_EQ(url.keyword(), TemplateURL(NULL, backup).keyword());
+ EXPECT_EQ(keyword.keyword(), backup.keyword());
EXPECT_EQ(keyword.url(), backup.url());
EXPECT_EQ(keyword.favicon_url, backup.favicon_url);
EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url);
@@ -183,13 +179,14 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
// Change the actual setting.
EXPECT_TRUE(keyword_table->meta_table_->SetValue(
KeywordTable::kDefaultSearchProviderKey, 2));
- EXPECT_TRUE(keyword_table->IsBackupSignatureValid());
+ EXPECT_TRUE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(2, keyword_table->GetDefaultSearchProviderID());
EXPECT_TRUE(keyword_table->GetDefaultSearchProviderBackup(&backup));
EXPECT_EQ(kInvalidTemplateURLID, backup.id);
EXPECT_EQ(keyword.short_name, backup.short_name);
- EXPECT_EQ(url.keyword(), TemplateURL(NULL, backup).keyword());
+ EXPECT_EQ(keyword.keyword(), backup.keyword());
EXPECT_EQ(keyword.url(), backup.url());
EXPECT_EQ(keyword.favicon_url, backup.favicon_url);
EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url);
@@ -202,7 +199,8 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
KeywordTable::kDefaultSearchProviderKey, 1));
EXPECT_TRUE(keyword_table->meta_table_->SetValue(
KeywordTable::kDefaultSearchIDBackupKey, 2));
- EXPECT_FALSE(keyword_table->IsBackupSignatureValid());
+ EXPECT_FALSE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID());
EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup));
EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange());
@@ -212,23 +210,26 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
KeywordTable::kDefaultSearchIDBackupKey, 1));
EXPECT_TRUE(keyword_table->meta_table_->SetValue(
KeywordTable::kBackupSignatureKey, std::string()));
- EXPECT_FALSE(keyword_table->IsBackupSignatureValid());
+ EXPECT_FALSE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID());
EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup));
EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange());
// Change keywords.
- EXPECT_TRUE(keyword_table->UpdateBackupSignature());
+ EXPECT_TRUE(keyword_table->UpdateBackupSignature(
+ WebDatabase::kCurrentVersionNumber));
sql::Statement remove_keyword(keyword_table->db_->GetUniqueStatement(
"DELETE FROM keywords WHERE id=1"));
EXPECT_TRUE(remove_keyword.Run());
- EXPECT_TRUE(keyword_table->IsBackupSignatureValid());
+ EXPECT_TRUE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID());
EXPECT_TRUE(keyword_table->GetDefaultSearchProviderBackup(&backup));
EXPECT_EQ(kInvalidTemplateURLID, backup.id);
EXPECT_EQ(keyword.short_name, backup.short_name);
- EXPECT_EQ(url.keyword(), TemplateURL(NULL, backup).keyword());
+ EXPECT_EQ(keyword.keyword(), backup.keyword());
EXPECT_EQ(keyword.url(), backup.url());
EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url);
EXPECT_EQ(keyword.favicon_url, backup.favicon_url);
@@ -240,7 +241,8 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) {
sql::Statement remove_keyword_backup(keyword_table->db_->GetUniqueStatement(
"DELETE FROM keywords_backup WHERE id=1"));
EXPECT_TRUE(remove_keyword_backup.Run());
- EXPECT_FALSE(keyword_table->IsBackupSignatureValid());
+ EXPECT_FALSE(keyword_table->IsBackupSignatureValid(
+ WebDatabase::kCurrentVersionNumber));
EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID());
EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup));
EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange());
@@ -263,29 +265,29 @@ TEST_F(KeywordTableTest, GetTableContents) {
keyword.date_created = base::Time::UnixEpoch();
keyword.last_modified = base::Time::UnixEpoch();
keyword.sync_guid = "1234-5678-90AB-CDEF";
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
- keyword.SetAutogenerateKeyword(true);
+ keyword.SetKeyword(ASCIIToUTF16("url"));
keyword.instant_url = "http://instant2/";
keyword.originating_url = GURL("http://originating.url/");
keyword.input_encodings.push_back("Shift_JIS");
keyword.id = 2;
keyword.prepopulate_id = 5;
keyword.sync_guid = "FEDC-BA09-8765-4321";
- TemplateURL url2(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url2));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
const char kTestContents[] = "1short_namekeywordhttp://favicon.url/"
- "http://url/1001url2000001234-5678-90AB-CDEF2short_nameurl"
- "http://favicon.url/http://url/1http://originating.url/00Shift_JIS1url251"
- "00http://instant2/0FEDC-BA09-8765-4321";
+ "http://url/1001url20001234-5678-90AB-CDEF2short_nameurl"
+ "http://favicon.url/http://url/1http://originating.url/00Shift_JIS1url250"
+ "http://instant2/0FEDC-BA09-8765-4321";
std::string contents;
- EXPECT_TRUE(keyword_table->GetTableContents("keywords", &contents));
+ EXPECT_TRUE(keyword_table->GetTableContents("keywords",
+ WebDatabase::kCurrentVersionNumber, &contents));
EXPECT_EQ(kTestContents, contents);
- EXPECT_TRUE(keyword_table->GetTableContents("keywords_backup", &contents));
+ EXPECT_TRUE(keyword_table->GetTableContents("keywords_backup",
+ WebDatabase::kCurrentVersionNumber, &contents));
EXPECT_EQ(kTestContents, contents);
}
@@ -306,29 +308,29 @@ TEST_F(KeywordTableTest, GetTableContentsOrdering) {
keyword.date_created = base::Time::UnixEpoch();
keyword.last_modified = base::Time::UnixEpoch();
keyword.sync_guid = "1234-5678-90AB-CDEF";
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
- keyword.SetAutogenerateKeyword(true);
+ keyword.SetKeyword(ASCIIToUTF16("url"));
keyword.instant_url = "http://instant2/";
keyword.originating_url = GURL("http://originating.url/");
keyword.input_encodings.push_back("Shift_JIS");
keyword.id = 1;
keyword.prepopulate_id = 5;
keyword.sync_guid = "FEDC-BA09-8765-4321";
- TemplateURL url2(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url2));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
const char kTestContents[] = "1short_nameurlhttp://favicon.url/http://url/1"
- "http://originating.url/00Shift_JIS1url25100http://instant2/0"
+ "http://originating.url/00Shift_JIS1url250http://instant2/0"
"FEDC-BA09-8765-43212short_namekeywordhttp://favicon.url/http://url/1001"
- "url2000001234-5678-90AB-CDEF";
+ "url20001234-5678-90AB-CDEF";
std::string contents;
- EXPECT_TRUE(keyword_table->GetTableContents("keywords", &contents));
+ EXPECT_TRUE(keyword_table->GetTableContents("keywords",
+ WebDatabase::kCurrentVersionNumber, &contents));
EXPECT_EQ(kTestContents, contents);
- EXPECT_TRUE(keyword_table->GetTableContents("keywords_backup", &contents));
+ EXPECT_TRUE(keyword_table->GetTableContents("keywords_backup",
+ WebDatabase::kCurrentVersionNumber, &contents));
EXPECT_EQ(kTestContents, contents);
}
@@ -346,16 +348,14 @@ TEST_F(KeywordTableTest, UpdateKeyword) {
keyword.show_in_default_list = true;
keyword.safe_for_autoreplace = true;
keyword.id = 1;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
- keyword.originating_url = GURL("http://originating.url/");
- keyword.SetAutogenerateKeyword(true);
+ keyword.SetKeyword(ASCIIToUTF16("url"));
keyword.instant_url = "http://instant2/";
+ keyword.originating_url = GURL("http://originating.url/");
keyword.input_encodings.push_back("Shift_JIS");
keyword.prepopulate_id = 5;
- TemplateURL url2(NULL, keyword);
- EXPECT_TRUE(keyword_table->UpdateKeyword(url2));
+ EXPECT_TRUE(keyword_table->UpdateKeyword(keyword));
KeywordTable::Keywords keywords;
EXPECT_TRUE(keyword_table->GetKeywords(&keywords));
@@ -363,9 +363,7 @@ TEST_F(KeywordTableTest, UpdateKeyword) {
const TemplateURLData& restored_keyword = keywords.front();
EXPECT_EQ(keyword.short_name, restored_keyword.short_name);
- EXPECT_EQ(url2.keyword(), TemplateURL(NULL, restored_keyword).keyword());
- EXPECT_EQ(keyword.autogenerate_keyword(),
- restored_keyword.autogenerate_keyword());
+ EXPECT_EQ(keyword.keyword(), restored_keyword.keyword());
EXPECT_EQ(keyword.suggestions_url, restored_keyword.suggestions_url);
EXPECT_EQ(keyword.instant_url, restored_keyword.instant_url);
EXPECT_EQ(keyword.favicon_url, restored_keyword.favicon_url);
@@ -390,8 +388,7 @@ TEST_F(KeywordTableTest, KeywordWithNoFavicon) {
keyword.SetURL("http://url/");
keyword.safe_for_autoreplace = true;
keyword.id = -100;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
KeywordTable::Keywords keywords;
EXPECT_TRUE(keyword_table->GetKeywords(&keywords));
@@ -399,7 +396,7 @@ TEST_F(KeywordTableTest, KeywordWithNoFavicon) {
const TemplateURLData& restored_keyword = keywords.front();
EXPECT_EQ(keyword.short_name, restored_keyword.short_name);
- EXPECT_EQ(url.keyword(), TemplateURL(NULL, restored_keyword).keyword());
+ EXPECT_EQ(keyword.keyword(), restored_keyword.keyword());
EXPECT_EQ(keyword.favicon_url, restored_keyword.favicon_url);
EXPECT_EQ(keyword.safe_for_autoreplace,
restored_keyword.safe_for_autoreplace);
@@ -416,14 +413,12 @@ TEST_F(KeywordTableTest, SanitizeURLs) {
keyword.SetKeyword(ASCIIToUTF16("legit"));
keyword.SetURL("http://url/");
keyword.id = 1000;
- TemplateURL url(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
keyword.short_name = ASCIIToUTF16("bogus");
keyword.SetKeyword(ASCIIToUTF16("bogus"));
keyword.id = 2000;
- TemplateURL url2(NULL, keyword);
- EXPECT_TRUE(keyword_table->AddKeyword(url2));
+ EXPECT_TRUE(keyword_table->AddKeyword(keyword));
KeywordTable::Keywords keywords;
EXPECT_TRUE(keyword_table->GetKeywords(&keywords));
@@ -437,7 +432,8 @@ TEST_F(KeywordTableTest, SanitizeURLs) {
s.BindString16(0, string16());
s.BindInt64(1, 2000);
EXPECT_TRUE(s.Run());
- EXPECT_TRUE(keyword_table->UpdateBackupSignature());
+ EXPECT_TRUE(keyword_table->UpdateBackupSignature(
+ WebDatabase::kCurrentVersionNumber));
// GetKeywords() should erase the entry with the empty URL field.
EXPECT_TRUE(keyword_table->GetKeywords(&keywords));
diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index b9c5c05..6cf2631 100644
--- a/chrome/browser/webdata/web_data_service.cc
+++ b/chrome/browser/webdata/web_data_service.cc
@@ -147,12 +147,10 @@ WebDatabase* WebDataService::GetDatabase() {
//
//////////////////////////////////////////////////////////////////////////////
-void WebDataService::AddKeyword(const TemplateURL& url) {
- // Ensure that the keyword is already generated (and cached) before caching
- // the TemplateURL for use on another keyword.
- url.EnsureKeyword();
- GenericRequest<TemplateURL>* request =
- new GenericRequest<TemplateURL>(this, GetNextRequestHandle(), NULL, url);
+void WebDataService::AddKeyword(const TemplateURLData& data) {
+ GenericRequest<TemplateURLData>* request =
+ new GenericRequest<TemplateURLData>(this, GetNextRequestHandle(), NULL,
+ data);
RegisterRequest(request);
ScheduleTask(FROM_HERE, Bind(&WebDataService::AddKeywordImpl, this, request));
}
@@ -165,12 +163,10 @@ void WebDataService::RemoveKeyword(TemplateURLID id) {
Bind(&WebDataService::RemoveKeywordImpl, this, request));
}
-void WebDataService::UpdateKeyword(const TemplateURL& url) {
- // Ensure that the keyword is already generated (and cached) before caching
- // the TemplateURL for use on another keyword.
- url.EnsureKeyword();
- GenericRequest<TemplateURL>* request =
- new GenericRequest<TemplateURL>(this, GetNextRequestHandle(), NULL, url);
+void WebDataService::UpdateKeyword(const TemplateURLData& data) {
+ GenericRequest<TemplateURLData>* request =
+ new GenericRequest<TemplateURLData>(this, GetNextRequestHandle(), NULL,
+ data);
RegisterRequest(request);
ScheduleTask(FROM_HERE,
Bind(&WebDataService::UpdateKeywordImpl, this, request));
@@ -739,7 +735,7 @@ int WebDataService::GetNextRequestHandle() {
//
////////////////////////////////////////////////////////////////////////////////
-void WebDataService::AddKeywordImpl(GenericRequest<TemplateURL>* request) {
+void WebDataService::AddKeywordImpl(GenericRequest<TemplateURLData>* request) {
InitializeDatabaseIfNecessary();
if (db_ && !request->IsCancelled(NULL)) {
db_->GetKeywordTable()->AddKeyword(request->arg());
@@ -758,7 +754,8 @@ void WebDataService::RemoveKeywordImpl(GenericRequest<TemplateURLID>* request) {
request->RequestComplete();
}
-void WebDataService::UpdateKeywordImpl(GenericRequest<TemplateURL>* request) {
+void WebDataService::UpdateKeywordImpl(
+ GenericRequest<TemplateURLData>* request) {
InitializeDatabaseIfNecessary();
if (db_ && !request->IsCancelled(NULL)) {
if (!db_->GetKeywordTable()->UpdateKeyword(request->arg())) {
diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h
index b4a6630..aea638a 100644
--- a/chrome/browser/webdata/web_data_service.h
+++ b/chrome/browser/webdata/web_data_service.h
@@ -335,9 +335,9 @@ class WebDataService
// the caller (TemplateURLService) does not need to know when the request is
// done.
- void AddKeyword(const TemplateURL& url);
+ void AddKeyword(const TemplateURLData& data);
void RemoveKeyword(TemplateURLID id);
- void UpdateKeyword(const TemplateURL& url);
+ void UpdateKeyword(const TemplateURLData& data);
// Fetches the keywords.
// On success, consumer is notified with WDResult<KeywordTable::Keywords>.
@@ -597,9 +597,9 @@ class WebDataService
// Keywords.
//
//////////////////////////////////////////////////////////////////////////////
- void AddKeywordImpl(GenericRequest<TemplateURL>* request);
+ void AddKeywordImpl(GenericRequest<TemplateURLData>* request);
void RemoveKeywordImpl(GenericRequest<TemplateURLID>* request);
- void UpdateKeywordImpl(GenericRequest<TemplateURL>* request);
+ void UpdateKeywordImpl(GenericRequest<TemplateURLData>* request);
void GetKeywordsImpl(WebDataRequest* request);
void SetDefaultSearchProviderImpl(GenericRequest<TemplateURLID>* r);
void SetBuiltinKeywordVersionImpl(GenericRequest<int>* r);
diff --git a/chrome/browser/webdata/web_database.cc b/chrome/browser/webdata/web_database.cc
index 828acd4..48e9262 100644
--- a/chrome/browser/webdata/web_database.cc
+++ b/chrome/browser/webdata/web_database.cc
@@ -21,11 +21,11 @@
// corresponding changes must happen in the unit tests, and new migration test
// added. See |WebDatabaseMigrationTest::kCurrentTestedVersionNumber|.
// static
-const int WebDatabase::kCurrentVersionNumber = 44;
+const int WebDatabase::kCurrentVersionNumber = 45;
namespace {
-const int kCompatibleVersionNumber = 44;
+const int kCompatibleVersionNumber = 45;
// Change the version number and possibly the compatibility version of
// |meta_table_|.
@@ -321,6 +321,14 @@ sql::InitStatus WebDatabase::MigrateOldVersionsAsNeeded() {
ChangeVersion(&meta_table_, 44, true);
// FALL THROUGH
+ case 44:
+ if (!keyword_table_->
+ MigrateToVersion45RemoveLogoIDAndAutogenerateColumns())
+ return FailedMigrationTo(45);
+
+ ChangeVersion(&meta_table_, 45, true);
+ // FALL THROUGH
+
// Add successive versions here. Each should set the version number and
// compatible version number as appropriate, then fall through to the next
// case.
diff --git a/chrome/browser/webdata/web_database_migration_unittest.cc b/chrome/browser/webdata/web_database_migration_unittest.cc
index 638c72c..7800579 100644
--- a/chrome/browser/webdata/web_database_migration_unittest.cc
+++ b/chrome/browser/webdata/web_database_migration_unittest.cc
@@ -194,7 +194,7 @@ class WebDatabaseMigrationTest : public testing::Test {
DISALLOW_COPY_AND_ASSIGN(WebDatabaseMigrationTest);
};
-const int WebDatabaseMigrationTest::kCurrentTestedVersionNumber = 44;
+const int WebDatabaseMigrationTest::kCurrentTestedVersionNumber = 45;
void WebDatabaseMigrationTest::LoadDatabase(const FilePath::StringType& file) {
std::string contents;
@@ -303,7 +303,6 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion22CorruptedToCurrent) {
ASSERT_TRUE(
connection.DoesColumnExist("credit_cards", "card_number_encrypted"));
ASSERT_TRUE(connection.DoesColumnExist("keywords", "id"));
- ASSERT_FALSE(connection.DoesColumnExist("keywords", "logo_id"));
}
// Load the database via the WebDatabase class and migrate the database to
@@ -329,47 +328,6 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion22CorruptedToCurrent) {
EXPECT_TRUE(
connection.DoesColumnExist("credit_cards", "card_number_encrypted"));
EXPECT_TRUE(connection.DoesColumnExist("keywords", "id"));
- EXPECT_TRUE(connection.DoesColumnExist("keywords", "logo_id"));
- }
-}
-
-// Tests that the |keywords| |logo_id| column gets added to the schema for a
-// version 24 database.
-TEST_F(WebDatabaseMigrationTest, MigrateVersion24ToCurrent) {
- // This schema is taken from a build prior to the addition of the |keywords|
- // |logo_id| column.
- ASSERT_NO_FATAL_FAILURE(LoadDatabase(FILE_PATH_LITERAL("version_24.sql")));
-
- // Verify pre-conditions. These are expectations for version 24 of the
- // database.
- {
- sql::Connection connection;
- ASSERT_TRUE(connection.Open(GetDatabasePath()));
-
- // Columns existing and not existing before current version.
- ASSERT_TRUE(connection.DoesColumnExist("keywords", "id"));
- ASSERT_FALSE(connection.DoesColumnExist("keywords", "logo_id"));
- }
-
- // Load the database via the WebDatabase class and migrate the database to
- // the current version.
- {
- WebDatabase db;
- ASSERT_EQ(sql::INIT_OK, db.Init(GetDatabasePath()));
- }
-
- // Verify post-conditions. These are expectations for current version of the
- // database.
- {
- sql::Connection connection;
- ASSERT_TRUE(connection.Open(GetDatabasePath()));
-
- // Check version.
- EXPECT_EQ(kCurrentTestedVersionNumber, VersionFromConnection(&connection));
-
- // |keywords| |logo_id| column should have been added.
- EXPECT_TRUE(connection.DoesColumnExist("keywords", "id"));
- EXPECT_TRUE(connection.DoesColumnExist("keywords", "logo_id"));
}
}
@@ -618,12 +576,10 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion27ToCurrent) {
EXPECT_EQ(std::string("{google:baseSuggestURL}search?client=chrome&hl="
"{language}&q={searchTerms}"), s2.ColumnString(11));
EXPECT_EQ(1, s2.ColumnInt(12));
- EXPECT_TRUE(s2.ColumnBool(13));
- EXPECT_EQ(6245, s2.ColumnInt(14));
- EXPECT_FALSE(s2.ColumnBool(15));
+ EXPECT_EQ(false, s2.ColumnBool(13));
+ EXPECT_EQ(std::string(), s2.ColumnString(14));
+ EXPECT_EQ(0, s2.ColumnInt(15));
EXPECT_EQ(std::string(), s2.ColumnString(16));
- EXPECT_EQ(0, s2.ColumnInt(17));
- EXPECT_EQ(std::string(), s2.ColumnString(18));
}
}
@@ -1845,14 +1801,12 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion42ToCurrent) {
EXPECT_EQ("{google:baseSuggestURL}search?client=chrome&hl={language}&"
"q={searchTerms}", s.ColumnString(11));
EXPECT_EQ(1, s.ColumnInt(12));
- EXPECT_TRUE(s.ColumnBool(13));
- EXPECT_EQ(6262, s.ColumnInt(14));
- EXPECT_FALSE(s.ColumnBool(15));
+ EXPECT_EQ(false, s.ColumnBool(13));
EXPECT_EQ("{google:baseURL}webhp?{google:RLZ}sourceid=chrome-instant&"
"ie={inputEncoding}&ion=1{searchTerms}&nord=1",
- s.ColumnString(16));
- EXPECT_EQ(0, s.ColumnInt(17));
- EXPECT_EQ("{1234-5678-90AB-CDEF}", s.ColumnString(18));
+ s.ColumnString(14));
+ EXPECT_EQ(0, s.ColumnInt(15));
+ EXPECT_EQ("{1234-5678-90AB-CDEF}", s.ColumnString(16));
EXPECT_FALSE(s.Step());
}
@@ -1936,3 +1890,133 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion43ToCurrent) {
EXPECT_FALSE(default_search_provider_id_backup_signature.empty());
}
}
+
+#if !defined(GOOGLE_CHROME_BUILD)
+// Tests that the |autogenerate_keyword| and |logo_id| columns get removed from
+// the keyword table schema for a version 45 database.
+//
+// This is enabled on Chromium only because a valid signature is required for
+// this test, which makes it key-dependent.
+TEST_F(WebDatabaseMigrationTest, MigrateVersion44ToCurrent) {
+ ASSERT_NO_FATAL_FAILURE(LoadDatabase(FILE_PATH_LITERAL("version_44.sql")));
+
+ // Verify pre-conditions. These are expectations for version 44 of the
+ // database.
+ {
+ sql::Connection connection;
+ ASSERT_TRUE(connection.Open(GetDatabasePath()));
+ ASSERT_TRUE(sql::MetaTable::DoesTableExist(&connection));
+
+ sql::MetaTable meta_table;
+ ASSERT_TRUE(meta_table.Init(&connection, 44, 44));
+
+ ASSERT_TRUE(connection.DoesColumnExist("keywords", "autogenerate_keyword"));
+ ASSERT_TRUE(connection.DoesColumnExist("keywords", "logo_id"));
+ }
+
+ // Load the database via the WebDatabase class and migrate the database to
+ // the current version.
+ {
+ WebDatabase db;
+ ASSERT_EQ(sql::INIT_OK, db.Init(GetDatabasePath()));
+ ASSERT_FALSE(db.GetKeywordTable()->DidDefaultSearchProviderChange());
+
+ // The backup table should match the keyword table.
+ std::string keywords_contents;
+ EXPECT_TRUE(db.GetKeywordTable()->GetTableContents("keywords",
+ kCurrentTestedVersionNumber, &keywords_contents));
+ std::string keywords_backup_contents;
+ EXPECT_TRUE(db.GetKeywordTable()->GetTableContents("keywords_backup",
+ kCurrentTestedVersionNumber, &keywords_backup_contents));
+ EXPECT_EQ(keywords_contents, keywords_backup_contents);
+ }
+
+ // Verify post-conditions. These are expectations for current version of the
+ // database.
+ {
+ sql::Connection connection;
+ ASSERT_TRUE(connection.Open(GetDatabasePath()));
+ ASSERT_TRUE(sql::MetaTable::DoesTableExist(&connection));
+
+ // Check version.
+ EXPECT_EQ(kCurrentTestedVersionNumber, VersionFromConnection(&connection));
+
+ sql::MetaTable meta_table;
+ ASSERT_TRUE(meta_table.Init(&connection, kCurrentTestedVersionNumber,
+ kCurrentTestedVersionNumber));
+
+ // We should have removed this obsolete key.
+ std::string default_search_provider_backup;
+ EXPECT_FALSE(meta_table.GetValue("Default Search Provider Backup",
+ &default_search_provider_backup));
+
+ // Two columns should have been removed.
+ EXPECT_FALSE(connection.DoesColumnExist("keywords",
+ "autogenerate_keyword"));
+ EXPECT_FALSE(connection.DoesColumnExist("keywords", "logo_id"));
+ }
+}
+#endif // !defined(GOOGLE_CHROME_BUILD)
+
+// Like MigrateVersion44ToCurrent above, but with a corrupt backup signature.
+// This should result in us dropping the backup table but successfully migrating
+// otherwise.
+//
+// Because this test doesn't rely on a valid signature, we can run it on
+// official builds as well.
+TEST_F(WebDatabaseMigrationTest, MigrateVersion44CorruptBackupToCurrent) {
+ ASSERT_NO_FATAL_FAILURE(
+ LoadDatabase(FILE_PATH_LITERAL("version_44_backup_corrupt.sql")));
+
+ // Verify pre-conditions. These are expectations for version 44 of the
+ // database.
+ {
+ sql::Connection connection;
+ ASSERT_TRUE(connection.Open(GetDatabasePath()));
+ ASSERT_TRUE(sql::MetaTable::DoesTableExist(&connection));
+
+ sql::MetaTable meta_table;
+ ASSERT_TRUE(meta_table.Init(&connection, 44, 44));
+
+ ASSERT_TRUE(connection.DoesColumnExist("keywords", "autogenerate_keyword"));
+ ASSERT_TRUE(connection.DoesColumnExist("keywords", "logo_id"));
+ }
+
+ // Load the database via the WebDatabase class and migrate the database to
+ // the current version.
+ {
+ WebDatabase db;
+ ASSERT_EQ(sql::INIT_OK, db.Init(GetDatabasePath()));
+ // We should detect a "search provider change" as a side effect of dropping
+ // the backup table.
+ ASSERT_TRUE(db.GetKeywordTable()->DidDefaultSearchProviderChange());
+ }
+
+ // Verify post-conditions. These are expectations for current version of the
+ // database.
+ {
+ sql::Connection connection;
+ ASSERT_TRUE(connection.Open(GetDatabasePath()));
+ ASSERT_TRUE(sql::MetaTable::DoesTableExist(&connection));
+
+ // Check version.
+ EXPECT_EQ(kCurrentTestedVersionNumber, VersionFromConnection(&connection));
+
+ sql::MetaTable meta_table;
+ ASSERT_TRUE(meta_table.Init(&connection, kCurrentTestedVersionNumber,
+ kCurrentTestedVersionNumber));
+
+ // We should have removed this obsolete key.
+ std::string default_search_provider_backup;
+ EXPECT_FALSE(meta_table.GetValue("Default Search Provider Backup",
+ &default_search_provider_backup));
+
+ // Two columns should have been removed.
+ EXPECT_FALSE(connection.DoesColumnExist("keywords",
+ "autogenerate_keyword"));
+ EXPECT_FALSE(connection.DoesColumnExist("keywords", "logo_id"));
+
+ // The backup table should be gone.
+ EXPECT_FALSE(connection.DoesTableExist("keywords_backup"));
+ }
+}