Closed Bug 1245107 Opened 4 years ago Closed 4 years ago

Intermittent e10s browser_closeTab.js | Found an unexpected tab at the end of test run: https://example.org/browser/browser/components/uitour/test/uitour.html

Categories

(Firefox :: Tours, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
e10s + ---
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: philor, Assigned: MattN)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [rr-chaos])

Attachments

(1 file)

Blocks: e10s-tests
tracking-e10s: --- → +
Is this fallout from bug 1244899?  It seems to have spiked shortly after that commit.  Should it be disabled like browser_UITour3.js, etc?
Flags: needinfo?(MattN+bmo)
This is easy to reproduce with rr chaos mode.
Whiteboard: [rr-chaos]
(In reply to Ben Kelly [:bkelly] from comment #2)
> Is this fallout from bug 1244899?  It seems to have spiked shortly after
> that commit.  Should it be disabled like browser_UITour3.js, etc?

It's because I just enabled this test for e10s in bug 1073247.
Assignee: nobody → MattN+bmo
Blocks: 1073247
Status: NEW → ASSIGNED
This test was doomed to be intermittent in E10S from the initial landing (which I reviewed :( ) since it's relying on an async message from the child to be handled before the test finishes.
Blocks: 1233771
The UITour content API call uses sendAsyncMessage so we can't assume a synchronous close.

Review commit: https://reviewboard.mozilla.org/r/35515/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35515/
Attachment #8720949 - Flags: review?(jaws)
Comment on attachment 8720949 [details]
MozReview Request: Bug 1245107 - browser_closeTab.js: wait for the tab to close. r=jaws

https://reviewboard.mozilla.org/r/35515/#review32195

::: browser/components/uitour/test/browser_closeTab.js:14
(Diff revision 1)
> +  let closePromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "TabClose");
>    yield gContentAPI.closeTab();
> +  yield closePromise;

It would be nice if closeTab returned a promise that was resolved either when the tab was closed or a no-op since this was the only tab.

I'm not sure what we are yielding on gContentAPI.closeTab() for, as far as I can tell UITour-content.js returns undefined for this function, and UITour.jsm returns true for the page event.
Attachment #8720949 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #13)
> Comment on attachment 8720949 [details]
> MozReview Request: Bug 1245107 - browser_closeTab.js: wait for the tab to
> close. r=jaws
> 
> https://reviewboard.mozilla.org/r/35515/#review32195
> 
> ::: browser/components/uitour/test/browser_closeTab.js:14
> (Diff revision 1)
> > +  let closePromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "TabClose");
> >    yield gContentAPI.closeTab();
> > +  yield closePromise;
> 
> It would be nice if closeTab returned a promise that was resolved either
> when the tab was closed or a no-op since this was the only tab.

Ideally that's how it would work but when UITour-content.js was implemented I don't think promises were available.

> I'm not sure what we are yielding on gContentAPI.closeTab() for, as far as I
> can tell UITour-content.js returns undefined for this function, and
> UITour.jsm returns true for the page event.

gContentAPI is now a Proxy that returns a promise when the UITour-content.js API is called.

The try push looks really good at the moment: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2789859775d7
Flags: needinfo?(MattN+bmo)
(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #14)
> gContentAPI is now a Proxy that returns a promise when the UITour-content.js
> API is called.

Forgot the link: https://mxr.mozilla.org/mozilla-central/source/browser/components/uitour/test/head.js?rev=4754d1254086#270
This is also passing locally on 10.10 with chaos mode enabled so I will push.
Comment on attachment 8720949 [details]
MozReview Request: Bug 1245107 - browser_closeTab.js: wait for the tab to close. r=jaws

Approval Request Comment

Since bug 1073247 got uplifted then this should too. We can wait to see if it fixes the problem.
Attachment #8720949 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a6c12ea824d6
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment on attachment 8720949 [details]
MozReview Request: Bug 1245107 - browser_closeTab.js: wait for the tab to close. r=jaws

Test only change, ok to uplift to aurora.
Attachment #8720949 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.