Revert "Fix bookmarks backup logic."

This reverts commit 99f5afe64b393809c4bdee6c92d0769091bd2ca0.

1 files changed, 52 insertions, 36 deletions

diff --git a/application/palemoon/components/nsBrowserGlue.js b/application/palemoon/components/nsBrowserGlue.js
index 78b14a2e4..ad48dab29 100644
--- a/application/palemoon/components/nsBrowserGlue.js
+++ b/application/palemoon/components/nsBrowserGlue.js
@@ -58,8 +58,8 @@ const PREF_PLUGINS_NOTIFYUSER = "plugins.update.notifyUser";
 const PREF_PLUGINS_UPDATEURL  = "plugins.update.url";
 
 // We try to backup bookmarks at idle times, to avoid doing that at shutdown.
-// Number of idle seconds before trying to backup bookmarks.  10 minutes.
-const BOOKMARKS_BACKUP_IDLE_TIME = 10 * 60;
+// Number of idle seconds before trying to backup bookmarks.  15 minutes.
+const BOOKMARKS_BACKUP_IDLE_TIME = 15 * 60;
 // Minimum interval in milliseconds between backups.
 const BOOKMARKS_BACKUP_INTERVAL = 86400 * 1000;
 // Maximum number of backups to create.  Old ones will be purged.
@@ -238,9 +238,9 @@ BrowserGlue.prototype = {
         this._onPlacesShutdown();
         break;
       case "idle":
-        if (this._idleService.idleTime > BOOKMARKS_BACKUP_IDLE_TIME * 1000) {
+        if ((this._idleService.idleTime > BOOKMARKS_BACKUP_IDLE_TIME * 1000) &&
+             this._shouldBackupBookmarks())
           this._backupBookmarks();
-        }
         break;
       case "distribution-customization-complete":
         Services.obs.removeObserver(this, "distribution-customization-complete");
@@ -941,7 +941,8 @@ BrowserGlue.prototype = {
           Services.prefs.getBoolPref("browser.bookmarks.restore_default_bookmarks");
         if (restoreDefaultBookmarks) {
           // Ensure that we already have a bookmarks backup for today.
-          yield this._backupBookmarks();
+          if (this._shouldBackupBookmarks())
+            yield this._backupBookmarks();
           importBookmarks = true;
         }
       } catch(ex) {}
@@ -950,7 +951,7 @@ BrowserGlue.prototype = {
       // from bookmarks.html, we will try to restore from JSON/JSONLZ4
       if (importBookmarks && !restoreDefaultBookmarks && !importBookmarksHTML) {
         // get latest JSON/JSONLZ4 backup
-        var bookmarksBackupFile = PlacesBackups.getMostRecentBackup();
+        var bookmarksBackupFile = yield PlacesBackups.getMostRecentBackup();
         if (bookmarksBackupFile) {
           // restore from JSON/JSONLZ4 backup
           yield BookmarkJSONUtils.importFromFile(bookmarksBackupFile, true);
@@ -1093,60 +1094,75 @@ BrowserGlue.prototype = {
     }
 
     let waitingForBackupToComplete = true;
-    this._backupBookmarks().then(
-      function onSuccess() {
-        waitingForBackupToComplete = false;
-      },
-      function onFailure() {
-        Cu.reportError("Unable to backup bookmarks.");
-        waitingForBackupToComplete = false;
-      }
-    );
+    if (this._shouldBackupBookmarks()) {
+      waitingForBackupToComplete = false;
+      this._backupBookmarks().then(
+        function onSuccess() {
+          waitingForBackupToComplete = true;
+        },
+        function onFailure() {
+          Cu.reportError("Unable to backup bookmarks.");
+          waitingForBackupToComplete = true;
+        }
+      );
+    }
 
     // Backup bookmarks to bookmarks.html to support apps that depend
     // on the legacy format.
-    let waitingForHTMLExportToComplete = false;
+    let waitingForHTMLExportToComplete = true;
+    // If this fails to get the preference value, we don't export.
     if (Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML")) {
-      // Exporting to HTML is explicitly enabled.
-      // We spin the event loop on shutdown, to wait for the export to finish.
-      waitingForHTMLExportToComplete = true;
+      // Exceptionally, since this is a non-default setting and HTML format is
+      // discouraged in favor of the JSON/JSONLZ4 backups, we spin the event
+      // loop on shutdown, to wait for the export to finish.  We cannot safely
+      // spin the event loop on shutdown until we include a watchdog to prevent
+      // potential hangs (bug 518683).  The asynchronous shutdown operations
+      // will then be handled by a shutdown service (bug 435058).
+      waitingForHTMLExportToComplete = false;
       BookmarkHTMLUtils.exportToFile(BookmarkHTMLUtils.defaultPath).then(
         function onSuccess() {
-          waitingForHTMLExportToComplete = false;
+          waitingForHTMLExportToComplete = true;
         },
         function onFailure() {
           Cu.reportError("Unable to auto export html.");
-          waitingForHTMLExportToComplete = false;
+          waitingForHTMLExportToComplete = true;
         }
       );
     }
 
-    // The events loop should spin at least once because waitingForBackupToComplete
-    // is true before checking whether backup should be made.
     let thread = Services.tm.currentThread;
-    while (waitingForBackupToComplete || waitingForHTMLExportToComplete) {
+    while (!waitingForBackupToComplete || !waitingForHTMLExportToComplete) {
       thread.processNextEvent(true);
     }
   },
 
   /**
+   * Determine whether to backup bookmarks or not.
+   * @return true if bookmarks should be backed up, false if not.
+   */
+  _shouldBackupBookmarks: function BG__shouldBackupBookmarks() {
+    let lastBackupFile = PlacesBackups.getMostRecent();
+
+    // Should backup bookmarks if there are no backups or the maximum interval between
+    // backups elapsed.
+    return (!lastBackupFile ||
+            new Date() - PlacesBackups.getDateForFile(lastBackupFile) > BOOKMARKS_BACKUP_INTERVAL);
+  },
+
+  /**
    * Backup bookmarks.
    */
   _backupBookmarks: function BG__backupBookmarks() {
     return Task.spawn(function() {
-      let lastBackupFile = yield PlacesBackups.getMostRecentBackup();
-      // We should backup bookmarks if there are no backups or the maximum
-      // interval between backups has lapsed.
-      let hasLapsed = (new Date() - PlacesBackups.getDateForFile(lastBackupFile)) > BOOKMARKS_BACKUP_INTERVAL;
-      if (!lastBackupFile || hasLapsed) {
-        let maxBackups = BOOKMARKS_BACKUP_MAX_BACKUPS;
-        try {
-          maxBackups = Services.prefs.getIntPref("browser.bookmarks.max_backups");
-        }
-        catch(ex) { /* Use default. */ }
-
-        yield PlacesBackups.create(maxBackups); // Don't force creation.
+      // Backup bookmarks if there are no backups or the maximum interval between
+      // backups elapsed.
+      let maxBackups = BOOKMARKS_BACKUP_MAX_BACKUPS;
+      try {
+        maxBackups = Services.prefs.getIntPref("browser.bookmarks.max_backups");
       }
+      catch(ex) { /* Use default. */ }
+
+      yield PlacesBackups.create(maxBackups); // Don't force creation.
     });
   },