diff --git a/browser/components/extensions/ext-bookmarks.js b/browser/components/extensions/ext-bookmarks.js index af6fa1207e42..c1d759dd0b29 100644 --- a/browser/components/extensions/ext-bookmarks.js +++ b/browser/components/extensions/ext-bookmarks.js @@ -8,9 +8,6 @@ Cu.import("resource://gre/modules/PlacesUtils.jsm"); var Bookmarks = PlacesUtils.bookmarks; Cu.import("resource://gre/modules/ExtensionUtils.jsm"); -var { - runSafe, -} = ExtensionUtils; XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm"); @@ -85,58 +82,38 @@ function convert(result) { extensions.registerSchemaAPI("bookmarks", "bookmarks", (extension, context) => { return { bookmarks: { - get: function(idOrIdList, callback) { + get: function(idOrIdList) { let list = Array.isArray(idOrIdList) ? idOrIdList : [idOrIdList]; - Task.spawn(function* () { + return Task.spawn(function* () { let bookmarks = []; for (let id of list) { - let bookmark; - try { - bookmark = yield Bookmarks.fetch({guid: id}); - if (!bookmark) { - // TODO: set lastError, not found - return []; - } - } catch (e) { - // TODO: set lastError, probably an invalid guid - return []; + let bookmark = yield Bookmarks.fetch({guid: id}); + if (!bookmark) { + throw new Error("Bookmark not found"); } bookmarks.push(convert(bookmark)); } return bookmarks; - }).then(results => runSafe(context, callback, results)); + }); }, - getChildren: function(id, callback) { + getChildren: function(id) { // TODO: We should optimize this. - getTree(id, true).then(result => { - runSafe(context, callback, result); - }, reason => { - // TODO: Set lastError - runSafe(context, callback, []); - }); + return getTree(id, true); }, - getTree: function(callback) { - getTree(Bookmarks.rootGuid, false).then(result => { - runSafe(context, callback, result); - }, reason => { - runSafe(context, callback, []); - }); + getTree: function() { + return getTree(Bookmarks.rootGuid, false); }, - getSubTree: function(id, callback) { - getTree(id, false).then(result => { - runSafe(context, callback, result); - }, reason => { - runSafe(context, callback, []); - }); + getSubTree: function(id) { + return getTree(id, false); }, // search - create: function(bookmark, callback) { + create: function(bookmark) { let info = { title: bookmark.title || "", }; @@ -159,25 +136,14 @@ extensions.registerSchemaAPI("bookmarks", "bookmarks", (extension, context) => { info.parentGuid = Bookmarks.unfiledGuid; } - let failure = reason => { - // TODO: set lastError. - if (callback) { - runSafe(context, callback, null); - } - }; - try { - Bookmarks.insert(info).then(result => { - if (callback) { - runSafe(context, callback, convert(result)); - } - }, failure); + return Bookmarks.insert(info).then(convert); } catch (e) { - failure(e); + return Promise.reject({ message: `Invalid bookmark: ${JSON.stringify(info)}` }); } }, - move: function(id, destination, callback) { + move: function(id, destination) { let info = { guid: id, }; @@ -189,24 +155,14 @@ extensions.registerSchemaAPI("bookmarks", "bookmarks", (extension, context) => { info.index = destination.index; } - let failure = reason => { - if (callback) { - runSafe(context, callback, null); - } - }; - try { - Bookmarks.update(info).then(result => { - if (callback) { - runSafe(context, callback, convert(result)); - } - }, failure); + return Bookmarks.update(info).then(convert); } catch (e) { - failure(e); + return Promise.reject({ message: `Invalid bookmark: ${JSON.stringify(info)}` }); } }, - update: function(id, changes, callback) { + update: function(id, changes) { let info = { guid: id, }; @@ -218,47 +174,26 @@ extensions.registerSchemaAPI("bookmarks", "bookmarks", (extension, context) => { info.url = changes.url; } - let failure = reason => { - if (callback) { - runSafe(context, callback, null); - } - }; - try { - Bookmarks.update(info).then(result => { - if (callback) { - runSafe(context, callback, convert(result)); - } - }, failure); + return Bookmarks.update(info).then(convert); } catch (e) { - failure(e); + return Promise.reject({ message: `Invalid bookmark: ${JSON.stringify(info)}` }); } }, - remove: function(id, callback) { + remove: function(id) { let info = { guid: id, }; - let failure = reason => { - if (callback) { - runSafe(context, callback, null); - } - }; - + // The API doesn't give you the old bookmark at the moment try { - Bookmarks.remove(info).then(result => { - if (callback) { - // The API doesn't give you the old bookmark at the moment - runSafe(context, callback); - } - }, failure); + return Bookmarks.remove(info).then(result => {}); } catch (e) { - failure(e); + return Promise.reject({ message: `Invalid bookmark: ${JSON.stringify(info)}` }); } }, }, }; }); - diff --git a/browser/components/extensions/schemas/bookmarks.json b/browser/components/extensions/schemas/bookmarks.json index 0c07aa8d9846..09f67c263639 100644 --- a/browser/components/extensions/schemas/bookmarks.json +++ b/browser/components/extensions/schemas/bookmarks.json @@ -109,6 +109,7 @@ "name": "get", "type": "function", "description": "Retrieves the specified BookmarkTreeNode(s).", + "async": "callback", "parameters": [ { "name": "idOrIdList", @@ -143,6 +144,7 @@ "name": "getChildren", "type": "function", "description": "Retrieves the children of the specified BookmarkTreeNode id.", + "async": "callback", "parameters": [ { "type": "string", @@ -166,6 +168,7 @@ "unsupported": true, "type": "function", "description": "Retrieves the recently added bookmarks.", + "async": "callback", "parameters": [ { "type": "integer", @@ -190,6 +193,7 @@ "name": "getTree", "type": "function", "description": "Retrieves the entire Bookmarks hierarchy.", + "async": "callback", "parameters": [ { "type": "function", @@ -208,6 +212,7 @@ "name": "getSubTree", "type": "function", "description": "Retrieves part of the Bookmarks hierarchy, starting at the specified node.", + "async": "callback", "parameters": [ { "type": "string", @@ -232,6 +237,7 @@ "unsupported": true, "type": "function", "description": "Searches for BookmarkTreeNodes matching the given query. Queries specified with an object produce BookmarkTreeNodes matching all specified properties.", + "async": "callback", "parameters": [ { "name": "query", @@ -281,6 +287,7 @@ "name": "create", "type": "function", "description": "Creates a bookmark or folder under the specified parentId. If url is NULL or missing, it will be a folder.", + "async": "callback", "parameters": [ { "$ref": "CreateDetails", @@ -303,6 +310,7 @@ "name": "move", "type": "function", "description": "Moves the specified BookmarkTreeNode to the provided location.", + "async": "callback", "parameters": [ { "type": "string", @@ -340,6 +348,7 @@ "name": "update", "type": "function", "description": "Updates the properties of a bookmark or folder. Specify only the properties that you want to change; unspecified properties will be left unchanged. Note: Currently, only 'title' and 'url' are supported.", + "async": "callback", "parameters": [ { "type": "string", @@ -376,6 +385,7 @@ "name": "remove", "type": "function", "description": "Removes a bookmark or an empty bookmark folder.", + "async": "callback", "parameters": [ { "type": "string", @@ -394,6 +404,7 @@ "unsupported": true, "type": "function", "description": "Recursively removes a bookmark folder.", + "async": "callback", "parameters": [ { "type": "string", @@ -412,6 +423,7 @@ "unsupported": true, "type": "function", "description": "Imports bookmarks from an html bookmark file", + "async": "callback", "parameters": [ { "type": "function", @@ -426,6 +438,7 @@ "unsupported": true, "type": "function", "description": "Exports bookmarks to an html bookmark file", + "async": "callback", "parameters": [ { "type": "function", diff --git a/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html b/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html index 192702e49427..4d4a294d3658 100644 --- a/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html +++ b/toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html @@ -14,120 +14,82 @@ "use strict"; function backgroundScript() { - function get(idOrIdList) { - return new Promise(resolve => { - browser.bookmarks.get(idOrIdList, resolve); - }); - } - - function create(bookmark) { - return new Promise(resolve => { - browser.bookmarks.create(bookmark, resolve); - }); - } - - function getChildren(id) { - return new Promise(resolve => { - browser.bookmarks.getChildren(id, resolve); - }); - } - - function update(id, changes) { - return new Promise(resolve => { - browser.bookmarks.update(id, changes, resolve); - }); - } - - function getTree(id) { - return new Promise(resolve => { - browser.bookmarks.getTree(resolve); - }); - } - - function remove(idOrIdList) { - return new Promise(resolve => { - browser.bookmarks.remove(idOrIdList, resolve); - }); - } - let unsortedId, ourId; function checkOurBookmark(bookmark) { - browser.test.assertEq(bookmark.id, ourId); + browser.test.assertEq(ourId, bookmark.id); browser.test.assertTrue("parentId" in bookmark); - browser.test.assertEq(bookmark.index, 0); // We assume there are no other bookmarks. - browser.test.assertEq(bookmark.url, "http://example.org/"); - browser.test.assertEq(bookmark.title, "test bookmark"); + browser.test.assertEq(0, bookmark.index); // We assume there are no other bookmarks. + browser.test.assertEq("http://example.org/", bookmark.url); + browser.test.assertEq("test bookmark", bookmark.title); browser.test.assertTrue("dateAdded" in bookmark); browser.test.assertFalse("dateGroupModified" in bookmark); browser.test.assertFalse("unmodifiable" in bookmark); } - get(["not-a-bookmark-guid"]).then(result => { - // TODO: check lastError - browser.test.assertEq(result.length, 0, "invalid bookmark guid returned nothing"); - return get(["000000000000"]); + let failures = 0; + let tallyFailure = error => { + browser.test.succeed(`Got expected error: ${error}`); + failures++; + }; + + browser.bookmarks.get(["not-a-bookmark-guid"]).catch(tallyFailure).then(result => { + return browser.bookmarks.get(["000000000000"]).catch(tallyFailure); }).then(results => { - // TODO: check lastError - browser.test.assertEq(results.length, 0, "correctly did not find bookmark"); - return create({title: "test bookmark", url: "http://example.org"}); + return browser.bookmarks.create({title: "test bookmark", url: "http://example.org"}); }).then(result => { ourId = result.id; checkOurBookmark(result); - return get(ourId); + return browser.bookmarks.get(ourId); }).then(results => { browser.test.assertEq(results.length, 1); checkOurBookmark(results[0]); unsortedId = results[0].parentId; - return get(unsortedId); + return browser.bookmarks.get(unsortedId); }).then(results => { let folder = results[0]; browser.test.assertEq(results.length, 1); - browser.test.assertEq(folder.id, unsortedId); + browser.test.assertEq(unsortedId, folder.id); browser.test.assertTrue("parentId" in folder); browser.test.assertTrue("index" in folder); browser.test.assertFalse("url" in folder); - browser.test.assertEq(folder.title, "Unsorted Bookmarks"); + browser.test.assertEq("Unsorted Bookmarks", folder.title); browser.test.assertTrue("dateAdded" in folder); browser.test.assertTrue("dateGroupModified" in folder); browser.test.assertFalse("unmodifiable" in folder); // TODO: Do we want to enable this? - return getChildren(unsortedId); + return browser.bookmarks.getChildren(unsortedId); }).then(results => { - browser.test.assertEq(results.length, 1); + browser.test.assertEq(1, results.length); checkOurBookmark(results[0]); - return update(ourId, {title: "new test title"}); + return browser.bookmarks.update(ourId, {title: "new test title"}); }).then(result => { - browser.test.assertEq(result.title, "new test title"); - browser.test.assertEq(result.id, ourId); + browser.test.assertEq("new test title", result.title); + browser.test.assertEq(ourId, result.id); - return getTree(); + return browser.bookmarks.getTree(); }).then(results => { - browser.test.assertEq(results.length, 1); + browser.test.assertEq(1, results.length); let bookmark = results[0].children.find(bookmark => bookmark.id == unsortedId); - browser.test.assertEq(bookmark.title, "Unsorted Bookmarks"); + browser.test.assertEq("Unsorted Bookmarks", bookmark.title); - return create({parentId: "invalid"}); + return browser.bookmarks.create({parentId: "invalid"}).catch(tallyFailure); }).then(result => { - // TODO: Check lastError - browser.test.assertEq(result, null); - - return remove(ourId); + return browser.bookmarks.remove(ourId); }).then(() => { - return get(ourId); + return browser.bookmarks.get(ourId).catch(tallyFailure); }).then(results => { - // TODO: Check lastError - browser.test.assertEq(results.length, 0); + return browser.bookmarks.remove("000000000000").catch(tallyFailure); + }).then(() => { + browser.test.assertEq(5, failures, "Expected failures"); - return remove("000000000000"); - }).then(() => { - // TODO: Check lastError - }).then(() => { browser.test.notifyPass("bookmarks"); + }).catch(error => { + browser.test.fail(`Error: ${String(error)} :: ${error.stack}`); }); }