Bug 1272652 - Firefox fails to import bookmarks from Chrome if it also imports a large history. r=gijs

MozReview-Commit-ID: 3w5TIPi2S8d

--HG--
extra : rebase_source : 923d3b65cd5f5d74f0ecea8db6b8a68c71012981

browser/components/migration/ChromeProfileMigrator.js

@@ -31,7 +31,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OSCrypto",
                                   "resource://gre/modules/OSCrypto.jsm");
-
+XPCOMUtils.defineLazyModuleGetter(this, "Sqlite",
+                                  "resource://gre/modules/Sqlite.jsm");
 /**
  * Get an nsIFile instance representing the expected location of user data
  * for this copy of Chrome/Chromium/Canary on different OSes.
@@ -290,54 +291,63 @@ function GetHistoryResource(aProfileFolder) {
   return {
     type: MigrationUtils.resourceTypes.HISTORY,
 
-    migrate: function(aCallback) {
-      let dbConn = Services.storage.openUnsharedDatabase(historyFile);
-      let stmt = dbConn.createAsyncStatement(
-        "SELECT url, title, last_visit_time, typed_count FROM urls WHERE hidden = 0");
+    migrate(aCallback) {
+      Task.spawn(function* () {
+        let db = yield Sqlite.openConnection({
+          path: historyFile.path
+        });
 
-      stmt.executeAsync({
-        handleResult : function(aResults) {
-          let places = [];
-          for (let row = aResults.getNextRow(); row; row = aResults.getNextRow()) {
-            try {
-              // if having typed_count, we changes transition type to typed.
-              let transType = PlacesUtils.history.TRANSITION_LINK;
-              if (row.getResultByName("typed_count") > 0)
-                transType = PlacesUtils.history.TRANSITION_TYPED;
-
-              places.push({
-                uri: NetUtil.newURI(row.getResultByName("url")),
-                title: row.getResultByName("title"),
-                visits: [{
-                  transitionType: transType,
-                  visitDate: chromeTimeToDate(
-                               row.getResultByName(
-                                 "last_visit_time")) * 1000,
-                }],
-              });
-            } catch (e) {
-              Cu.reportError(e);
-            }
-          }
+        let rows = yield db.execute(`SELECT url, title, last_visit_time, typed_count
+                                     FROM urls WHERE hidden = 0`);
+        yield db.close();
 
+        let places = [];
+        for (let row of rows) {
           try {
-            PlacesUtils.asyncHistory.updatePlaces(places);
+            // if having typed_count, we changes transition type to typed.
+            let transType = PlacesUtils.history.TRANSITION_LINK;
+            if (row.getResultByName("typed_count") > 0)
+              transType = PlacesUtils.history.TRANSITION_TYPED;
+
+            places.push({
+              uri: NetUtil.newURI(row.getResultByName("url")),
+              title: row.getResultByName("title"),
+              visits: [{
+                transitionType: transType,
+                visitDate: chromeTimeToDate(
+                             row.getResultByName(
+                               "last_visit_time")) * 1000,
+              }],
+            });
           } catch (e) {
             Cu.reportError(e);
           }
-        },
-
-        handleError : function(aError) {
-          Cu.reportError("Async statement execution returned with '" +
-                         aError.result + "', '" + aError.message + "'");
-        },
-
-        handleCompletion : function(aReason) {
-          dbConn.asyncClose();
-          aCallback(aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED);
         }
-      });
-      stmt.finalize();
+
+        if (places.length > 0) {
+          yield new Promise((resolve, reject) => {
+            PlacesUtils.asyncHistory.updatePlaces(places, {
+              _success: false,
+              handleResult: function() {
+                // Importing any entry is considered a successful import.
+                this._success = true;
+              },
+              handleError: function() {},
+              handleCompletion: function() {
+                if (this._success) {
+                  resolve();
+                } else {
+                  reject(new Error("Couldn't add visits"));
+                }
+              }
+            });
+          });
+        }
+      }).then(() => { aCallback(true); },
+              ex => {
+                Cu.reportError(ex);
+                aCallback(false);
+              });
     }
   };
 }

browser/components/migration/MigrationUtils.jsm

@@ -19,6 +19,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "BookmarkHTMLUtils",
                                   "resource://gre/modules/BookmarkHTMLUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+                                  "resource://gre/modules/PromiseUtils.jsm");
 
 var gMigrators = null;
 var gProfileStartup = null;
@@ -227,8 +229,15 @@ this.MigratorPrototype = {
     if (aItems != Ci.nsIBrowserProfileMigrator.ALL)
       resources = resources.filter(r => aItems & r.type);
 
+    // Used to periodically give back control to the main-thread loop.
+    let unblockMainThread = function () {
+      return new Promise(resolve => {
+        Services.tm.mainThread.dispatch(resolve, Ci.nsIThread.DISPATCH_NORMAL);
+      });
+    };
+
     // Called either directly or through the bookmarks import callback.
-    function doMigrate() {
+    let doMigrate = Task.async(function*() {
       // TODO: use Map (for the items) and Set (for the resources)
       // once they are iterable.
       let resourcesGroupedByItems = new Map();
@@ -256,6 +265,7 @@ this.MigratorPrototype = {
         let itemSuccess = false;
         for (let res of itemResources) {
           let resource = res;
+          let completeDeferred = PromiseUtils.defer();
           let resourceDone = function(aSuccess) {
             let resourceIndex = itemResources.indexOf(resource);
             if (resourceIndex != -1) {
@@ -269,23 +279,31 @@ this.MigratorPrototype = {
                 if (resourcesGroupedByItems.size == 0)
                   notify("Migration:Ended");
               }
+              completeDeferred.resolve();
             }
           }
 
-          Services.tm.mainThread.dispatch(function() {
-            // If migrate throws, an error occurred, and the callback
-            // (itemMayBeDone) might haven't been called.
-            try {
-              resource.migrate(resourceDone);
-            }
-            catch(ex) {
-              Cu.reportError(ex);
-              resourceDone(false);
-            }
-          }, Ci.nsIThread.DISPATCH_NORMAL);
+          // If migrate throws, an error occurred, and the callback
+          // (itemMayBeDone) might haven't been called.
+          try {
+            resource.migrate(resourceDone);
+          }
+          catch(ex) {
+            Cu.reportError(ex);
+            resourceDone(false);
+          }
+
+          // Certain resources must be ran sequentially or they could fail,
+          // for example bookmarks and history (See bug 1272652).
+          if (key == MigrationUtils.resourceTypes.BOOKMARKS ||
+              key == MigrationUtils.resourceTypes.HISTORY) {
+            yield completeDeferred.promise;
+          }
+
+          yield unblockMainThread();
         }
       }
-    }
+    });
 
     if (MigrationUtils.isStartupMigration && !this.startupOnlyMigrator) {
       MigrationUtils.profileStartup.doStartup();

toolkit/modules/Sqlite.jsm

@@ -11,7 +11,7 @@ this.EXPORTED_SYMBOLS = [
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 // The time to wait before considering a transaction stuck and rejecting it.
-const TRANSACTIONS_QUEUE_TIMEOUT_MS = 120000 // 2 minutes
+const TRANSACTIONS_QUEUE_TIMEOUT_MS = 240000 // 4 minutes
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");