Bug 1309614 - finish initializing places before we import stuff, r?mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 13 Oct 2016 17:02:29 +0100
changeset 426564 ac85cb1c10f353a8e44d011095bfd25d7ba4e255
parent 425967 94b0fddf96b43942bdd851a3275042909ea37e09
child 534213 9f9a10c1a9e8c64538722bd4810190aae9b7f8d5
push id32744
push usergijskruitbosch@gmail.com
push dateTue, 18 Oct 2016 19:42:01 +0000
reviewersmak
bugs1309614
milestone52.0a1
Bug 1309614 - finish initializing places before we import stuff, r?mak MozReview-Commit-ID: JucE0HjQdwC
browser/components/migration/MigrationUtils.jsm
browser/components/nsBrowserGlue.js
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -4,16 +4,17 @@
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["MigrationUtils", "MigratorPrototype"];
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 const TOPIC_WILL_IMPORT_BOOKMARKS = "initial-migration-will-import-default-bookmarks";
 const TOPIC_DID_IMPORT_BOOKMARKS = "initial-migration-did-import-default-bookmarks";
+const TOPIC_PLACES_DEFAULTS_FINISHED = "places-browser-init-complete";
 
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AutoMigrate",
                                   "resource:///modules/AutoMigrate.jsm");
@@ -318,38 +319,45 @@ this.MigratorPrototype = {
 
           yield unblockMainThread();
         }
       }
     });
 
     if (MigrationUtils.isStartupMigration && !this.startupOnlyMigrator) {
       MigrationUtils.profileStartup.doStartup();
-
-      // If we're about to migrate bookmarks, first import the default bookmarks.
-      // Note We do not need to do so for the Firefox migrator
+      // First import the default bookmarks.
+      // Note: We do not need to do so for the Firefox migrator
       // (=startupOnlyMigrator), as it just copies over the places database
       // from another profile.
-      const BOOKMARKS = MigrationUtils.resourceTypes.BOOKMARKS;
-      let migratingBookmarks = resources.some(r => r.type == BOOKMARKS);
-      if (migratingBookmarks) {
+      Task.spawn(function* () {
+        // Tell nsBrowserGlue we're importing default bookmarks.
         let browserGlue = Cc["@mozilla.org/browser/browserglue;1"].
                           getService(Ci.nsIObserver);
         browserGlue.observe(null, TOPIC_WILL_IMPORT_BOOKMARKS, "");
 
-        // Note doMigrate doesn't care about the success of the import.
-        let onImportComplete = function() {
-          browserGlue.observe(null, TOPIC_DID_IMPORT_BOOKMARKS, "");
-          doMigrate();
-        };
-        BookmarkHTMLUtils.importFromURL(
-          "chrome://browser/locale/bookmarks.html", true).then(
-          onImportComplete, onImportComplete);
-        return;
-      }
+        // Import the default bookmarks. We ignore whether or not we succeed.
+        yield BookmarkHTMLUtils.importFromURL(
+          "chrome://browser/locale/bookmarks.html", true).catch(r => r);
+
+        // We'll tell nsBrowserGlue we've imported bookmarks, but before that
+        // we need to make sure we're going to know when it's finished
+        // initializing places:
+        let placesInitedPromise = new Promise(resolve => {
+          let onPlacesInited = function() {
+            Services.obs.removeObserver(onPlacesInited, TOPIC_PLACES_DEFAULTS_FINISHED);
+            resolve();
+          };
+          Services.obs.addObserver(onPlacesInited, TOPIC_PLACES_DEFAULTS_FINISHED, false);
+        });
+        browserGlue.observe(null, TOPIC_DID_IMPORT_BOOKMARKS, "");
+        yield placesInitedPromise;
+        doMigrate();
+      });
+      return;
     }
     doMigrate();
   },
 
   /**
    * DO NOT OVERRIDE - After deCOMing migration, this code
    * won't be part of the migrator itself.
    *
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1594,18 +1594,22 @@ BrowserGlue.prototype = {
       // If bookmarks are not imported, then initialize smart bookmarks.  This
       // happens during a common startup.
       // Otherwise, if any kind of import runs, smart bookmarks creation should be
       // delayed till the import operations has finished.  Not doing so would
       // cause them to be overwritten by the newly imported bookmarks.
       if (!importBookmarks) {
         // Now apply distribution customized bookmarks.
         // This should always run after Places initialization.
-        yield this._distributionCustomizer.applyBookmarks();
-        yield this.ensurePlacesDefaultQueriesInitialized();
+        try {
+          yield this._distributionCustomizer.applyBookmarks();
+          yield this.ensurePlacesDefaultQueriesInitialized();
+        } catch (e) {
+          Cu.reportError(e);
+        }
       }
       else {
         // An import operation is about to run.
         // Don't try to recreate smart bookmarks if autoExportHTML is true or
         // smart bookmarks are disabled.
         let smartBookmarksVersion = 0;
         try {
           smartBookmarksVersion = Services.prefs.getIntPref("browser.places.smartBookmarksVersion");
@@ -1637,17 +1641,17 @@ BrowserGlue.prototype = {
             // complete.
             yield this.ensurePlacesDefaultQueriesInitialized();
           } catch (e) {
             Cu.reportError(e);
           }
 
         }
         else {
-          Cu.reportError("Unable to find bookmarks.html file.");
+          Cu.reportError(new Error("Unable to find bookmarks.html file."));
         }
 
         // Reset preferences, so we won't try to import again at next run
         if (importBookmarksHTML)
           Services.prefs.setBoolPref("browser.places.importBookmarksHTML", false);
         if (restoreDefaultBookmarks)
           Services.prefs.setBoolPref("browser.bookmarks.restore_default_bookmarks",
                                      false);
@@ -1674,28 +1678,33 @@ BrowserGlue.prototype = {
           if (profileLastUse > lastBackupTime) {
             let backupAge = Math.round((profileLastUse - lastBackupTime) / 86400000);
             // Report the age of the last available backup.
             try {
               Services.telemetry
                       .getHistogramById("PLACES_BACKUPS_DAYSFROMLAST")
                       .add(backupAge);
             } catch (ex) {
-              Cu.reportError("Unable to report telemetry.");
+              Cu.reportError(new Error("Unable to report telemetry."));
             }
 
             if (backupAge > BOOKMARKS_BACKUP_MAX_INTERVAL_DAYS)
               this._bookmarksBackupIdleTime /= 2;
           }
         }
         this._idleService.addIdleObserver(this, this._bookmarksBackupIdleTime);
       }
 
+    }.bind(this)).catch(ex => {
+      Cu.reportError(ex);
+    }).then(() => {
+      // NB: deliberately after the catch so that we always do this, even if
+      // we threw halfway through initializing in the Task above.
       Services.obs.notifyObservers(null, "places-browser-init-complete", "");
-    }.bind(this));
+    });
   },
 
   /**
    * Places shut-down tasks
    * - finalize components depending on Places.
    * - export bookmarks as HTML, if so configured.
    */
   _onPlacesShutdown: function BG__onPlacesShutdown() {