Bug 983571 - browser.bookmarks.autoExportHTML = true no longer works. r=Yoric
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 25 Mar 2014 17:25:58 +0100
changeset 175313 f162ae54353de2c67d068c3d8e59146fba20b08d
parent 175312 23f6bebf6b2e3dcfa29e6f4d85fbe4b928653be2
child 175314 9463549368305a199aab2088f240b08a826e85d2
push id26486
push userkwierso@gmail.com
push dateWed, 26 Mar 2014 03:03:25 +0000
treeherdermozilla-central@140ac04d7454 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs983571
milestone31.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 983571 - browser.bookmarks.autoExportHTML = true no longer works. r=Yoric
browser/components/nsBrowserGlue.js
browser/components/places/tests/unit/test_browserGlue_bookmarkshtml.js
browser/components/places/tests/unit/xpcshell.ini
toolkit/modules/AsyncShutdown.jsm
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1070,16 +1070,29 @@ BrowserGlue.prototype = {
     let importBookmarksHTML = false;
     try {
       importBookmarksHTML =
         Services.prefs.getBoolPref("browser.places.importBookmarksHTML");
       if (importBookmarksHTML)
         importBookmarks = true;
     } catch(ex) {}
 
+    // Support legacy bookmarks.html format for apps that depend on that format.
+    let autoExportHTML = false;
+    try {
+      autoExportHTML = Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML");
+    } catch (ex) {} // Do not export.
+    if (autoExportHTML) {
+      // Sqlite.jsm and Places shutdown happen at profile-before-change, thus,
+      // to be on the safe side, this should run earlier.
+      AsyncShutdown.profileChangeTeardown.addBlocker(
+        "Places: export bookmarks.html",
+        () => BookmarkHTMLUtils.exportToFile(BookmarkHTMLUtils.defaultPath));
+    }
+
     Task.spawn(function() {
       // Check if Safe Mode or the user has required to restore bookmarks from
       // default profile's bookmarks.html
       let restoreDefaultBookmarks = false;
       try {
         restoreDefaultBookmarks =
           Services.prefs.getBoolPref("browser.bookmarks.restore_default_bookmarks");
         if (restoreDefaultBookmarks) {
@@ -1127,20 +1140,16 @@ BrowserGlue.prototype = {
         // This should always run after Places initialization.
         this._distributionCustomizer.applyBookmarks();
         this.ensurePlacesDefaultQueriesInitialized();
       }
       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 autoExportHTML = false;
-        try {
-          autoExportHTML = Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML");
-        } catch(ex) {}
         let smartBookmarksVersion = 0;
         try {
           smartBookmarksVersion = Services.prefs.getIntPref("browser.places.smartBookmarksVersion");
         } catch(ex) {}
         if (!autoExportHTML && smartBookmarksVersion != -1)
           Services.prefs.setIntPref("browser.places.smartBookmarksVersion", 0);
 
         let bookmarksUrl = null;
@@ -1233,29 +1242,16 @@ BrowserGlue.prototype = {
   _onPlacesShutdown: function BG__onPlacesShutdown() {
     this._sanitizer.onShutdown();
     PageThumbs.uninit();
 
     if (this._bookmarksBackupIdleTime) {
       this._idleService.removeIdleObserver(this, this._bookmarksBackupIdleTime);
       delete this._bookmarksBackupIdleTime;
     }
-
-    // Support legacy bookmarks.html format for apps that depend on that format.
-    try {
-      if (Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML")) {
-        // places-shutdown happens at profile-change-teardown, so here we
-        // can safely add a profile-before-change blocker.
-        AsyncShutdown.profileBeforeChange.addBlocker(
-          "Places: bookmarks.html",
-          () => BookmarkHTMLUtils.exportToFile(BookmarkHTMLUtils.defaultPath)
-                                 .then(null, Cu.reportError)
-        );
-      }
-    } catch (ex) {} // Do not export.
   },
 
   /**
    * If a backup for today doesn't exist, this creates one.
    */
   _backupBookmarks: function BG__backupBookmarks() {
     return Task.spawn(function() {
       let lastBackupFile = yield PlacesBackups.getMostRecentBackup();
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/unit/test_browserGlue_bookmarkshtml.js
@@ -0,0 +1,33 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Tests that nsBrowserGlue correctly exports bookmarks.html at shutdown if
+ * browser.bookmarks.autoExportHTML is set to true.
+ */
+
+function run_test() {
+  run_next_test();
+}
+
+add_task(function* () {
+  remove_bookmarks_html();
+
+  Services.prefs.setBoolPref("browser.bookmarks.autoExportHTML", true);
+  do_register_cleanup(() => Services.prefs.clearUserPref("browser.bookmarks.autoExportHTML"));
+
+  // Initialize nsBrowserGlue before Places.
+  Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIBrowserGlue);
+
+  // Initialize Places through the History Service.
+  Cc["@mozilla.org/browser/nav-history-service;1"]
+    .getService(Ci.nsINavHistoryService);
+
+  Services.obs.addObserver(function observer() {
+    Services.obs.removeObserver(observer, "profile-before-change");
+    check_bookmarks_html();
+  }, "profile-before-change", false);
+});
--- a/browser/components/places/tests/unit/xpcshell.ini
+++ b/browser/components/places/tests/unit/xpcshell.ini
@@ -4,16 +4,17 @@ tail =
 firefox-appdir = browser
 support-files =
   bookmarks.glue.html
   bookmarks.glue.json
   corruptDB.sqlite
   distribution.ini
 
 [test_421483.js]
+[test_browserGlue_bookmarkshtml.js]
 [test_browserGlue_corrupt.js]
 [test_browserGlue_corrupt_nobackup.js]
 [test_browserGlue_corrupt_nobackup_default.js]
 [test_browserGlue_distribution.js]
 [test_browserGlue_migrate.js]
 [test_browserGlue_prefs.js]
 [test_browserGlue_restore.js]
 [test_browserGlue_smartBookmarks.js]
--- a/toolkit/modules/AsyncShutdown.jsm
+++ b/toolkit/modules/AsyncShutdown.jsm
@@ -281,17 +281,17 @@ Spinner.prototype = {
    * before we proceed to the next runstate. If |action| returns a promise,
    * we wait until the promise is resolved/rejected before proceeding
    * to the next runstate.
    */
   addBlocker: function(condition) {
     if (!this._conditions) {
       throw new Error("Phase " + this._topic +
                       " has already begun, it is too late to register" +
-                      " completion conditions.");
+                      " completion condition '" + condition.name + "'.");
     }
     this._conditions.add(condition);
   },
 
   observe: function() {
     let topic = this._topic;
     Services.obs.removeObserver(this, topic);
 
@@ -449,12 +449,13 @@ Spinner.prototype = {
 };
 
 
 // List of well-known runstates
 // Ideally, runstates should be registered from the component that decides
 // when they start/stop. For compatibility with existing startup/shutdown
 // mechanisms, we register a few runstates here.
 
+this.AsyncShutdown.profileChangeTeardown = getPhase("profile-change-teardown");
 this.AsyncShutdown.profileBeforeChange = getPhase("profile-before-change");
 this.AsyncShutdown.sendTelemetry = getPhase("profile-before-change2");
 this.AsyncShutdown.webWorkersShutdown = getPhase("web-workers-shutdown");
 Object.freeze(this.AsyncShutdown);