Closed
Bug 1131818
Opened 10 years ago
Closed 10 years ago
[e10s] Add AsyncUtils, an e10s version of EventUtils
Categories
(Testing :: Mochitest, defect)
Tracking
(e10s+, firefox40 fixed)
People
(Reporter: billm, Assigned: enndeakin)
References
Details
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #8562379 -
Flags: review?(mconley)
|
||
Updated•10 years ago
|
tracking-e10s:
--- → +
|
||
Comment 1•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
Flags: needinfo?(wmccloskey)
|
|
||
Updated•10 years ago
|
Attachment #8562379 -
Flags: review?(mconley)
|
Assignee | |
Comment 4•10 years ago
|
||
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.
|
|
Assignee | |
Comment 5•10 years ago
|
||
Assignee: wmccloskey → enndeakin
Attachment #8562379 -
Attachment is obsolete: true
Attachment #8589685 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(wmccloskey)
Attachment #8590929 -
Flags: review?(mconley)
|
|
||
Comment 6•10 years ago
|
||
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.
Attachment #8590929 -
Flags: review?(mconley) → review+
|
||
Comment 8•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
|
||
Comment 9•10 years ago
|
||
Hi Neil, can you provide a point value.
Iteration: --- → 40.2 - 27 Apr
Flags: needinfo?(enndeakin)
Flags: firefox-backlog+
|
|
Assignee | |
Updated•10 years ago
|
Points: --- → 3
Flags: needinfo?(enndeakin)
You need to log in
before you can comment on or make changes to this bug.
Description
•