Bug 1433979 - Remove deprecated code from PlacesBackups.jsm as it is no longer necessary. r=mak
authorMark Banner <standard8@mozilla.com>
Mon, 29 Jan 2018 18:20:24 +0000
changeset 401725 d5600d2ea770b2ed55395213a4b20898df492227
parent 401724 c1c0bf22cf97a5d89729cc71da993b3a48aa6ceb
child 401726 b9195839434fed0228166224ca60fbf0cd235e8a
push id58922
push usermbanner@mozilla.com
push dateWed, 31 Jan 2018 17:28:24 +0000
treeherderautoland@d5600d2ea770 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1433979
milestone60.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 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);
 });