Bug 983571 - browser.bookmarks.autoExportHTML = true no longer works. r=Yoric a=sylvestre
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 25 Mar 2014 17:25:58 +0100
changeset 185096 4254ea7e36689810103438605f13b6267bba16dc
parent 185095 849084164ca7107ab757c86cddfbbd0f717afe6d
child 185097 72b5814f3a3b20c3b20a53a73e5307f31f2b7cfa
push id5507
push usermak77@bonardo.net
push dateWed, 26 Mar 2014 10:12:18 +0000
treeherdermozilla-aurora@4254ea7e3668 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric, sylvestre
bugs983571
milestone30.0a2
Bug 983571 - browser.bookmarks.autoExportHTML = true no longer works. r=Yoric a=sylvestre
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
@@ -1058,16 +1058,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) {
@@ -1115,20 +1128,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;
@@ -1221,29 +1230,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);