[sync] Add typed url sync metadata to the history db

components/history/core/browser/url_database.cc

Index: components/history/core/browser/url_database.cc
diff --git a/components/history/core/browser/url_database.cc b/components/history/core/browser/url_database.cc
index 37d2830b4a9f28e50d7056fedba6c94190764463..5a2acc5dd6ffdd6c63b7e110c01b3fffc14ea97e 100644
--- a/components/history/core/browser/url_database.cc
+++ b/components/history/core/browser/url_database.cc
@@ -568,26 +568,7 @@ bool URLDatabase::DropStarredIDFromURLs() {
   if (!GetDB().DoesColumnExist("urls", "starred_id"))
     return true;  // urls is already updated, no need to continue.
 
-  // Create a temporary table to contain the new URLs table.
-  if (!CreateTemporaryURLTable()) {
-    NOTREACHED();
-    return false;
-  }
-
-  // Copy the contents.
-  if (!GetDB().Execute(
-      "INSERT INTO temp_urls (id, url, title, visit_count, typed_count, "
-      "last_visit_time, hidden, favicon_id) "
-      "SELECT id, url, title, visit_count, typed_count, last_visit_time, "
-      "hidden, favicon_id FROM urls")) {
-    NOTREACHED() << GetDB().GetErrorMessage();
-    return false;
-  }
-
-  // Rename/commit the tmp table.
-  CommitTemporaryURLTable();
-
-  return true;
+  return RecreateURLTableWithAllContents();
 }
 
 bool URLDatabase::CreateURLTable(bool is_temporary) {
@@ -600,15 +581,15 @@ bool URLDatabase::CreateURLTable(bool is_temporary) {
   std::string sql;
   sql.append("CREATE TABLE ");
   sql.append(name);
-  sql.append("("
-      "id INTEGER PRIMARY KEY,"
+  sql.append(
+      "("
+      "id INTEGER PRIMARY KEY AUTOINCREMENT,"

[brettw, 2017/03/13 19:58:59]

I don't understand the bigger picture about why autoincrement is required here.
Can you explain? According to the sqlite page on this:
https://www.sqlite.org/autoinc.html
we shouldn't use autoincrement unless we want the specific behavior.

[Gang Wu, 2017/03/14 01:18:49]

For sync propose, we need to have an unique key to identify the URLs, using URL
as a key is too expensive since URL could be very long. ROWID is short, so we
prefer to use it. But since ROWID might be reused after deletion(without
AUTOINCREMENT), and if Sync not working somehow during deleting and reusing,
once Sync come back,  Sync would use ROWID and timestamp to see if there is an
update need to be synced. And sync will only send new URL to server, but missed
deleted URL. that is why we need to add AUTOINCREMENT here.  

       "url LONGVARCHAR,"
       "title LONGVARCHAR,"
       "visit_count INTEGER DEFAULT 0 NOT NULL,"
       "typed_count INTEGER DEFAULT 0 NOT NULL,"
       "last_visit_time INTEGER NOT NULL,"
-      "hidden INTEGER DEFAULT 0 NOT NULL,"
-      "favicon_id INTEGER DEFAULT 0 NOT NULL)"); // favicon_id is not used now.
+      "hidden INTEGER DEFAULT 0 NOT NULL)");
 
   return GetDB().Execute(sql.c_str());
 }
@@ -618,6 +599,29 @@ bool URLDatabase::CreateMainURLIndex() {
       "CREATE INDEX IF NOT EXISTS urls_url_index ON urls (url)");
 }
 
+bool URLDatabase::RecreateURLTableWithAllContents() {
+  // Create a temporary table to contain the new URLs table.
+  if (!CreateTemporaryURLTable()) {
+    NOTREACHED();
+    return false;
+  }
+
+  // Copy the contents.
+  if (!GetDB().Execute(
+          "INSERT INTO temp_urls (id, url, title, visit_count, typed_count, "
+          "last_visit_time, hidden) "
+          "SELECT id, url, title, visit_count, typed_count, last_visit_time, "
+          "hidden FROM urls")) {
+    NOTREACHED() << GetDB().GetErrorMessage();
+    return false;
+  }
+
+  // Rename/commit the tmp table.
+  CommitTemporaryURLTable();
+
+  return true;
+}
+
 const int kLowQualityMatchTypedLimit = 1;
 const int kLowQualityMatchVisitLimit = 4;
 const int kLowQualityMatchAgeLimitInDays = 3;