Bug 715268 - Downgrades may cause missing favicons GUIDs.

r=dietrich

--HG--
rename : toolkit/components/places/tests/migration/places_v10_from_v11.sqlite => toolkit/components/places/tests/migration/places_v10_from_v14.sqlite
rename : toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v11.js => toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v14.js

toolkit/components/places/Database.cpp

@@ -728,7 +728,12 @@ Database::InitSchema(bool* aDatabaseMigrated)
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
-      // Firefox 11 uses schema version 14.
+      if (currentSchemaVersion < 16) {
+        rv = MigrateV16Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 11 uses schema version 16.
 
       // Schema Upgrades must add migration code here.
 
@@ -1545,6 +1550,8 @@ Database::MigrateV13Up()
 nsresult
 Database::MigrateV14Up()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   // For existing profiles, we may not have a moz_favicons.guid column.
   // Add it here. We want it to be unique, but ALTER TABLE doesn't allow
   // a uniqueness constraint, so the index must be created separately.
@@ -1560,17 +1567,18 @@ Database::MigrateV14Up()
     ));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Generate GUIDs for our existing favicons.
-    rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
-      "UPDATE moz_favicons "
-      "SET guid = GENERATE_GUID()"
-    ));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    // And now we can make the column unique.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_FAVICONS_GUID);
     NS_ENSURE_SUCCESS(rv, rv);
   }
+
+  // Generate GUID for any favicon missing it.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_favicons "
+    "SET guid = GENERATE_GUID() "
+    "WHERE guid ISNULL "
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   return NS_OK;
 }
 
@@ -1600,6 +1608,23 @@ Database::MigrateV15Up()
   return NS_OK;
 }
 
+nsresult
+Database::MigrateV16Up()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Due to Bug 715268 downgraded and then upgraded profiles may lack favicons
+  // guids, so fillup any missing ones.
+  nsresult rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_favicons "
+    "SET guid = GENERATE_GUID() "
+    "WHERE guid ISNULL "
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 void
 Database::Shutdown()
 {
@@ -1632,7 +1657,7 @@ Database::Observe(nsISupports *aSubject,
                   const char *aTopic,
                   const PRUnichar *aData)
 {
-  NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
+  MOZ_ASSERT(NS_IsMainThread());
  
   if (strcmp(aTopic, TOPIC_PROFILE_CHANGE_TEARDOWN) == 0) {
     // Tests simulating shutdown may cause multiple notifications.
@@ -1678,27 +1703,38 @@ Database::Observe(nsISupports *aSubject,
 
 #ifdef DEBUG
     { // Sanity check for missing guids.
+      bool haveNullGuids = false;
       nsCOMPtr<mozIStorageStatement> stmt;
+
       nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
         "SELECT 1 "
         "FROM moz_places "
         "WHERE guid IS NULL "
-        "UNION ALL "
+      ), getter_AddRefs(stmt));
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->ExecuteStep(&haveNullGuids);
+      NS_ENSURE_SUCCESS(rv, rv);
+      MOZ_ASSERT(!haveNullGuids && "Found a page without a GUID!");
+
+      rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
         "SELECT 1 "
         "FROM moz_bookmarks "
         "WHERE guid IS NULL "
-        "UNION ALL "
+      ), getter_AddRefs(stmt));
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->ExecuteStep(&haveNullGuids);
+      NS_ENSURE_SUCCESS(rv, rv);
+      MOZ_ASSERT(!haveNullGuids && "Found a bookmark without a GUID!");
+
+      rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
         "SELECT 1 "
         "FROM moz_favicons "
         "WHERE guid IS NULL "
       ), getter_AddRefs(stmt));
       NS_ENSURE_SUCCESS(rv, rv);
-
-      bool haveNullGuids;
       rv = stmt->ExecuteStep(&haveNullGuids);
       NS_ENSURE_SUCCESS(rv, rv);
-      NS_ASSERTION(!haveNullGuids,
-                   "Someone added an entry without adding a GUID!");
+      MOZ_ASSERT(!haveNullGuids && "Found a favicon without a GUID!");
     }
 #endif
 

toolkit/components/places/Database.h

@@ -47,7 +47,7 @@
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 15
+#define DATABASE_SCHEMA_VERSION 16
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
@@ -298,6 +298,7 @@ protected:
   nsresult MigrateV13Up();
   nsresult MigrateV14Up();
   nsresult MigrateV15Up();
+  nsresult MigrateV16Up();
 
   nsresult UpdateBookmarkRootTitles();
   nsresult CheckAndUpdateGUIDs();

toolkit/components/places/tests/head_common.js

@@ -35,7 +35,7 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
-const CURRENT_SCHEMA_VERSION = 15;
+const CURRENT_SCHEMA_VERSION = 16;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
 const NS_APP_PROFILE_DIR_STARTUP = "ProfDS";

toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v14.js

@@ -2,7 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * This file tests migration invariants from a database with schema version 11
+ * This file tests migration invariants from a database with schema version 14
  * that was then downgraded to a database with a schema version 10.  Places
  * should then migrate this database to one with the current schema version.
  */
@@ -127,6 +127,6 @@ function test_final_state()
 
 function run_test()
 {
-  setPlacesDatabase("places_v10_from_v11.sqlite");
+  setPlacesDatabase("places_v10_from_v14.sqlite");
   run_next_test();
 }

toolkit/components/places/tests/migration/xpcshell.ini

@@ -3,7 +3,7 @@ head = head_migration.js
 tail = 
 
 [test_current_from_v10.js]
-[test_current_from_v10_migrated_from_v11.js]
+[test_current_from_v10_migrated_from_v14.js]
 [test_database_from_alpha.js]
 [test_database_from_v6_no_frecency.js]
 [test_database_from_v6_no_indices.js]