Add BrowserTestUtils.withNewTab

RESOLVED FIXED in Firefox 39

Status

Testing
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

unspecified
mozilla39
Points:
1
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
This is an utility function to execute a test on a web page.
(Assignee)

Updated

3 years ago
Blocks: 1143903
(Assignee)

Comment 1

3 years ago
Created attachment 8579604 [details] [diff] [review]
The patch

The first user would be the login tests:

yield BrowserTestUtils.withNewTab({
  gBrowser,
  url: "https://example.com/browser/toolkit/components/" +
       "passwordmgr/test/browser/form_basic.html",
}, function* (browser) {
  // ...
  yield ContentTask.spawn(browser, null, function* () { /*...*/ });
});
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8579604 - Flags: review?(smacleod)
(Assignee)

Updated

3 years ago
Iteration: --- → 39.2 - 23 Mar
Points: --- → 1
Flags: firefox-backlog+
Comment on attachment 8579604 [details] [diff] [review]
The patch

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

::: testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ +52,5 @@
> +   */
> +  withNewTab: Task.async(function* (options, taskFn) {
> +    // In theory we may be subject to a race condition here, since we cannot set
> +    // up the browserLoaded listener before calling addTab. In practice this is
> +    // how most tests currently do their setup, so this is expected to work.

So I believe this is guaranteed to work because the way we're listening for the load is in the content-utils.js framescript, and then sending an async message up. Since we don't yield the event loop before setting up our listener for that message (in browserLoaded) we can't miss it.

That being said I don't think we need this comment about a possible race condition. What I said above should probably be documented in browserLoaded though.

If you still think this comment is needed, I'm fine with that too though.
Attachment #8579604 - Flags: review?(smacleod) → review+
(Assignee)

Comment 3

3 years ago
(In reply to Steven MacLeod [:smacleod] from comment #2)
> So I believe this is guaranteed to work because the way we're listening for
> the load is in the content-utils.js framescript, and then sending an async
> message up. Since we don't yield the event loop before setting up our
> listener for that message (in browserLoaded) we can't miss it.

You're right, I've documented this as you suggested and pushed to fx-team:

https://hg.mozilla.org/integration/fx-team/rev/477656ff3fc5
https://hg.mozilla.org/mozilla-central/rev/477656ff3fc5
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.