Migrate old Shortcuts DB data to conform to new type values

chrome/browser/history/shortcuts_database.cc

Index: chrome/browser/history/shortcuts_database.cc
diff --git a/chrome/browser/history/shortcuts_database.cc b/chrome/browser/history/shortcuts_database.cc
index 51d6f09293fbffe58da621bfab05911739d5a9d8..87781ba90a5eb8a3b499095a85a036dd8ea65e05 100644
--- a/chrome/browser/history/shortcuts_database.cc
+++ b/chrome/browser/history/shortcuts_database.cc
@@ -11,11 +11,18 @@
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
+#include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
 
 namespace {
 
+// Current version number. We write databases at the "current" version number,
+// but any previous version that can read the "compatible" one can make do with
+// or database without *too* many bad effects.
+const int kCurrentVersionNumber = 28;

[Anuj, 2014/02/10 22:18:44]

Not sure what version numbers I should use here.

[Peter Kasting, 2014/02/10 22:29:47]

If you didn't have a table before, use version 1 (for both).

+const int kCompatibleVersionNumber = 16;
+
 void BindShortcutToStatement(
     const history::ShortcutsBackend::Shortcut& shortcut,
     sql::Statement* s) {
@@ -157,6 +164,11 @@ void ShortcutsDatabase::LoadShortcuts(GuidToShortcutMap* shortcuts) {
   }
 }
 
+//static
+int ShortcutsDatabase::GetCurrentVersion() {

[Peter Kasting, 2014/02/10 22:29:47]

It doesn't seem like we need this as its own function, at least as this patch is
coded?

[Anuj, 2014/02/11 07:20:04]

Done.

+  return kCurrentVersionNumber;
+}
+
 ShortcutsDatabase::~ShortcutsDatabase() {
 }
 
@@ -196,6 +208,54 @@ bool ShortcutsDatabase::EnsureTable() {
         transaction.Commit();
   }
 
+  if (sql::MetaTable::DoesTableExist(&db_)) {

[Peter Kasting, 2014/02/10 22:29:47]

Don't you mean !exist?

[Anuj, 2014/02/11 07:20:04]

Done.

+    meta_table_.Init(&db_, GetCurrentVersion(), kCompatibleVersionNumber);
+    // With new suggestion types being added, following values have changed:

[Peter Kasting, 2014/02/10 22:29:47]

This comment is more confusing than helpful.  We didn't change the meaning of
SEARCH_OTHER_ENGINE to SEARCH_SUGGEST_ENTITY, we just renumbered the enum
values.  If you want to write a comment saying things like "SEARCH_OTHER_ENGINE
changed from 9 to 13", that'd be OK, but I think if we make the migration steps
simple, they'll become self-documenting so such a comment won't be necessary.

[Anuj, 2014/02/11 07:20:04]

Done.

+    // SEARCH_OTHER_ENGINE -> SEARCH_SUGGEST_ENTITY
+    // EXTENSION_APP -> SEARCH_SUGGEST_INFINITE
+    // CONTACT -> SEARCH_SUGGEST_PERSONALIZED
+    // BOOKMARK_TITLE -> SEARCH_SUGGEST_PROFILE
+    sql::Transaction transaction(&db_);
+    transaction.Begin();
+    // Migrate old SEARCH_OTHER_ENGINE values to the new type value.
+    // The ENTITY suggestion values always have a description and always have a
+    // https google search url with gs_ssp parameter.  The old
+    // SEARCH_OTHER_ENGINE entries will definitely fail the url check, but empty
+    // description test is also sufficient and faster.
+    db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+          "SET type = 13 " // New SEARCH_OTHER_ENGINE
+          "where type = 9 AND " // Old SEARCH_OTHER_ENGINE
+          "(description = '' OR "
+          "url NOT REGEXP '^https://www\\.google\\..*/search\\?.*&?gs_ssp=.*')")
+        .c_str()) &&
+      // Migrate old EXTENSION_APP values to the new type value.
+      // The INFINITE suggestions are not launched. So it is safe to update all
+      // of them to EXTENSION_APP type wherever the url is not a google search.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+            "SET type = 14 " // New EXTENSION_APP
+            "where type = 10 AND " // Old EXTENSION_APP
+            "url NOT REGEXP '^https://www\\.google\\..*/search\\?.*xssi=t.*')")
+          .c_str()) &&
+      // Migrate old CONTACT values to the new type value.
+      // The PERSONALIZED suggestions are a https google search url.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+            "SET type = 15 " // New CONTACT
+            "where type = 11 AND " // Old CONTACT
+            "url NOT REGEXP '^https://www\\.google\\..*/search\\?.*xssi=t.*')")
+          .c_str()) &&
+      // Migrate old BOOKMARK_TITLE values to the new type value.
+      // The PROFILE suggestion values always have a description and always have
+      // a https google search url with tbs parameter.  The old BOOKMARK_TITLE
+      // entries will definitely fail the url check, but empty description test
+      // is also sufficient and faster.
+      db_.Execute(base::StringPrintf("UPDATE omni_box_shortcuts "
+            "SET type = 16 " // New BOOKMARK_TITLE
+            "where type = 12 AND " // Old BOOKMARK_TITLE
+            "(description = '' OR "
+            "url NOT REGEXP '^https://www\\.google\\..*/search\\?.*&?tbs=.*')")
+          .c_str()) &&
+      transaction.Commit();
+  }
   return true;
 }