From cbfeb4cc13d41778290e1cd2a766d3a40138ba6a Mon Sep 17 00:00:00 2001 From: Raymond Lee Date: Wed, 17 Apr 2013 13:36:02 +0800 Subject: [PATCH] Bug 852034 - Replace restoreBookmarksFromJSONFile with importFromFile. r=mano --- browser/components/nsBrowserGlue.js | 7 +- browser/components/places/content/places.js | 18 +- .../tests/unit/test_browserGlue_prefs.js | 4 +- .../sync/tests/unit/test_bookmark_engine.js | 2 +- .../components/places/BookmarkJSONUtils.jsm | 209 ++++++++++-------- .../bookmarks/test_405938_restore_queries.js | 2 +- .../test_417228-exclude-from-backup.js | 4 +- .../bookmarks/test_417228-other-roots.js | 2 +- .../test_424958-json-quoted-folders.js | 2 +- .../places/tests/bookmarks/test_448584.js | 2 +- .../places/tests/bookmarks/test_458683.js | 2 +- .../places/tests/unit/test_384370.js | 2 +- .../test_bookmarks_restore_notification.js | 29 ++- 13 files changed, 163 insertions(+), 122 deletions(-) diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index 91f0333046ac..c590e1d298c7 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -32,6 +32,9 @@ XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils", XPCOMUtils.defineLazyModuleGetter(this, "BookmarkHTMLUtils", "resource://gre/modules/BookmarkHTMLUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "BookmarkJSONUtils", + "resource://gre/modules/BookmarkJSONUtils.jsm"); + XPCOMUtils.defineLazyModuleGetter(this, "webappsUI", "resource:///modules/webappsUI.jsm"); @@ -931,7 +934,7 @@ BrowserGlue.prototype = { var bookmarksBackupFile = PlacesUtils.backups.getMostRecent("json"); if (bookmarksBackupFile) { // restore from JSON backup - PlacesUtils.restoreBookmarksFromJSONFile(bookmarksBackupFile); + yield BookmarkJSONUtils.importFromFile(bookmarksBackupFile, true); importBookmarks = false; } else { @@ -1031,6 +1034,8 @@ BrowserGlue.prototype = { this._idleService.addIdleObserver(this, BOOKMARKS_BACKUP_IDLE_TIME); this._isIdleObserver = true; } + + Services.obs.notifyObservers(null, "places-browser-init-complete", ""); }.bind(this)); }, diff --git a/browser/components/places/content/places.js b/browser/components/places/content/places.js index a64637ed1acd..2fa3bd42097c 100644 --- a/browser/components/places/content/places.js +++ b/browser/components/places/content/places.js @@ -3,7 +3,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); Components.utils.import("resource:///modules/MigrationUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Task", + "resource://gre/modules/Task.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "BookmarkJSONUtils", + "resource://gre/modules/BookmarkJSONUtils.jsm"); var PlacesOrganizer = { _places: null, @@ -441,12 +446,13 @@ var PlacesOrganizer = { PlacesUIUtils.getString("bookmarksRestoreAlert"))) return; - try { - PlacesUtils.restoreBookmarksFromJSONFile(aFile); - } - catch(ex) { - this._showErrorAlert(PlacesUIUtils.getString("bookmarksRestoreParseError")); - } + Task.spawn(function() { + try { + yield BookmarkJSONUtils.importFromFile(aFile, true); + } catch(ex) { + PlacesOrganizer._showErrorAlert(PlacesUIUtils.getString("bookmarksRestoreParseError")); + } + }); }, _showErrorAlert: function PO__showErrorAlert(aMsg) { diff --git a/browser/components/places/tests/unit/test_browserGlue_prefs.js b/browser/components/places/tests/unit/test_browserGlue_prefs.js index d3997057e4ea..e8366a6146f2 100644 --- a/browser/components/places/tests/unit/test_browserGlue_prefs.js +++ b/browser/components/places/tests/unit/test_browserGlue_prefs.js @@ -39,7 +39,7 @@ function waitForImportAndSmartBookmarks(aCallback) { // Wait for Places init notification. Services.obs.addObserver(function(aSubject, aTopic, aData) { Services.obs.removeObserver(arguments.callee, - PlacesUtils.TOPIC_INIT_COMPLETE); + "places-browser-init-complete"); do_execute_soon(function () { // Ensure preferences status. do_check_false(Services.prefs.getBoolPref(PREF_AUTO_EXPORT_HTML)); @@ -58,7 +58,7 @@ function waitForImportAndSmartBookmarks(aCallback) { run_next_test(); }); - }, PlacesUtils.TOPIC_INIT_COMPLETE, false); + }, "places-browser-init-complete", false); }, function test_import() diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index 782713d97a97..aa486c3e0471 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -231,7 +231,7 @@ add_task(function test_restorePromptsReupload() { do_check_eq(wbos[0], bmk2_guid); _("Now restore from a backup."); - PlacesUtils.restoreBookmarksFromJSONFile(backupFile); + yield BookmarkJSONUtils.importFromFile(backupFile, true); _("Ensure we have the bookmarks we expect locally."); let guids = store.getAllIDs(); diff --git a/toolkit/components/places/BookmarkJSONUtils.jsm b/toolkit/components/places/BookmarkJSONUtils.jsm index 6b29f1f1e8a4..28a5f55791cb 100644 --- a/toolkit/components/places/BookmarkJSONUtils.jsm +++ b/toolkit/components/places/BookmarkJSONUtils.jsm @@ -49,7 +49,7 @@ this.BookmarkJSONUtils = Object.freeze({ */ importFromFile: function BJU_importFromFile(aFile, aReplace) { let importer = new BookmarkImporter(); - return importer.importFromURL(NetUtil.newURI(aFile).spec, aReplace); + return importer.importFromFile(aFile, aReplace); }, /** @@ -70,6 +70,31 @@ this.BookmarkJSONUtils = Object.freeze({ function BookmarkImporter() {} BookmarkImporter.prototype = { + /** + * Import bookmarks from a file. + * + * @param aFile + * the bookmark file. + * @param aReplace + * Boolean if true, replace existing bookmarks, else merge. + * + * @return {Promise} + * @resolves When the new bookmarks have been created. + * @rejects JavaScript exception. + */ + importFromFile: function(aFile, aReplace) { + if (aFile.exists()) { + return this.importFromURL(NetUtil.newURI(aFile).spec, aReplace); + } + + notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN); + + return Task.spawn(function() { + notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED); + throw new Error("File does not exist."); + }); + }, + /** * Import bookmarks from a url. * @@ -143,104 +168,104 @@ BookmarkImporter.prototype = { Services.tm.mainThread.dispatch(function() { deferred.resolve(); // Nothing to restore }, Ci.nsIThread.DISPATCH_NORMAL); - } + } else { + // Ensure tag folder gets processed last + nodes[0].children.sort(function sortRoots(aNode, bNode) { + return (aNode.root && aNode.root == "tagsFolder") ? 1 : + (bNode.root && bNode.root == "tagsFolder") ? -1 : 0; + }); - // Ensure tag folder gets processed last - nodes[0].children.sort(function sortRoots(aNode, bNode) { - return (aNode.root && aNode.root == "tagsFolder") ? 1 : - (bNode.root && bNode.root == "tagsFolder") ? -1 : 0; - }); - - let batch = { - nodes: nodes[0].children, - runBatched: function runBatched() { - if (aReplace) { - // Get roots excluded from the backup, we will not remove them - // before restoring. - let excludeItems = PlacesUtils.annotations.getItemsWithAnnotation( - PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO); - // Delete existing children of the root node, excepting: - // 1. special folders: delete the child nodes - // 2. tags folder: untag via the tagging api - let root = PlacesUtils.getFolderContents(PlacesUtils.placesRootId, + let batch = { + nodes: nodes[0].children, + runBatched: function runBatched() { + if (aReplace) { + // Get roots excluded from the backup, we will not remove them + // before restoring. + let excludeItems = PlacesUtils.annotations.getItemsWithAnnotation( + PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO); + // Delete existing children of the root node, excepting: + // 1. special folders: delete the child nodes + // 2. tags folder: untag via the tagging api + let root = PlacesUtils.getFolderContents(PlacesUtils.placesRootId, false, false).root; - let childIds = []; - for (let i = 0; i < root.childCount; i++) { - let childId = root.getChild(i).itemId; - if (excludeItems.indexOf(childId) == -1 && - childId != PlacesUtils.tagsFolderId) { - childIds.push(childId); - } - } - root.containerOpen = false; - - for (let i = 0; i < childIds.length; i++) { - let rootItemId = childIds[i]; - if (PlacesUtils.isRootItem(rootItemId)) { - PlacesUtils.bookmarks.removeFolderChildren(rootItemId); - } else { - PlacesUtils.bookmarks.removeItem(rootItemId); - } - } - } - - let searchIds = []; - let folderIdMap = []; - - batch.nodes.forEach(function(node) { - if (!node.children || node.children.length == 0) - return; // Nothing to restore for this root - - if (node.root) { - let container = PlacesUtils.placesRootId; // Default to places root - switch (node.root) { - case "bookmarksMenuFolder": - container = PlacesUtils.bookmarksMenuFolderId; - break; - case "tagsFolder": - container = PlacesUtils.tagsFolderId; - break; - case "unfiledBookmarksFolder": - container = PlacesUtils.unfiledBookmarksFolderId; - break; - case "toolbarFolder": - container = PlacesUtils.toolbarFolderId; - break; - } - - // Insert the data into the db - node.children.forEach(function(child) { - let index = child.index; - let [folders, searches] = - this.importJSONNode(child, container, index, 0); - for (let i = 0; i < folders.length; i++) { - if (folders[i]) - folderIdMap[i] = folders[i]; + let childIds = []; + for (let i = 0; i < root.childCount; i++) { + let childId = root.getChild(i).itemId; + if (excludeItems.indexOf(childId) == -1 && + childId != PlacesUtils.tagsFolderId) { + childIds.push(childId); } - searchIds = searchIds.concat(searches); - }.bind(this)); - } else { - this.importJSONNode( - node, PlacesUtils.placesRootId, node.index, 0); + } + root.containerOpen = false; + + for (let i = 0; i < childIds.length; i++) { + let rootItemId = childIds[i]; + if (PlacesUtils.isRootItem(rootItemId)) { + PlacesUtils.bookmarks.removeFolderChildren(rootItemId); + } else { + PlacesUtils.bookmarks.removeItem(rootItemId); + } + } } - }.bind(this)); - // Fixup imported place: uris that contain folders - searchIds.forEach(function(aId) { - let oldURI = PlacesUtils.bookmarks.getBookmarkURI(aId); - let uri = fixupQuery(oldURI, folderIdMap); - if (!uri.equals(oldURI)) { - PlacesUtils.bookmarks.changeBookmarkURI(aId, uri); - } - }); + let searchIds = []; + let folderIdMap = []; - Services.tm.mainThread.dispatch(function() { - deferred.resolve(); - }, Ci.nsIThread.DISPATCH_NORMAL); - }.bind(this) - }; + batch.nodes.forEach(function(node) { + if (!node.children || node.children.length == 0) + return; // Nothing to restore for this root - PlacesUtils.bookmarks.runInBatchMode(batch, null); + if (node.root) { + let container = PlacesUtils.placesRootId; // Default to places root + switch (node.root) { + case "bookmarksMenuFolder": + container = PlacesUtils.bookmarksMenuFolderId; + break; + case "tagsFolder": + container = PlacesUtils.tagsFolderId; + break; + case "unfiledBookmarksFolder": + container = PlacesUtils.unfiledBookmarksFolderId; + break; + case "toolbarFolder": + container = PlacesUtils.toolbarFolderId; + break; + } + + // Insert the data into the db + node.children.forEach(function(child) { + let index = child.index; + let [folders, searches] = + this.importJSONNode(child, container, index, 0); + for (let i = 0; i < folders.length; i++) { + if (folders[i]) + folderIdMap[i] = folders[i]; + } + searchIds = searchIds.concat(searches); + }.bind(this)); + } else { + this.importJSONNode( + node, PlacesUtils.placesRootId, node.index, 0); + } + }.bind(this)); + + // Fixup imported place: uris that contain folders + searchIds.forEach(function(aId) { + let oldURI = PlacesUtils.bookmarks.getBookmarkURI(aId); + let uri = fixupQuery(oldURI, folderIdMap); + if (!uri.equals(oldURI)) { + PlacesUtils.bookmarks.changeBookmarkURI(aId, uri); + } + }); + + Services.tm.mainThread.dispatch(function() { + deferred.resolve(); + }, Ci.nsIThread.DISPATCH_NORMAL); + }.bind(this) + }; + + PlacesUtils.bookmarks.runInBatchMode(batch, null); + } return deferred.promise; }, diff --git a/toolkit/components/places/tests/bookmarks/test_405938_restore_queries.js b/toolkit/components/places/tests/bookmarks/test_405938_restore_queries.js index ac5ba42dd9f0..f4921783d131 100644 --- a/toolkit/components/places/tests/bookmarks/test_405938_restore_queries.js +++ b/toolkit/components/places/tests/bookmarks/test_405938_restore_queries.js @@ -219,7 +219,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } // validate diff --git a/toolkit/components/places/tests/bookmarks/test_417228-exclude-from-backup.js b/toolkit/components/places/tests/bookmarks/test_417228-exclude-from-backup.js index eb5542188a55..5a876e74a386 100644 --- a/toolkit/components/places/tests/bookmarks/test_417228-exclude-from-backup.js +++ b/toolkit/components/places/tests/bookmarks/test_417228-exclude-from-backup.js @@ -132,7 +132,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } @@ -147,7 +147,7 @@ function run_test() { PlacesUtils.bookmarks.removeItem(test._excludeRootId); // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } diff --git a/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js b/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js index ec12dfddd678..f966953fc492 100644 --- a/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js +++ b/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js @@ -158,7 +158,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile, excludedItemsFromRestore); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } // validate diff --git a/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js b/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js index 280972857d84..1c7804e77a03 100644 --- a/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js +++ b/toolkit/components/places/tests/bookmarks/test_424958-json-quoted-folders.js @@ -88,7 +88,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } // validate diff --git a/toolkit/components/places/tests/bookmarks/test_448584.js b/toolkit/components/places/tests/bookmarks/test_448584.js index 2619a53642e9..4fd827e24dcc 100644 --- a/toolkit/components/places/tests/bookmarks/test_448584.js +++ b/toolkit/components/places/tests/bookmarks/test_448584.js @@ -110,7 +110,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } // validate diff --git a/toolkit/components/places/tests/bookmarks/test_458683.js b/toolkit/components/places/tests/bookmarks/test_458683.js index 0d942ea2e032..9eb658297d9f 100644 --- a/toolkit/components/places/tests/bookmarks/test_458683.js +++ b/toolkit/components/places/tests/bookmarks/test_458683.js @@ -131,7 +131,7 @@ function run_test() { // restore json file try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } // validate diff --git a/toolkit/components/places/tests/unit/test_384370.js b/toolkit/components/places/tests/unit/test_384370.js index 7abc2a698094..0e7a6b86c891 100644 --- a/toolkit/components/places/tests/unit/test_384370.js +++ b/toolkit/components/places/tests/unit/test_384370.js @@ -69,7 +69,7 @@ function run_test() { // 2. empty bookmarks db // 3. import bookmarks.exported.json try { - PlacesUtils.restoreBookmarksFromJSONFile(jsonFile); + yield BookmarkJSONUtils.importFromFile(jsonFile, true); } catch(ex) { do_throw("couldn't import the exported file: " + ex); } LOG("imported json"); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js b/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js index bc70f33d3d1f..43f885ea7a74 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_restore_notification.js @@ -54,7 +54,7 @@ var tests = [ yield BookmarkJSONUtils.exportToFile(this.file); remove_all_bookmarks(); try { - PlacesUtils.restoreBookmarksFromJSONFile(this.file); + yield BookmarkJSONUtils.importFromFile(this.file, true); } catch (e) { do_throw(" Restore should not have failed"); @@ -71,12 +71,14 @@ var tests = [ folderId: null, run: function () { this.file = createFile("bookmarks-test_restoreNotification.json"); - try { - PlacesUtils.restoreBookmarksFromJSONFile(this.file); - } - catch (e) { - do_throw(" Restore should not have failed"); - } + Task.spawn(function() { + try { + yield BookmarkJSONUtils.importFromFile(this.file, true); + } + catch (e) { + do_throw(" Restore should not have failed" + e); + } + }.bind(this)); } }, @@ -89,11 +91,14 @@ var tests = [ run: function () { this.file = Services.dirsvc.get("ProfD", Ci.nsILocalFile); this.file.append("this file doesn't exist because nobody created it"); - try { - PlacesUtils.restoreBookmarksFromJSONFile(this.file); - do_throw(" Restore should have failed"); - } - catch (e) {} + Task.spawn(function() { + try { + yield BookmarkJSONUtils.importFromFile(this.file, true); + do_throw(" Restore should have failed"); + } + catch (e) { + } + }.bind(this)); } },