From a658c410cf4da1fba0005d0a659478d87d1e95d1 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Thu, 5 Jan 2012 11:43:29 +0100 Subject: [PATCH] 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 | 68 +++++++++++++----- toolkit/components/places/Database.h | 3 +- .../components/places/tests/head_common.js | 2 +- ..._v11.sqlite => places_v10_from_v14.sqlite} | Bin 1081344 -> 1114112 bytes ...est_current_from_v10_migrated_from_v14.js} | 4 +- .../places/tests/migration/xpcshell.ini | 2 +- 6 files changed, 58 insertions(+), 21 deletions(-) rename toolkit/components/places/tests/migration/{places_v10_from_v11.sqlite => places_v10_from_v14.sqlite} (97%) rename toolkit/components/places/tests/migration/{test_current_from_v10_migrated_from_v11.js => test_current_from_v10_migrated_from_v14.js} (98%) diff --git a/toolkit/components/places/Database.cpp b/toolkit/components/places/Database.cpp index 72cbb8b989bb..06b00c36369f 100644 --- a/toolkit/components/places/Database.cpp +++ b/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 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 diff --git a/toolkit/components/places/Database.h b/toolkit/components/places/Database.h index 20da36e1c05d..82df715c4422 100644 --- a/toolkit/components/places/Database.h +++ b/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(); diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 24d671f6ee23..0b0d6d082370 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/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"; diff --git a/toolkit/components/places/tests/migration/places_v10_from_v11.sqlite b/toolkit/components/places/tests/migration/places_v10_from_v14.sqlite similarity index 97% rename from toolkit/components/places/tests/migration/places_v10_from_v11.sqlite rename to toolkit/components/places/tests/migration/places_v10_from_v14.sqlite index e3f9ef4463fbfeb0edcd02a8592520bf2978ea42..810416564845b673f74c6c2587e803662c6edc99 100644 GIT binary patch delta 312 zcmZo@aB669nIJ97!oa|w1jH~P3uG`&G|=Z^VbFaf&s%VpK`A?XVz#kRc6MP)et+KI zJmtJ?IY)A23*|O$+?~bMs4vAXuByt|q&WF`!5khPh4j+Q6onAih!D-qvN;D>SSqC$ z94BtHwNFuE7uQu~Y-G;NOG&K&sfsVn%PcHS1@m+BtK!oV%QBPm^NN+6gIpa$Topq7 zJOe{r6+Hc1Tq6`FKdhDzg{e@`0O{8>mu3_9R-L@RM5cLJVf(T|Mj&PaVrC#_0b*7l zW&>h&Am#vKP9Ww2Vs0Sj0b*Vt<^y8>?aK-UqWHwPfPQDMzspenyZ&4K$ND$*&+8x6 U-`%(|Cwt?8Xp!l6|1&EC09$!(c>n+a delta 174 zcmZo@aA|08njkI8#K6Fy2*fZT4P-D*G|=Z^V$gjg$y+JKpqL{&G22+6;BH|{et+KI zJmtJ?IY)A2H*VaOHSxgK&9XTMShyNhq}at(RT-POCzoc=Vb?U5W)t^ToxHz9rg>Rm x`?5ktAZ7w$W*}w(Vpbq#17da{<^W<&Am##MZXo6XVqPHT17iN|%L)ae_yA3@Ix_$O diff --git a/toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v11.js b/toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v14.js similarity index 98% rename from toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v11.js rename to toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v14.js index 6386ecc9d4ac..038a94a08f81 100644 --- a/toolkit/components/places/tests/migration/test_current_from_v10_migrated_from_v11.js +++ b/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(); } diff --git a/toolkit/components/places/tests/migration/xpcshell.ini b/toolkit/components/places/tests/migration/xpcshell.ini index b38c198b0292..6722a8be386d 100644 --- a/toolkit/components/places/tests/migration/xpcshell.ini +++ b/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]