Bug 853388: Trigger XPI database conversion from SQLITE based on schema version preference

toolkit/mozapps/extensions/Makefile.in

@@ -15,7 +15,7 @@ endif
 
 # This is used in multiple places, so is defined here to avoid it getting
 # out of sync.
-DEFINES += -DMOZ_EXTENSIONS_DB_SCHEMA=14
+DEFINES += -DMOZ_EXTENSIONS_DB_SCHEMA=15
 
 # Additional debugging info is exposed in debug builds, or by setting the
 # MOZ_EM_DEBUG environment variable when building.

toolkit/mozapps/extensions/XPIProviderUtils.js

@@ -43,6 +43,8 @@ const FILE_XPI_ADDONS_LIST            = "extensions.ini";
 // The value for this is in Makefile.in
 #expand const DB_SCHEMA                       = __MOZ_EXTENSIONS_DB_SCHEMA__;
 
+// The last version of DB_SCHEMA implemented in SQLITE
+const LAST_SQLITE_DB_SCHEMA           = 14;
 const PREF_DB_SCHEMA                  = "extensions.databaseSchema";
 const PREF_PENDING_OPERATIONS         = "extensions.pendingOperations";
 const PREF_EM_ENABLED_ADDONS          = "extensions.enabledAddons";
@@ -469,26 +471,21 @@ this.XPIDatabase = {
    *              {location: {id1:{addon1}, id2:{addon2}}, location2:{...}, ...}
    *              if there is useful information
    */
-  loadSqliteData: function XPIDB_loadSqliteData() {
+  getMigrateDataFromSQLITE: function XPIDB_getMigrateDataFromSQLITE() {
     let connection = null;
     let dbfile = FileUtils.getFile(KEY_PROFILEDIR, [FILE_DATABASE], true);
-    if (!dbfile.exists()) {
-      return false;
-    }
     // Attempt to open the database
     try {
       connection = Services.storage.openUnsharedDatabase(dbfile);
     }
     catch (e) {
-      // exists but SQLITE can't open it
       WARN("Failed to open sqlite database " + dbfile.path + " for upgrade", e);
-      this.migrateData = null;
-      return true;
+      return null;
     }
     LOG("Migrating data from sqlite");
-    this.migrateData = this.getMigrateDataFromDatabase(connection);
+    let migrateData = this.getMigrateDataFromDatabase(connection);
     connection.close();
-    return true;
+    return migrateData;
   },
 
   /**
@@ -578,13 +575,18 @@ this.XPIDatabase = {
     }
     catch (e) {
       if (e.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
-        // XXX re-implement logic to decide whether to upgrade database
-        // by checking the DB_SCHEMA_VERSION preference.
-        // Fall back to attempting database upgrades
-        WARN("Extensions database not found; attempting to upgrade");
-        // See if there is SQLITE to migrate from
-        if (!this.loadSqliteData()) {
-          // Nope, try RDF
+        try {
+          let schemaVersion = Services.prefs.getIntPref(PREF_DB_SCHEMA);
+          if (schemaVersion <= LAST_SQLITE_DB_SCHEMA) {
+            // we should have an older SQLITE database
+            this.migrateData = this.getMigrateDataFromSQLITE();
+          }
+          // else we've upgraded before but the JSON file is gone, fall through
+          // and rebuild from scratch
+        }
+        catch(e) {
+          // No schema version pref means either a really old upgrade (RDF) or
+          // a new profile
           this.migrateData = this.getMigrateDataFromRDF();
         }
 
@@ -595,6 +597,7 @@ this.XPIDatabase = {
             " exists but is not readable; rebuilding in memory", e);
         // XXX open question - if we can overwrite at save time, should we, or should we
         // leave the locked database in case we can recover from it next time we start up?
+        // The old code made one attempt to remove the locked file before it rebuilt in memory
         this.lockedDatabase = true;
         // XXX TELEMETRY report when this happens?
         this.rebuildDatabase(aRebuildOnError);

toolkit/mozapps/extensions/test/xpcshell/test_migrate2.js

@@ -2,7 +2,7 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
-// Checks that we migrate data from future versions of the database
+// Checks that we migrate data from SQLITE databases
 // Note that since the database doesn't contain the foreignInstall field we
 // should just assume that no add-ons  in the user profile were foreignInstalls
 
@@ -177,7 +177,7 @@ function run_test() {
   stmt.finalize();
 
   db.schemaVersion = 10000;
-  Services.prefs.setIntPref("extensions.databaseSchema", 100);
+  Services.prefs.setIntPref("extensions.databaseSchema", 14);
   db.close();
 
   startupManager();

toolkit/mozapps/extensions/test/xpcshell/test_migrate_max_version.js

@@ -0,0 +1,102 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// Checks that we don't migrate data from SQLITE if
+// the "extensions.databaseSchema" preference shows we've
+// already upgraded to JSON
+
+// Enable loading extensions from the user and system scopes
+Services.prefs.setIntPref("extensions.enabledScopes",
+                          AddonManager.SCOPE_PROFILE + AddonManager.SCOPE_USER +
+                          AddonManager.SCOPE_SYSTEM);
+
+var addon1 = {
+  id: "addon1@tests.mozilla.org",
+  version: "1.0",
+  name: "Test 1",
+  targetApplications: [{
+    id: "xpcshell@tests.mozilla.org",
+    minVersion: "1",
+    maxVersion: "1"
+  }]
+};
+
+createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+const profileDir = gProfD.clone();
+profileDir.append("extensions");
+
+function run_test() {
+  writeInstallRDFForExtension(addon1, profileDir);
+
+  // Write out a minimal database
+  let dbfile = gProfD.clone();
+  dbfile.append("extensions.sqlite");
+  let db = AM_Cc["@mozilla.org/storage/service;1"].
+           getService(AM_Ci.mozIStorageService).
+           openDatabase(dbfile);
+  db.createTable("addon", "internal_id INTEGER PRIMARY KEY AUTOINCREMENT, " +
+                          "id TEXT, location TEXT, version TEXT, active INTEGER, " +
+                          "userDisabled INTEGER, installDate INTEGER");
+  db.createTable("targetApplication", "addon_internal_id INTEGER, " +
+                                      "id TEXT, minVersion TEXT, maxVersion TEXT");
+  let stmt = db.createStatement("INSERT INTO addon VALUES (NULL, :id, :location, " +
+                                ":version, :active, :userDisabled, :installDate)");
+
+  let internal_ids = {};
+
+  [["addon1@tests.mozilla.org", "app-profile", "1.0", "0", "1", "0"]
+   ].forEach(function(a) {
+    stmt.params.id = a[0];
+    stmt.params.location = a[1];
+    stmt.params.version = a[2];
+    stmt.params.active = a[3];
+    stmt.params.userDisabled = a[4];
+    stmt.params.installDate = a[5];
+    stmt.execute();
+    internal_ids[a[0]] = db.lastInsertRowID;
+  });
+  stmt.finalize();
+
+  db.schemaVersion = 15;
+  Services.prefs.setIntPref("extensions.databaseSchema", 14);
+  db.close();
+
+  startupManager();
+
+  AddonManager.getAddonByID("addon1@tests.mozilla.org",
+                            function check_before_rebuild (a1) {
+    // First check that it migrated OK once
+    // addon1 was disabled in the database
+    do_check_neq(a1, null);
+    do_check_true(a1.userDisabled);
+    do_check_false(a1.appDisabled);
+    do_check_false(a1.isActive);
+    do_check_false(a1.strictCompatibility);
+    do_check_false(a1.foreignInstall);
+
+    run_next_test();
+  });
+}
+
+// now shut down, remove the JSON database, 
+// start up again, and make sure the data didn't migrate this time
+add_test(function rebuild_again() {
+  shutdownManager();
+  gExtensionsJSON.remove(true);
+  startupManager();
+
+  AddonManager.getAddonByID("addon1@tests.mozilla.org",
+                            function check_after_rebuild(a1) {
+    // addon1 was rebuilt from extensions directory,
+    // so it appears enabled as a foreign install
+    do_check_neq(a1, null);
+    do_check_false(a1.userDisabled);
+    do_check_false(a1.appDisabled);
+    do_check_true(a1.isActive);
+    do_check_false(a1.strictCompatibility);
+    do_check_true(a1.foreignInstall);
+
+    run_next_test();
+  });
+});

toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini

@@ -205,6 +205,7 @@ skip-if = os == "android"
 [test_migrate4.js]
 [test_migrate5.js]
 [test_migrateAddonRepository.js]
+[test_migrate_max_version.js]
 [test_onPropertyChanged_appDisabled.js]
 [test_permissions.js]
 [test_plugins.js]