1131818 - [e10s] Add AsyncUtils, an e10s version of EventUtils

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: async-utils

It's currently possible to use EventUtils in e10s tests, but it only sorta works. Event dispatch is disabled from CPOWs, so functions like synthesizeMouseAtPoint don't actually work.

This patch adds async versions of these methods. The async methods don't wait for the event to actually be dispatched. For a lot of tests, that's fine.

Comment 1

[Dave Townsend [:mossop]]

It might be useful to have the methods return a promise that is resolved after the event has been dispatched. Doing this by sending an async method from the child after dispatching would mean any messages sent by event listeners in content would be dispatched first.

Comment 2

[Steven MacLeod [:smacleod]]

Bill, I'm working on something slightly related right now in Bug 1093566. Currently I'm loading a "BTU" (BrowserTestUtils) variable into the scope which will contain a lot of the methods present in the head.js files scattered throughout the tree. Example snippet:
> BrowserTestUtils.prototype = {
>   /**
>    * @param browser A xul:browser.
>    *
>    * @param ignoreSubFrames A boolean indicating if loads from subframes
>    *                        should be ignored.
>    *
>    * @return A Promise which resolves when a load event is triggered
>    *         for browser.
>    */
>   promiseBrowserLoaded(browser, ignoreSubFrames=true) {
>     return new Promise(resolve => {
>       browser.messageManager.addMessageListener("browser-test-utils:loadEvent",
>                                                  function onLoad(msg) {
>         if (!ignoreSubFrames || !msg.data.subframe) {
>           browser.messageManager.removeMessageListener(
>             "browser-test-utils:loadEvent", onLoad);
>           resolve();
>         }
>       });
>     });
>   },

I'm wondering if it'd be worth combining these into the same library, or if you think they should just be separate.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Gonna hold off on review until we hear back from billm on comment 1 and comment 2.

Comment 4

[Neil Deakin]

Attachment: Updated patch

This version implements this with returning a promise that resolves when the mouse event is finished. I also just put it in BrowserTestUtils.jsm instead of a new file. I changed the syntax of this version of synthesizeMouse so that it can also take a selector string instead of an element to avoid using wrappers.

I'm going to try it out on a few tests before reviewing.

Comment 5

[Neil Deakin]

Attachment: Updated patch, improve the test a bit

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8590929 [details] [diff] [review]
Updated patch, improve the test a bit

Review of attachment 8590929 [details] [diff] [review]:
-----------------------------------------------------------------

My only major concern is the openNewForegroundTab bit - I'm not familiar with that method, and dxr is coming up empty. If you swap withNewTab for that, I think you're all good here, assuming the test passes.

::: testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ +282,5 @@
> +   *  Versions of EventUtils.jsm synthesizeMouse functions that synthesize a
> +   *  mouse event in a child process and return promises that resolve when the
> +   *  event has fired and completed. Instead of a window, a browser is required
> +   *  to be passed to this function.
> +   * 

Nit - trailing whitespace. Same on lines 295 and 297.

@@ +321,5 @@
> +                          {object: cpowObject});
> +    });
> +  },
> +
> +  synthesizeMouseAtCenter(target, event, browser)

Nit - let's add a brief comment that the caller should refer to the synthesizeMouse documentation for arguments.

@@ +328,5 @@
> +    event.centered = true;
> +    return BrowserTestUtils.synthesizeMouse(target, 0, 0, event, browser);
> +  },
> +
> +  synthesizeMouseAtPoint(offsetX, offsetY, event, browser)

Same as above.

::: testing/mochitest/tests/SimpleTest/AsyncUtilsContent.js
@@ +25,5 @@
> +
> +  let left = data.x;
> +  let top = data.y;
> +  if (target) {
> +    var rect = target.getBoundingClientRect();

let, not var

::: testing/mochitest/tests/browser/browser_BrowserTestUtils.js
@@ +6,5 @@
> +}
> +
> +add_task(function* () {
> +  let onClickEvt = 'document.getElementById("out").textContent = event.target.localName + "," + event.clientX + "," + event.clientY;'
> +  const url = "<body onclick='" + onClickEvt + "' style='margin: 0'>" +

We could probably make this more readable with a template string - example:

const url = `
<body onclick="${onClickEvt}" style="margin: 0;">
  <button id="one" style="margin: 0; margin-left: 16px; margin-top: 15px; width: 30px; height: 40px;">
    Test
  </button>
  <div onmousedown="event.preventDefault();" style="margin: 0; width: 80px; height: 60px;">
    Other
  </div>
  <span id="out"></span>
</body>`;

@@ +10,5 @@
> +  const url = "<body onclick='" + onClickEvt + "' style='margin: 0'>" +
> +              "<button id='one' style='margin: 0; margin-left: 16px; margin-top: 14px; width: 30px; height: 40px;'>Test</button>" +
> +              "<div onmousedown='event.preventDefault()' style='margin: 0; width: 80px; height: 60px;'>Other</div>" +
> +              "<span id='out'></span></body>";
> +  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "data:text/html," + url);

I'm not sure where openNewForegroundTab is defined...

Since you don't care about the tab itself, you might want to use BrowserTestUtils.withNewTab, which will open and close the tab for you, along with making sure the tab is loaded.

@@ +34,5 @@
> +  yield BrowserTestUtils.synthesizeMouseAtCenter("body > div", {}, browser);
> +  details = yield getLastEventDetails(browser);
> +  is(details, "div,40,84", "synthesizeMouseAtCenter with complex selector");
> +
> +  let result = yield BrowserTestUtils.synthesizeMouseAtCenter("body > div", { type: "mousedown" }, browser);

Let's rename result to "cancelled", to make it clearer what we're getting back.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ddadc870ef6

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4ddadc870ef6

Comment 9

[Marco Mucci [:MarcoM]]

Hi Neil, can you provide a point value.