Fix bookmarks backup logic.

This is a follow-up to an incomplete patch submitted to Tycho:
https://github.com/MoonchildProductions/Pale-Moon/pull/1082

Also reduces the idle time before attempting to backup in the background
to 10 minutes (was 15).

1 files changed, 36 insertions, 52 deletions

diff --git a/application/palemoon/components/nsBrowserGlue.js b/application/palemoon/components/nsBrowserGlue.js
index f0a7aa22a..64b193d43 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.  15 minutes.
-const BOOKMARKS_BACKUP_IDLE_TIME = 15 * 60;
+// Number of idle seconds before trying to backup bookmarks.  10 minutes.
+const BOOKMARKS_BACKUP_IDLE_TIME = 10 * 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) &&
-             this._shouldBackupBookmarks())
+        if (this._idleService.idleTime > BOOKMARKS_BACKUP_IDLE_TIME * 1000) {
           this._backupBookmarks();
+        }
         break;
       case "distribution-customization-complete":
         Services.obs.removeObserver(this, "distribution-customization-complete");
@@ -949,8 +949,7 @@ BrowserGlue.prototype = {
           Services.prefs.getBoolPref("browser.bookmarks.restore_default_bookmarks");
         if (restoreDefaultBookmarks) {
           // Ensure that we already have a bookmarks backup for today.
-          if (this._shouldBackupBookmarks())
-            yield this._backupBookmarks();
+          yield this._backupBookmarks();
           importBookmarks = true;
         }
       } catch(ex) {}
@@ -959,7 +958,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 = yield PlacesBackups.getMostRecentBackup();
+        var bookmarksBackupFile = PlacesBackups.getMostRecentBackup();
         if (bookmarksBackupFile) {
           // restore from JSON/JSONLZ4 backup
           yield BookmarkJSONUtils.importFromFile(bookmarksBackupFile, true);
@@ -1102,75 +1101,60 @@ BrowserGlue.prototype = {
     }
 
     let waitingForBackupToComplete = true;
-    if (this._shouldBackupBookmarks()) {
-      waitingForBackupToComplete = false;
-      this._backupBookmarks().then(
-        function onSuccess() {
-          waitingForBackupToComplete = true;
-        },
-        function onFailure() {
-          Cu.reportError("Unable to backup bookmarks.");
-          waitingForBackupToComplete = true;
-        }
-      );
-    }
+    this._backupBookmarks().then(
+      function onSuccess() {
+        waitingForBackupToComplete = false;
+      },
+      function onFailure() {
+        Cu.reportError("Unable to backup bookmarks.");
+        waitingForBackupToComplete = false;
+      }
+    );
 
     // Backup bookmarks to bookmarks.html to support apps that depend
     // on the legacy format.
-    let waitingForHTMLExportToComplete = true;
-    // If this fails to get the preference value, we don't export.
+    let waitingForHTMLExportToComplete = false;
     if (Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML")) {
-      // 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;
+      // Exporting to HTML is explicitly enabled.
+      // We spin the event loop on shutdown, to wait for the export to finish.
+      waitingForHTMLExportToComplete = true;
       BookmarkHTMLUtils.exportToFile(BookmarkHTMLUtils.defaultPath).then(
         function onSuccess() {
-          waitingForHTMLExportToComplete = true;
+          waitingForHTMLExportToComplete = false;
         },
         function onFailure() {
           Cu.reportError("Unable to auto export html.");
-          waitingForHTMLExportToComplete = true;
+          waitingForHTMLExportToComplete = false;
         }
       );
     }
 
+    // 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() {
-      // 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. */ }
+      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.
+        yield PlacesBackups.create(maxBackups); // Don't force creation.
+      }
     });
   },