Commit de8eb2f4 authored by Rob Wu's avatar Rob Wu
Browse files

Bug 1373234 - Avoid races in tests involving menu creation r=mixedpuppy 

browser.menus.create and browser.contextMenus.create are asynchronous
APIs. For compatibility reasons, they cannot return a promise (since
they already return an integer).

This modifies all tests that use either of these APIs, to ensure that
the test waits for the callback of the last menus/contextMenus.create
call before continuing with the test.

In some cases in browser_ext_menus.js I did not add any callbacks,
because there were other asynchronous API calls (browser.tabs API)
that already guaranteed that the test made a round-trip to the parent
process before continuing.

This fixes:
- Bug 1462862 (browser_ext_contextMenus_icons.js)
- Bug 1403429 (browser_ext_contextMenus_onclick.js)
- Bug 1321182 (browser_ext_contextMenus.js)
- Bug 1373234 (browser_ext_menus.js)

MozReview-Commit-ID: IZFUyIw8Tbl

--HG--
extra : rebase_source : 4f28acf0189b4b93808b13200cf966a04873bf78
parent c97eabf9

browser/components/extensions/test/browser/browser_ext_contextMenus.js

+13 −10
Original line number Diff line number Diff line
@@ -20,13 +20,14 @@ add_task(async function() { 
        title: "Click me!",

        contexts: ["image"],

      });

      browser.contextMenus.onClicked.addListener(() => {});

      browser.contextMenus.create({

        id: "clickme-page",

        title: "Click me!",

        contexts: ["page"],

      });

      browser.contextMenus.onClicked.addListener(() => {});

      }, () => {

        browser.test.notifyPass();

      });

    },

  });
@@ -530,10 +531,6 @@ add_task(async function test_bookmark_contextmenu() { 
        title,

        parentId: "toolbar_____",

      });

      await browser.contextMenus.create({

        title: "Get bookmark",

        contexts: ["bookmark"],

      });

      browser.contextMenus.onClicked.addListener(async (info) => {

        browser.test.assertEq(newBookmark.id, info.bookmarkId, "Bookmark ID matches");
@@ -544,7 +541,12 @@ add_task(async function test_bookmark_contextmenu() { 
        await browser.bookmarks.remove(info.bookmarkId);

        browser.test.sendMessage("test-finish");

      });

      browser.contextMenus.create({

        title: "Get bookmark",

        contexts: ["bookmark"],

      }, () => {

        browser.test.sendMessage("bookmark-created");

      });

    },

  });

  await extension.startup();
@@ -569,12 +571,13 @@ add_task(async function test_bookmark_context_requires_permission() { 
    manifest: {

      permissions: ["contextMenus"],

    },

    async background() {

      await browser.contextMenus.create({

    background() {

      browser.contextMenus.create({

        title: "Get bookmark",

        contexts: ["bookmark"],

      });

      }, () => {

        browser.test.sendMessage("bookmark-created");

      });

    },

  });

  await extension.startup();

browser/components/extensions/test/browser/browser_ext_contextMenus_icons.js

+13 −13
Original line number Diff line number Diff line
@@ -34,9 +34,9 @@ add_task(async function test_root_icon() { 


      browser.contextMenus.create({

        title: "child",

      });



      }, () => {

        browser.test.notifyPass("contextmenus-icons");

      });

    },

  });
@@ -104,16 +104,6 @@ add_task(async function test_child_icon() { 
    },



    background: function() {

      browser.contextMenus.create({

        title: "child1",

        id: "contextmenu-child1",

        icons: {

          18: "red_icon.png",

        },

      });



      browser.test.sendMessage("single-contextmenu-item-added");



      browser.test.onMessage.addListener(msg => {

        if (msg !== "add-additional-contextmenu-items") {

          return;
@@ -133,9 +123,19 @@ add_task(async function test_child_icon() { 
          icons: {

            18: "green_icon.png",

          },

        }, () => {

          browser.test.notifyPass("extra-contextmenu-items-added");

        });

      });



        browser.test.notifyPass("extra-contextmenu-items-added");

      browser.contextMenus.create({

        title: "child1",

        id: "contextmenu-child1",

        icons: {

          18: "red_icon.png",

        },

      }, () => {

        browser.test.sendMessage("single-contextmenu-item-added");

      });

    },

  });

browser/components/extensions/test/browser/browser_ext_contextMenus_onclick.js

+3 −2
Original line number Diff line number Diff line
@@ -205,8 +205,9 @@ add_task(async function test_onclick_modifiers() { 
    function onclick(info) {

      browser.test.sendMessage("click", info);

    }

    browser.contextMenus.create({contexts: ["all"], title: "modify", onclick});

    browser.contextMenus.create({contexts: ["all"], title: "modify", onclick}, () => {

      browser.test.sendMessage("ready");

    });

  }



  const extension = ExtensionTestUtils.loadExtension({manifest, background});

browser/components/extensions/test/browser/browser_ext_menus.js

+33 −29
Original line number Diff line number Diff line
@@ -57,11 +57,11 @@ add_task(async function test_actionContextMenus() { 
    const contexts = ["page_action", "browser_action"];



    const parentId = browser.menus.create({contexts, title: "parent"});

    await browser.menus.create({parentId, title: "click A"});

    await browser.menus.create({parentId, title: "click B"});

    browser.menus.create({parentId, title: "click A"});

    browser.menus.create({parentId, title: "click B"});



    for (let i = 1; i < 9; i++) {

      await browser.menus.create({contexts, id: `${i}`, title: `click ${i}`});

      browser.menus.create({contexts, id: `${i}`, title: `click ${i}`});

    }



    browser.menus.onClicked.addListener((info, tab) => {
@@ -120,11 +120,11 @@ add_task(async function test_hiddenPageActionContextMenu() { 
    const contexts = ["page_action"];



    const parentId = browser.menus.create({contexts, title: "parent"});

    await browser.menus.create({parentId, title: "click A"});

    await browser.menus.create({parentId, title: "click B"});

    browser.menus.create({parentId, title: "click A"});

    browser.menus.create({parentId, title: "click B"});



    for (let i = 1; i < 9; i++) {

      await browser.menus.create({contexts, id: `${i}`, title: `click ${i}`});

      browser.menus.create({contexts, id: `${i}`, title: `click ${i}`});

    }



    const [tab] = await browser.tabs.query({active: true});
@@ -171,12 +171,13 @@ add_task(async function test_bookmarkContextMenu() { 
    manifest: {

      permissions: ["menus", "bookmarks"],

    },

    async background() {

      await browser.menus.create({title: "blarg", contexts: ["bookmark"]});

    background() {

      browser.menus.onShown.addListener(() => {

        browser.test.sendMessage("hello");

      });

      browser.menus.create({title: "blarg", contexts: ["bookmark"]}, () => {

        browser.test.sendMessage("ready");

      });

    },

  });
@@ -202,14 +203,14 @@ add_task(async function test_tabContextMenu() { 
      permissions: ["menus"],

    },

    async background() {

      await browser.menus.create({

      browser.menus.create({

        id: "alpha-beta-parent", title: "alpha-beta parent", contexts: ["tab"],

      });



      await browser.menus.create({parentId: "alpha-beta-parent", title: "alpha"});

      await browser.menus.create({parentId: "alpha-beta-parent", title: "beta"});

      browser.menus.create({parentId: "alpha-beta-parent", title: "alpha"});

      browser.menus.create({parentId: "alpha-beta-parent", title: "beta"});



      await browser.menus.create({title: "dummy", contexts: ["page"]});

      browser.menus.create({title: "dummy", contexts: ["page"]});



      browser.menus.onClicked.addListener((info, tab) => {

        browser.test.sendMessage("click", {info, tab});
@@ -224,9 +225,10 @@ add_task(async function test_tabContextMenu() { 
    manifest: {

      permissions: ["menus"],

    },

    async background() {

      await browser.menus.create({title: "gamma", contexts: ["tab"]});

    background() {

      browser.menus.create({title: "gamma", contexts: ["tab"]}, () => {

        browser.test.sendMessage("ready");

      });

    },

  });
@@ -279,8 +281,9 @@ add_task(async function test_onclick_frameid() { 
    function onclick(info) {

      browser.test.sendMessage("click", info);

    }

    browser.menus.create({contexts: ["frame", "page"], title: "modify", onclick});

    browser.menus.create({contexts: ["frame", "page"], title: "modify", onclick}, () => {

      browser.test.sendMessage("ready");

    });

  }



  const extension = ExtensionTestUtils.loadExtension({manifest, background});
@@ -313,20 +316,20 @@ add_task(async function test_multiple_contexts_init() { 
  };



  function background() {

    browser.menus.create({id: "parent", title: "parent"});

    browser.menus.create({id: "parent", title: "parent"}, () => {

      browser.tabs.create({url: "tab.html", active: false});

    });

  }



  const files = {

    "tab.html": "<!DOCTYPE html><meta charset=utf-8><script src=tab.js></script>",

    "tab.js": function() {

      browser.menus.create({parentId: "parent", id: "child", title: "child"});



      browser.menus.onClicked.addListener(info => {

        browser.test.sendMessage("click", info);

      });



      browser.menus.create({parentId: "parent", id: "child", title: "child"}, () => {

        browser.test.sendMessage("ready");

      });

    },

  };
@@ -358,10 +361,11 @@ add_task(async function test_tools_menu() { 
    manifest: {

      permissions: ["menus"],

    },

    async background() {

      await browser.menus.create({title: "alpha", contexts: ["tools_menu"]});

      await browser.menus.create({title: "beta", contexts: ["tools_menu"]});

    background() {

      browser.menus.create({title: "alpha", contexts: ["tools_menu"]});

      browser.menus.create({title: "beta", contexts: ["tools_menu"]}, () => {

        browser.test.sendMessage("ready");

      });

    },

  });
@@ -370,7 +374,7 @@ add_task(async function test_tools_menu() { 
      permissions: ["menus"],

    },

    async background() {

      await browser.menus.create({title: "gamma", contexts: ["tools_menu"]});

      browser.menus.create({title: "gamma", contexts: ["tools_menu"]});

      browser.menus.onClicked.addListener((info, tab) => {

        browser.test.sendMessage("click", {info, tab});

      });