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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/webdata/keyword_table.cc

Issue 10381016: Remove the "autogenerate keyword" bit on TemplateURL. (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: Created 8 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/browser/webdata/keyword_table.h ('K') | « chrome/browser/webdata/keyword_table.h ('k') | chrome/browser/webdata/keyword_table_unittest.cc » ('j') | sync/protocol/search_engine_specifics.proto » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/webdata/keyword_table.cc
===================================================================
--- chrome/browser/webdata/keyword_table.cc (revision 135424)
+++ chrome/browser/webdata/keyword_table.cc (working copy)
@@ -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::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 @@
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 @@
"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 @@
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 @@
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 @@
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 @@
}
bool KeywordTable::GetDefaultSearchProviderBackup(TemplateURLData* backup) {
- if (!IsBackupSignatureValid())
+ if (!IsBackupSignatureValid(WebDatabase::kCurrentVersionNumber))
return false;
int64 backup_id = kInvalidTemplateURLID;
@@ -212,7 +226,7 @@
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::DidDefaultSearchProviderChange() {
- if (!IsBackupSignatureValid()) {
+ if (!IsBackupSignatureValid(WebDatabase::kCurrentVersionNumber)) {
UMA_HISTOGRAM_ENUMERATION(
protector::kProtectorHistogramDefaultSearchProvider,
protector::kProtectorErrorBackupInvalid,
@@ -340,16 +354,43 @@
}
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
bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s,
TemplateURLData* data) {
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 @@
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 @@
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 @@
}
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::GetTableContents(const char* table_name,
+ int table_version,
std::string* contents) {
DCHECK(contents);
@@ -400,16 +443,19 @@
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 @@
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 @@
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 @@
*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());
+}
« chrome/browser/webdata/keyword_table.h ('K') | « chrome/browser/webdata/keyword_table.h ('k') | chrome/browser/webdata/keyword_table_unittest.cc » ('j') | sync/protocol/search_engine_specifics.proto » ('J')

Powered by Google App Engine
This is Rietveld 408576698