Remove the protector service.

Remove the protector service.


BUG=165008
NOTRY=true


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173169

[Ivan Korotkov, 2012-12-10 09:21:07]

PTAL: (fear not, the CL looks huge but it's mostly deletions)

Scott: the whole CL
Peter: webdata and keywords_table
Nirnimesh: test/ and automation/
Mattias: prefs/

[Mattias Nissler (ping if slow), 2012-12-10 10:01:50]

LGTM. I take it protector is dead?

[Ivan Korotkov, 2012-12-10 10:25:27]

Yup.

[sky, 2012-12-10 15:46:19]

LGTM

https://codereview.chromium.org/11493003/diff/8012/chrome/browser/webdata/key...
File chrome/browser/webdata/keyword_table.cc (right):

https://codereview.chromium.org/11493003/diff/8012/chrome/browser/webdata/key...
chrome/browser/webdata/keyword_table.cc:16: // #include "base/stringprintf.h"
If not needed, remove.

[dennis_jeffrey, 2012-12-10 17:48:06]

automation/ and test/ LGTM

fyi, Nirnimesh has moved to another project.

[Ivan Korotkov, 2012-12-10 18:25:24]

https://codereview.chromium.org/11493003/diff/8012/chrome/browser/webdata/key...
File chrome/browser/webdata/keyword_table.cc (right):

https://codereview.chromium.org/11493003/diff/8012/chrome/browser/webdata/key...
chrome/browser/webdata/keyword_table.cc:16: // #include "base/stringprintf.h"
On 2012/12/10 15:46:20, sky wrote:
> If not needed, remove.

Done.

[Peter Kasting, 2012-12-11 20:09:06]

LGTMq

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/search_eng...
File chrome/browser/search_engines/search_engine_type.h (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/search_eng...
chrome/browser/search_engines/search_engine_type.h:47: // engines below.
Nit: Just copy the comment from above.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:358: // TODO(ivankr):
remove 'backup' dictionary from Preferences.
Can you just do this now?  If not, I wouldn't put this comment here, as it
doesn't really have any context here; just file a bug.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
File chrome/browser/webdata/keyword_table.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
chrome/browser/webdata/keyword_table.cc:294: default_search_id)) {
Nit: File style: No {} when body is one line (several places)

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
chrome/browser/webdata/keyword_table.cc:328: "")) {
Nit: std::string() (2 palces)

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database.cc:161: // Some malware tried to force
protector to re-sign things by lowering the
Nit: In the future, people won't know what protector is, so I'd just say "Some
malware used to lower the version number, causing migration to fail. ..."

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_migration_unittest.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_migration_unittest.cc:128: void
CheckHasBackupData(sql::MetaTable* meta_table) {
Nit: Should we replace these two functions with one function that takes a bool
as to whether the backup data exists?

This would also presumably correct inconsistent-looking details like how this
function doesn't check for keywords_backup to exist but the other function does?

[Ivan Korotkov, 2012-12-12 17:02:57]

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/search_eng...
File chrome/browser/search_engines/search_engine_type.h (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/search_eng...
chrome/browser/search_engines/search_engine_type.h:47: // engines below.
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Nit: Just copy the comment from above.

Done.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:358: // TODO(ivankr):
remove 'backup' dictionary from Preferences.
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Can you just do this now?  If not, I wouldn't put this comment here, as it
> doesn't really have any context here; just file a bug.

Right, I left that TODO for myself and forgot about it. Done.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
File chrome/browser/webdata/keyword_table.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
chrome/browser/webdata/keyword_table.cc:294: default_search_id)) {
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Nit: File style: No {} when body is one line (several places)

Done.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/ke...
chrome/browser/webdata/keyword_table.cc:328: "")) {
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Nit: std::string() (2 palces)

Done.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database.cc:161: // Some malware tried to force
protector to re-sign things by lowering the
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Nit: In the future, people won't know what protector is, so I'd just say "Some
> malware used to lower the version number, causing migration to fail. ..."

Done.

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
File chrome/browser/webdata/web_database_migration_unittest.cc (right):

https://codereview.chromium.org/11493003/diff/18001/chrome/browser/webdata/we...
chrome/browser/webdata/web_database_migration_unittest.cc:128: void
CheckHasBackupData(sql::MetaTable* meta_table) {
On 2012/12/11 20:09:06, Peter Kasting wrote:
> Nit: Should we replace these two functions with one function that takes a bool
> as to whether the backup data exists?
> 
> This would also presumably correct inconsistent-looking details like how this
> function doesn't check for keywords_backup to exist but the other function
does?

It's actually intentional that I didn't check for keywords_backup in
CheckHasBackupData because it wasn't there in most DB versions. I've added a
separate check to versions where it ought to be.
So I'd rather keep this as 2 separate functions if you don't insist.

[commit-bot: I haz the power, 2012-12-13 10:36:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11493003/29001

[commit-bot: I haz the power, 2012-12-13 10:38:11]

Failed to apply patch for chrome/test/data/web_database/version_47.sql:
File exist but was about to be overwriten
Patch:   NR 
chrome/test/data/web_database/version_46_backup_corrupt.sql->chrome/test/data/web_database/version_47.sql
Index: chrome/test/data/web_database/version_47.sql
diff --git a/chrome/test/data/web_database/version_46_backup_corrupt.sql
b/chrome/test/data/web_database/version_47.sql
similarity index 66%
rename from chrome/test/data/web_database/version_46_backup_corrupt.sql
rename to chrome/test/data/web_database/version_47.sql
index
8dcf7cb25ef771f4138ee46f5f3805aa05cff6ef..aae06e8e9b7ca63f185a7032352bbc21e410e6ed
100644
--- a/chrome/test/data/web_database/version_46_backup_corrupt.sql
+++ b/chrome/test/data/web_database/version_47.sql
@@ -1,12 +1,33 @@
 PRAGMA foreign_keys=OFF;
 BEGIN TRANSACTION;
 CREATE TABLE meta(key LONGVARCHAR NOT NULL UNIQUE PRIMARY KEY, value
LONGVARCHAR);
-INSERT INTO "meta" VALUES('Default Search Provider ID','0');
-INSERT INTO "meta" VALUES('version','46');
-INSERT INTO "meta" VALUES('last_compatible_version','46');
-INSERT INTO "meta" VALUES('Default Search Provider ID Backup','0');
+INSERT INTO "meta" VALUES('version','47');
+INSERT INTO "meta" VALUES('last_compatible_version','47');
+INSERT INTO "meta" VALUES('Builtin Keyword Version','46');
+INSERT INTO "meta" VALUES('Default Search Provider ID','2');
+INSERT INTO "meta" VALUES('Default Search Provider ID Backup','2');
 INSERT INTO "meta" VALUES('Default Search Provider ID Backup Signature','');
-INSERT INTO "meta" VALUES('Builtin Keyword Version','39');
+CREATE TABLE keywords (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,alternate_urls VARCHAR);
+CREATE TABLE keywords_backup(
+  id INT,
+  short_name TEXT,
+  keyword TEXT,
+  favicon_url TEXT,
+  url TEXT,
+  safe_for_autoreplace INT,
+  originating_url TEXT,
+  date_created INT,
+  usage_count INT,
+  input_encodings TEXT,
+  show_in_default_list INT,
+  suggest_url TEXT,
+  prepopulate_id INT,
+  created_by_policy INT,
+  instant_url TEXT,
+  last_modified INT,
+  sync_guid TEXT,
+  alternate_urls TEXT
+);
 CREATE TABLE autofill (name VARCHAR, value VARCHAR, value_lower VARCHAR,
pair_id INTEGER PRIMARY KEY, count INTEGER DEFAULT 1);
 CREATE TABLE credit_cards ( guid VARCHAR PRIMARY KEY, name_on_card VARCHAR,
expiration_month INTEGER, expiration_year INTEGER, card_number_encrypted BLOB,
date_modified INTEGER NOT NULL DEFAULT 0);
 CREATE TABLE autofill_dates ( pair_id INTEGER DEFAULT 0, date_created INTEGER
DEFAULT 0);
@@ -18,10 +39,8 @@ CREATE TABLE autofill_profiles_trash ( guid VARCHAR);
 CREATE TABLE web_app_icons (url LONGVARCHAR,width int,height int,image BLOB,
UNIQUE (url, width, height));
 CREATE TABLE web_apps (url LONGVARCHAR UNIQUE,has_all_images INTEGER NOT NULL);
 CREATE TABLE token_service (service VARCHAR PRIMARY KEY NOT
NULL,encrypted_token BLOB);
-CREATE TABLE web_intents (service_url LONGVARCHAR,action VARCHAR,type
VARCHAR,title LONGVARCHAR,disposition VARCHAR,scheme VARCHAR,UNIQUE
(service_url, action, type));
-CREATE TABLE web_intents_defaults (action VARCHAR,type VARCHAR,url_pattern
LONGVARCHAR,user_date INTEGER,suppression INTEGER,service_url LONGVARCHAR,scheme
VARCHAR,UNIQUE (action, type, url_pattern));
-CREATE TABLE keywords (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);
-CREATE TABLE keywords_backup(id INT,short_name TEXT,keyword TEXT,favicon_url
TEXT,url TEXT,safe_for_autoreplace INT,originating_url TEXT,date_created
INT,usage_count INT,input_encodings TEXT,show_in_default_list INT,suggest_url
TEXT,prepopulate_id INT,created_by_policy INT,instant_url TEXT,last_modified
INT,sync_guid TEXT);
+CREATE TABLE web_intents ( service_url LONGVARCHAR, action VARCHAR, type
VARCHAR, title LONGVARCHAR, disposition VARCHAR, scheme VARCHAR, UNIQUE
(service_url, action, scheme, type));
+CREATE TABLE web_intents_defaults ( action VARCHAR, type VARCHAR, url_pattern
LONGVARCHAR, user_date INTEGER, suppression INTEGER, service_url LONGVARCHAR,
scheme VARCHAR, UNIQUE (action, scheme, type, url_pattern));
 CREATE INDEX autofill_name ON autofill (name);
 CREATE INDEX autofill_name_value_lower ON autofill (name, value_lower);
 CREATE INDEX autofill_dates_pair_id ON autofill_dates (pair_id);

[commit-bot: I haz the power, 2012-12-13 16:37:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11493003/33001

[commit-bot: I haz the power, 2012-12-13 17:22:50]

Retried try job too often on linux_rel for step(s) interactive_ui_tests,
unit_tests

[commit-bot: I haz the power, 2012-12-13 20:46:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11493003/33001

[commit-bot: I haz the power, 2012-12-13 21:32:43]

Retried try job too often on linux_rel for step(s) interactive_ui_tests,
unit_tests

[commit-bot: I haz the power, 2012-12-14 10:41:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11493003/42002

[commit-bot: I haz the power, 2012-12-14 16:20:43]

Retried try job too often on win_rel for step(s) browser_tests

[Ivan Korotkov, 2012-12-14 16:57:58]

Looks like a repeated failure of a couple of flaky tests on win_rel

[commit-bot: I haz the power, 2012-12-14 16:59:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ivankr@chromium.org/11493003/42002

[commit-bot: I haz the power, 2012-12-14 17:01:01]

Change committed as 173169