Bug 1366702 - Invoke menus.create callback if error occurs + tests draft
authorRob Wu <rob@robwu.nl>
Fri, 20 Jul 2018 19:59:55 +0200
changeset 820929 9580e6ffe560fcfa97a7fe80a4f72b0d972956f7
parent 820804 4f12d77b4f9b6adaf06615c1c8cdc14de836dc1a
push id116992
push userbmo:rob@robwu.nl
push dateFri, 20 Jul 2018 18:36:50 +0000
bugs1366702
milestone63.0a1
Bug 1366702 - Invoke menus.create callback if error occurs + tests MozReview-Commit-ID: 6TuxyXVtWrI
browser/components/extensions/child/ext-menus.js
browser/components/extensions/test/browser/browser-common.ini
browser/components/extensions/test/browser/browser_ext_menus_errors.js
--- a/browser/components/extensions/child/ext-menus.js
+++ b/browser/components/extensions/child/ext-menus.js
@@ -119,18 +119,24 @@ this.menusInternal = class extends Exten
           delete createProperties.onclick;
           context.childManager.callParentAsyncFunction("menusInternal.create", [
             createProperties,
           ]).then(() => {
             if (onclick) {
               onClickedProp.setListener(createProperties.id, onclick);
             }
             if (callback) {
-              callback();
+              context.runSafeWithoutClone(callback);
             }
+          }).catch(error => {
+            context.withLastError(error, null, () => {
+              if (callback) {
+                context.runSafeWithoutClone(callback);
+              }
+            });
           });
           return createProperties.id;
         },
 
         update(id, updateProperties) {
           let {onclick} = updateProperties;
           delete updateProperties.onclick;
           return context.childManager.callParentAsyncFunction("menusInternal.update", [
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -98,16 +98,17 @@ support-files =
 skip-if = (verify && (os == 'linux' || os == 'mac'))
 [browser_ext_getViews.js]
 [browser_ext_history_redirect.js]
 [browser_ext_identity_indication.js]
 [browser_ext_incognito_views.js]
 [browser_ext_incognito_popup.js]
 [browser_ext_lastError.js]
 [browser_ext_menus.js]
+[browser_ext_menus_errors.js]
 [browser_ext_menus_event_order.js]
 [browser_ext_menus_events.js]
 [browser_ext_menus_refresh.js]
 [browser_ext_omnibox.js]
 skip-if = (debug && (os == 'linux' || os == 'mac')) || (verify && (os == 'linux' || os == 'mac')) # Bug 1417052
 [browser_ext_openPanel.js]
 skip-if = (verify && !debug && (os == 'linux' || os == 'mac'))
 [browser_ext_optionsPage_browser_style.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_menus_errors.js
@@ -0,0 +1,94 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+"use strict";
+
+add_task(async function test_create_error() {
+  // lastError is the only means to communicate errors in the menus.create API,
+  // so make sure that a warning is logged to the console if the error is not
+  // checked.
+  let waitForConsole = new Promise(resolve => {
+    SimpleTest.waitForExplicitFinish();
+    SimpleTest.monitorConsole(resolve, [
+      // Callback exists, lastError is checked. Should not be logged.
+      {message: /Unchecked lastError value: Error: ID already exists: some_id/, forbid: true},
+      // No callback, lastError not checked. Should be logged.
+      {message: /Unchecked lastError value: Error: Could not find any MenuItem with id: noCb/},
+      // Callback exists, lastError not checked. Should be logged.
+      {message: /Unchecked lastError value: Error: Could not find any MenuItem with id: cbIgnoreError/},
+    ]);
+  });
+
+  async function background() {
+    // Note: browser.menus.create returns the menu ID instead of a promise, so
+    // we have to use callbacks.
+    await new Promise(resolve => {
+      browser.menus.create({id: "some_id", title: "menu item"}, () => {
+        browser.test.assertEq(null, browser.runtime.lastError, "Expected no error");
+        resolve();
+      });
+    });
+
+    // Callback exists, lastError is checked:
+    await new Promise(resolve => {
+      browser.menus.create({id: "some_id", title: "menu item"}, () => {
+        browser.test.assertEq("ID already exists: some_id", browser.runtime.lastError.message, "Expected error");
+        resolve();
+      });
+    });
+
+    // No callback, lastError not checked:
+    browser.menus.create({id: "noCb", parentId: "noCb", title: "menu item"});
+
+    // Callback exists, lastError not checked:
+    await new Promise(resolve => {
+      browser.menus.create({id: "cbIgnoreError", parentId: "cbIgnoreError", title: "menu item"}, () => {
+        resolve();
+      });
+    });
+
+    // Do another roundtrip with the menus API to ensure that any console
+    // error messages from the previous call are flushed.
+    await browser.menus.removeAll();
+
+    browser.test.sendMessage("done");
+  }
+
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {permissions: ["menus"]},
+    background,
+  });
+  await extension.startup();
+  await extension.awaitMessage("done");
+
+  await extension.unload();
+
+  SimpleTest.endMonitorConsole();
+  await waitForConsole;
+});
+
+add_task(async function test_update_error() {
+  async function background() {
+    const id = browser.menus.create({title: "menu item"});
+
+    await browser.test.assertRejects(
+      browser.menus.update(id, {parentId: "bogus"}),
+      "Could not find any MenuItem with id: bogus",
+      "menus.update with invalid parentMenuId should fail");
+
+    await browser.test.assertRejects(
+      browser.menus.update(id, {parentId: id}),
+      "MenuItem cannot be an ancestor (or self) of its new parent.",
+      "menus.update cannot assign itself as the parent of a menu.");
+
+    browser.test.sendMessage("done");
+  }
+
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {permissions: ["menus"]},
+    background,
+  });
+  await extension.startup();
+  await extension.awaitMessage("done");
+  await extension.unload();
+});