Bug 1433979 - Remove deprecated code from PlacesBackups.jsm as it is no longer necessary. r?mak draft
authorMark Banner <standard8@mozilla.com>
Mon, 29 Jan 2018 18:20:24 +0000
changeset 749550 1a726d7a01ddadc1ae99c60cee4f05905361172f
parent 749226 c1154ebbe3fa43176dffcb0782809c648a027bcd
push id97428
push userbmo:standard8@mozilla.com
push dateWed, 31 Jan 2018 17:24:34 +0000
reviewersmak
bugs1433979
milestone60.0a1
Bug 1433979 - Remove deprecated code from PlacesBackups.jsm as it is no longer necessary. r?mak MozReview-Commit-ID: 5bNYJZvRInk
browser/components/places/content/places.js
toolkit/components/places/PlacesBackups.jsm
toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@@ -527,17 +527,18 @@ var PlacesOrganizer = {
    * of those items.
    */
   backupBookmarks: function PO_backupBookmarks() {
     let backupsDir = Services.dirsvc.get("Desk", Ci.nsIFile);
     let fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
     let fpCallback = function fpCallback_done(aResult) {
       if (aResult != Ci.nsIFilePicker.returnCancel) {
         // There is no OS.File version of the filepicker yet (Bug 937812).
-        PlacesBackups.saveBookmarksToJSONFile(fp.file.path);
+        PlacesBackups.saveBookmarksToJSONFile(fp.file.path)
+                     .catch(Cu.reportError);
       }
     };
 
     fp.init(window, PlacesUIUtils.getString("bookmarksBackupTitle"),
             Ci.nsIFilePicker.modeSave);
     fp.appendFilter(PlacesUIUtils.getString("bookmarksRestoreFilterName"),
                     RESTORE_FILEPICKER_FILTER_EXT);
     fp.defaultString = PlacesBackups.getFilenameForDate();
--- a/toolkit/components/places/PlacesBackups.jsm
+++ b/toolkit/components/places/PlacesBackups.jsm
@@ -7,30 +7,22 @@
 this.EXPORTED_SYMBOLS = ["PlacesBackups"];
 
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 const Cc = Components.classes;
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
-ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
-ChromeUtils.import("resource://gre/modules/osfile.jsm");
-ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
-ChromeUtils.defineModuleGetter(this, "BookmarkJSONUtils",
-  "resource://gre/modules/BookmarkJSONUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "Deprecated",
-  "resource://gre/modules/Deprecated.jsm");
-ChromeUtils.defineModuleGetter(this, "OS",
-  "resource://gre/modules/osfile.jsm");
-
-XPCOMUtils.defineLazyGetter(this, "localFileCtor",
-  () => Components.Constructor("@mozilla.org/file/local;1",
-                               "nsIFile", "initWithPath"));
+XPCOMUtils.defineLazyModuleGetters(this, {
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
+  BookmarkJSONUtils: "resource://gre/modules/BookmarkJSONUtils.jsm",
+  OS: "resource://gre/modules/osfile.jsm",
+});
 
 XPCOMUtils.defineLazyGetter(this, "filenamesRegex",
   () => /^bookmarks-([0-9-]+)(?:_([0-9]+)){0,1}(?:_([a-z0-9=+-]{24})){0,1}\.(json(lz4)?)$/i
 );
 
 /**
  * Appends meta-data information to a given filename.
  */
@@ -99,53 +91,29 @@ async function getTopLevelFolderIds() {
     guids.push({
       id: row.getResultByName("id"),
       guid: row.getResultByName("guid")
     });
   }
   return guids;
 }
 
-
 this.PlacesBackups = {
   /**
    * Matches the backup filename:
    *  0: file name
    *  1: date in form Y-m-d
    *  2: bookmarks count
    *  3: contents hash
    *  4: file extension
    */
   get filenamesRegex() {
     return filenamesRegex;
   },
 
-  get folder() {
-    Deprecated.warning(
-      "PlacesBackups.folder is deprecated and will be removed in a future version",
-      "https://bugzilla.mozilla.org/show_bug.cgi?id=859695");
-    return this._folder;
-  },
-
-  /**
-   * This exists just to avoid spamming deprecate warnings from internal calls
-   * needed to support deprecated methods themselves.
-   */
-  get _folder() {
-    let bookmarksBackupDir = Services.dirsvc.get("ProfD", Ci.nsIFile);
-    bookmarksBackupDir.append(this.profileRelativeFolderPath);
-    if (!bookmarksBackupDir.exists()) {
-      bookmarksBackupDir.create(Ci.nsIFile.DIRECTORY_TYPE, parseInt("0700", 8));
-      if (!bookmarksBackupDir.exists())
-        throw ("Unable to create bookmarks backup folder");
-    }
-    delete this._folder;
-    return this._folder = bookmarksBackupDir;
-  },
-
   /**
    * Gets backup folder asynchronously.
    * @return {Promise}
    * @resolve the folder (the folder string path).
    */
   getBackupFolder: function PB_getBackupFolder() {
     return (async () => {
       if (this._backupFolder) {
@@ -159,55 +127,16 @@ this.PlacesBackups = {
   },
 
   get profileRelativeFolderPath() {
     return "bookmarkbackups";
   },
 
   /**
    * Cache current backups in a sorted (by date DESC) array.
-   */
-  get entries() {
-    Deprecated.warning(
-      "PlacesBackups.entries is deprecated and will be removed in a future version",
-      "https://bugzilla.mozilla.org/show_bug.cgi?id=859695");
-    return this._entries;
-  },
-
-  /**
-   * This exists just to avoid spamming deprecate warnings from internal calls
-   * needed to support deprecated methods themselves.
-   */
-  get _entries() {
-    delete this._entries;
-    this._entries = [];
-    let files = this._folder.directoryEntries;
-    while (files.hasMoreElements()) {
-      let entry = files.getNext().QueryInterface(Ci.nsIFile);
-      // A valid backup is any file that matches either the localized or
-      // not-localized filename (bug 445704).
-      if (!entry.isHidden() && filenamesRegex.test(entry.leafName)) {
-        // Remove bogus backups in future dates.
-        if (this.getDateForFile(entry) > new Date()) {
-          entry.remove(false);
-          continue;
-        }
-        this._entries.push(entry);
-      }
-    }
-    this._entries.sort((a, b) => {
-      let aDate = this.getDateForFile(a);
-      let bDate = this.getDateForFile(b);
-      return bDate - aDate;
-    });
-    return this._entries;
-  },
-
-  /**
-   * Cache current backups in a sorted (by date DESC) array.
    * @return {Promise}
    * @resolve a sorted array of string paths.
    */
   getBackupFiles: function PB_getBackupFiles() {
     return (async () => {
       if (this._backupFiles)
         return this._backupFiles;
 
@@ -295,34 +224,16 @@ this.PlacesBackups = {
     let filename = (aBackupFile instanceof Ci.nsIFile) ? aBackupFile.leafName
                                                        : OS.Path.basename(aBackupFile);
     let matches = filename.match(filenamesRegex);
     if (!matches)
       throw ("Invalid backup file name: " + filename);
     return new Date(matches[1].replace(/-/g, "/"));
   },
 
-  /**
-   * Get the most recent backup file.
-   *
-   * @returns nsIFile backup file
-   */
-  getMostRecent: function PB_getMostRecent() {
-    Deprecated.warning(
-      "PlacesBackups.getMostRecent is deprecated and will be removed in a future version",
-      "https://bugzilla.mozilla.org/show_bug.cgi?id=859695");
-
-    for (let i = 0; i < this._entries.length; i++) {
-      let rx = /\.json(lz4)?$/;
-      if (this._entries[i].leafName.match(rx))
-        return this._entries[i];
-    }
-    return null;
-  },
-
    /**
     * Get the most recent backup file.
     *
     * @return {Promise}
     * @result the path to the file.
     */
    getMostRecentBackup: function PB_getMostRecentBackup() {
      return (async () => {
@@ -341,74 +252,63 @@ this.PlacesBackups = {
    * Serializes bookmarks using JSON, and writes to the supplied file.
    * Note: any item that should not be backed up must be annotated with
    *       "places/excludeFromBackup".
    *
    * @param aFilePath
    *        OS.File path for the "bookmarks.json" file to be created.
    * @return {Promise}
    * @resolves the number of serialized uri nodes.
-   * @deprecated passing an nsIFile is deprecated
    */
-  saveBookmarksToJSONFile: function PB_saveBookmarksToJSONFile(aFilePath) {
-    if (aFilePath instanceof Ci.nsIFile) {
-      Deprecated.warning("Passing an nsIFile to PlacesBackups.saveBookmarksToJSONFile " +
-                         "is deprecated. Please use an OS.File path instead.",
-                         "https://developer.mozilla.org/docs/JavaScript_OS.File");
-      aFilePath = aFilePath.path;
-    }
-    return (async () => {
-      let { count: nodeCount, hash: hash } =
-        await BookmarkJSONUtils.exportToFile(aFilePath);
+  async saveBookmarksToJSONFile(aFilePath) {
+    let { count: nodeCount, hash: hash } =
+      await BookmarkJSONUtils.exportToFile(aFilePath);
 
-      let backupFolderPath = await this.getBackupFolder();
-      if (OS.Path.dirname(aFilePath) == backupFolderPath) {
-        // We are creating a backup in the default backups folder,
-        // so just update the internal cache.
-        this._entries.unshift(new localFileCtor(aFilePath));
-        if (!this._backupFiles) {
-          await this.getBackupFiles();
+    let backupFolderPath = await this.getBackupFolder();
+    if (OS.Path.dirname(aFilePath) == backupFolderPath) {
+      // We are creating a backup in the default backups folder,
+      // so just update the internal cache.
+      if (!this._backupFiles) {
+        await this.getBackupFiles();
+      }
+      this._backupFiles.unshift(aFilePath);
+    } else {
+      // If we are saving to a folder different than our backups folder, then
+      // we also want to create a new compressed version in it.
+      // This way we ensure the latest valid backup is the same saved by the
+      // user.  See bug 424389.
+      let mostRecentBackupFile = await this.getMostRecentBackup();
+      if (!mostRecentBackupFile ||
+          hash != getHashFromFilename(OS.Path.basename(mostRecentBackupFile))) {
+        let name = this.getFilenameForDate(undefined, true);
+        let newFilename = appendMetaDataToFilename(name,
+                                                   { count: nodeCount,
+                                                     hash });
+        let newFilePath = OS.Path.join(backupFolderPath, newFilename);
+        let backupFile = await getBackupFileForSameDate(name);
+        if (backupFile) {
+          // There is already a backup for today, replace it.
+          await OS.File.remove(backupFile, { ignoreAbsent: true });
+          if (!this._backupFiles)
+            await this.getBackupFiles();
+          else
+            this._backupFiles.shift();
+          this._backupFiles.unshift(newFilePath);
+        } else {
+          // There is no backup for today, add the new one.
+          if (!this._backupFiles)
+            await this.getBackupFiles();
+          this._backupFiles.unshift(newFilePath);
         }
-        this._backupFiles.unshift(aFilePath);
-      } else {
-        // If we are saving to a folder different than our backups folder, then
-        // we also want to create a new compressed version in it.
-        // This way we ensure the latest valid backup is the same saved by the
-        // user.  See bug 424389.
-        let mostRecentBackupFile = await this.getMostRecentBackup();
-        if (!mostRecentBackupFile ||
-            hash != getHashFromFilename(OS.Path.basename(mostRecentBackupFile))) {
-          let name = this.getFilenameForDate(undefined, true);
-          let newFilename = appendMetaDataToFilename(name,
-                                                     { count: nodeCount,
-                                                       hash });
-          let newFilePath = OS.Path.join(backupFolderPath, newFilename);
-          let backupFile = await getBackupFileForSameDate(name);
-          if (backupFile) {
-            // There is already a backup for today, replace it.
-            await OS.File.remove(backupFile, { ignoreAbsent: true });
-            if (!this._backupFiles)
-              await this.getBackupFiles();
-            else
-              this._backupFiles.shift();
-            this._backupFiles.unshift(newFilePath);
-          } else {
-            // There is no backup for today, add the new one.
-            this._entries.unshift(new localFileCtor(newFilePath));
-            if (!this._backupFiles)
-              await this.getBackupFiles();
-            this._backupFiles.unshift(newFilePath);
-          }
-          let jsonString = await OS.File.read(aFilePath);
-          await OS.File.writeAtomic(newFilePath, jsonString, { compression: "lz4" });
-        }
+        let jsonString = await OS.File.read(aFilePath);
+        await OS.File.writeAtomic(newFilePath, jsonString, { compression: "lz4" });
       }
+    }
 
-      return nodeCount;
-    })();
+    return nodeCount;
   },
 
   /**
    * Creates a dated backup in <profile>/bookmarkbackups.
    * Stores the bookmarks using a lz4 compressed JSON file.
    * Note: any item that should not be backed up must be annotated with
    *       "places/excludeFromBackup".
    *
@@ -423,17 +323,16 @@ this.PlacesBackups = {
    */
   create: function PB_create(aMaxBackups, aForceBackup) {
     let limitBackups = async () => {
       let backupFiles = await this.getBackupFiles();
       if (typeof aMaxBackups == "number" && aMaxBackups > -1 &&
           backupFiles.length >= aMaxBackups) {
         let numberOfBackupsToDelete = backupFiles.length - aMaxBackups;
         while (numberOfBackupsToDelete--) {
-          this._entries.pop();
           let oldestBackup = this._backupFiles.pop();
           await OS.File.remove(oldestBackup);
         }
       }
     };
 
     return (async () => {
       if (aMaxBackups === 0) {
@@ -450,17 +349,16 @@ this.PlacesBackups = {
       // were required to enforce a new backup.
       let backupFile = await getBackupFileForSameDate(newBackupFilename);
       if (backupFile && !aForceBackup)
         return;
 
       if (backupFile) {
         // In case there is a backup for today we should recreate it.
         this._backupFiles.shift();
-        this._entries.shift();
         await OS.File.remove(backupFile, { ignoreAbsent: true });
       }
 
       // Now check the hash of the most recent backup, and try to create a new
       // backup, if that fails due to hash conflict, just rename the old backup.
       let mostRecentBackupFile = await this.getMostRecentBackup();
       let mostRecentHash = mostRecentBackupFile &&
                            getHashFromFilename(OS.Path.basename(mostRecentBackupFile));
@@ -479,32 +377,30 @@ this.PlacesBackups = {
                                                              hash });
       } catch (ex) {
         if (!ex.becauseSameHash) {
           throw ex;
         }
         // The last backup already contained up-to-date information, just
         // rename it as if it was today's backup.
         this._backupFiles.shift();
-        this._entries.shift();
         newBackupFile = mostRecentBackupFile;
         // Ensure we retain the proper extension when renaming
         // the most recent backup file.
         if (/\.json$/.test(OS.Path.basename(mostRecentBackupFile)))
           newBackupFilename = this.getFilenameForDate();
         newFilenameWithMetaData = appendMetaDataToFilename(
           newBackupFilename,
           { count: this.getBookmarkCountForFile(mostRecentBackupFile),
             hash: mostRecentHash });
       }
 
       // Append metadata to the backup filename.
       let newBackupFileWithMetadata = OS.Path.join(backupFolder, newFilenameWithMetaData);
       await OS.File.move(newBackupFile, newBackupFileWithMetadata);
-      this._entries.unshift(new localFileCtor(newBackupFileWithMetadata));
       this._backupFiles.unshift(newBackupFileWithMetadata);
 
       // Limit the number of backups.
       await limitBackups(aMaxBackups);
     })();
   },
 
   /**
--- a/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js
+++ b/toolkit/components/places/tests/bookmarks/test_818593-store-backup-metadata.js
@@ -10,23 +10,21 @@ add_task(async function test_saveBookmar
   // Add a bookmark
   let bookmark = await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     title: "Get Firefox!",
     url: "http://getfirefox.com/"
   });
 
   // Test saveBookmarksToJSONFile()
-  let backupFile = FileUtils.getFile("TmpD", ["bookmarks.json"]);
-  backupFile.create(Ci.nsIFile.NORMAL_FILE_TYPE, parseInt("0600", 8));
+  let backupFile = OS.Path.join(OS.Constants.Path.tmpDir, "bookmarks.json");
 
   let nodeCount = await PlacesBackups.saveBookmarksToJSONFile(backupFile, true);
   Assert.ok(nodeCount > 0);
-  Assert.ok(backupFile.exists());
-  Assert.equal(backupFile.leafName, "bookmarks.json");
+  Assert.ok(await OS.File.exists(backupFile));
 
   // Ensure the backup would be copied to our backups folder when the original
   // backup is saved somewhere else.
   let recentBackup = await PlacesBackups.getMostRecentBackup();
   let matches = OS.Path.basename(recentBackup).match(PlacesBackups.filenamesRegex);
   Assert.equal(matches[2], nodeCount);
   Assert.equal(matches[3].length, 24);
 
@@ -40,12 +38,12 @@ add_task(async function test_saveBookmar
 
   let mostRecentBackupFile = await PlacesBackups.getMostRecentBackup();
   Assert.notEqual(mostRecentBackupFile, null);
   matches = OS.Path.basename(recentBackup).match(PlacesBackups.filenamesRegex);
   Assert.equal(matches[2], nodeCount);
   Assert.equal(matches[3].length, 24);
 
   // Cleanup
-  backupFile.remove(false);
+  await OS.File.remove(backupFile);
   await PlacesBackups.create(0);
   await PlacesUtils.bookmarks.remove(bookmark);
 });