Bug 1444329 - Stop using nsIScriptableUnicodeConverter when importing bookmarks. r?mak,r?hsivonen
This also changes the way an empty file is handled when importing bookmarks - it will now throw with a syntax error.
The reason for the change is that using fetch's json converter, can't handle an empty file, and we can't easily adjust the data without reverting to JSON.parse manually.
MozReview-Commit-ID: KmVWZA5HIT
--- a/toolkit/components/places/BookmarkJSONUtils.jsm
+++ b/toolkit/components/places/BookmarkJSONUtils.jsm
@@ -4,18 +4,18 @@
var EXPORTED_SYMBOLS = [ "BookmarkJSONUtils" ];
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.import("resource://gre/modules/osfile.jsm");
ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
-ChromeUtils.defineModuleGetter(this, "NetUtil",
- "resource://gre/modules/NetUtil.jsm");
+Cu.importGlobalProperties(["fetch"]);
+
ChromeUtils.defineModuleGetter(this, "PlacesBackups",
"resource://gre/modules/PlacesBackups.jsm");
/**
* Generates an hash for the given string.
*
* @note The generated hash is returned in base64 form. Mind the fact base64
* is case-sensitive if you are going to reuse this code.
@@ -166,62 +166,40 @@ BookmarkImporter.prototype = {
*
* @param aSpec
* url of the bookmark data.
*
* @return {Promise}
* @resolves When the new bookmarks have been created.
* @rejects JavaScript exception.
*/
- importFromURL(spec) {
- return new Promise((resolve, reject) => {
- let streamObserver = {
- onStreamComplete: (aLoader, aContext, aStatus, aLength, aResult) => {
- let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
- createInstance(Ci.nsIScriptableUnicodeConverter);
- converter.charset = "UTF-8";
- try {
- let jsonString = converter.convertFromByteArray(aResult,
- aResult.length);
- resolve(this.importFromJSON(jsonString));
- } catch (ex) {
- Cu.reportError("Failed to import from URL: " + ex);
- reject(ex);
- }
- }
- };
+ async importFromURL(spec) {
+ let nodes = await (await fetch(spec)).json();
- let uri = NetUtil.newURI(spec);
- let channel = NetUtil.newChannel({
- uri,
- loadUsingSystemPrincipal: true
- });
- let streamLoader = Cc["@mozilla.org/network/stream-loader;1"]
- .createInstance(Ci.nsIStreamLoader);
- streamLoader.init(streamObserver);
- channel.asyncOpen2(streamLoader);
- });
+ if (!nodes.children || !nodes.children.length) {
+ return;
+ }
+
+ await this.import(nodes);
},
/**
* Import bookmarks from a compressed file.
*
* @param aFilePath
* OS.File path string of the bookmark data.
*
* @return {Promise}
* @resolves When the new bookmarks have been created.
* @rejects JavaScript exception.
*/
importFromCompressedFile: async function BI_importFromCompressedFile(aFilePath) {
let aResult = await OS.File.read(aFilePath, { compression: "lz4" });
- let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
- createInstance(Ci.nsIScriptableUnicodeConverter);
- converter.charset = "UTF-8";
- let jsonString = converter.convertFromByteArray(aResult, aResult.length);
+ let decoder = new TextDecoder();
+ let jsonString = decoder.decode(aResult);
await this.importFromJSON(jsonString);
},
/**
* Import bookmarks from a JSON string.
*
* @param {String} aString JSON string of serialized bookmark data.
* @return {Promise}
@@ -232,19 +210,23 @@ BookmarkImporter.prototype = {
let nodes =
PlacesUtils.unwrapNodes(aString, PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER);
if (nodes.length == 0 || !nodes[0].children ||
nodes[0].children.length == 0) {
return;
}
- // Change to nodes[0].children as we don't import the root, and also filter
+ await this.import(nodes[0]);
+ },
+
+ async import(rootNode) {
+ // Change to rootNode.children as we don't import the root, and also filter
// out any obsolete "tagsFolder" sections.
- nodes = nodes[0].children.filter(node => !node.root || node.root != "tagsFolder");
+ let nodes = rootNode.children.filter(node => node.root !== "tagsFolder");
// If we're replacing, then erase existing bookmarks first.
if (this._replace) {
await PlacesUtils.bookmarks.eraseEverything({ source: this._source });
}
let folderIdToGuidMap = {};
let searchGuids = [];
--- a/toolkit/components/places/tests/bookmarks/test_1016953-renaming-uncompressed.js
+++ b/toolkit/components/places/tests/bookmarks/test_1016953-renaming-uncompressed.js
@@ -22,21 +22,19 @@ add_task(async function test_same_date_s
await OS.File.move(tempPath, backupFile);
// Force a compressed backup which fallbacks to rename
await PlacesBackups.create();
let mostRecentBackupFile = await PlacesBackups.getMostRecentBackup();
// check to ensure not renamed to jsonlz4
Assert.equal(mostRecentBackupFile, backupFile);
// inspect contents and check if valid json
- let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
- createInstance(Ci.nsIScriptableUnicodeConverter);
- converter.charset = "UTF-8";
let result = await OS.File.read(mostRecentBackupFile);
- let jsonString = converter.convertFromByteArray(result, result.length);
+ let decoder = new TextDecoder();
+ let jsonString = decoder.decode(result);
info("Check is valid JSON");
JSON.parse(jsonString);
// Cleanup
await OS.File.remove(backupFile);
await OS.File.remove(tempPath);
PlacesBackups._backupFiles = null; // To force re-cache of backupFiles
});
@@ -52,21 +50,19 @@ add_task(async function test_same_date_d
let filename = "bookmarks-" + PlacesBackups.toISODateString(dateObj) + "_" +
count + "_differentHash==.json";
let backupFile = OS.Path.join(backupFolder, filename);
await OS.File.move(tempPath, backupFile);
await PlacesBackups.create(); // Force compressed backup
let mostRecentBackupFile = await PlacesBackups.getMostRecentBackup();
// Decode lz4 compressed file to json and check if json is valid
- let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
- createInstance(Ci.nsIScriptableUnicodeConverter);
- converter.charset = "UTF-8";
let result = await OS.File.read(mostRecentBackupFile, { compression: "lz4" });
- let jsonString = converter.convertFromByteArray(result, result.length);
+ let decoder = new TextDecoder();
+ let jsonString = decoder.decode(result);
info("Check is valid JSON");
JSON.parse(jsonString);
// Cleanup
await OS.File.remove(mostRecentBackupFile);
await OS.File.remove(tempPath);
PlacesBackups._backupFiles = null; // To force re-cache of backupFiles
});
--- a/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js
+++ b/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js
@@ -130,53 +130,50 @@ add_task(async function test_json_restor
let file = await promiseFile("bookmarks-test_restoreNotification.json");
await addBookmarks();
await BookmarkJSONUtils.exportToFile(file);
await PlacesUtils.bookmarks.eraseEverything();
try {
await BookmarkJSONUtils.importFromFile(file, { replace: true });
} catch (e) {
- do_throw(" Restore should not have failed" + e);
+ do_throw(" Restore should not have failed " + e);
}
await checkObservers(expectPromises, expectedData);
await teardown(file);
});
add_task(async function test_json_restore_empty() {
let expectedData = {
data: NSIOBSERVER_DATA_JSON,
folderId: null
};
- let expectPromises = registerObservers(true);
+ let expectPromises = registerObservers(false);
- info("JSON restore: empty file should succeed");
+ info("JSON restore: empty file should fail");
let file = await promiseFile("bookmarks-test_restoreNotification.json");
- try {
- await BookmarkJSONUtils.importFromFile(file, { replace: true });
- } catch (e) {
- do_throw(" Restore should not have failed" + e);
- }
+ await Assert.rejects(BookmarkJSONUtils.importFromFile(file, { replace: true }),
+ /SyntaxError/, "Restore should reject for an empty file.");
await checkObservers(expectPromises, expectedData);
await teardown(file);
});
add_task(async function test_json_restore_nonexist() {
let expectedData = {
data: NSIOBSERVER_DATA_JSON,
folderId: null
};
let expectPromises = registerObservers(false);
info("JSON restore: nonexistent file should fail");
let file = Services.dirsvc.get("ProfD", Ci.nsIFile);
file.append("this file doesn't exist because nobody created it 1");
- Assert.rejects(BookmarkJSONUtils.importFromFile(file.path, { replace: true }),
+ await Assert.rejects(BookmarkJSONUtils.importFromFile(file.path, { replace: true }),
/Cannot restore from nonexisting json file/, "Restore should reject for a non-existent file.");
await checkObservers(expectPromises, expectedData);
await teardown(file);
});
add_task(async function test_html_restore_normal() {
let expectedData = {