Closed Bug 1524726 Opened Last year Closed 11 months ago

Race condition in browser/components/sessionstore/test/browser_aboutSessionRestore.js

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

Attachments

(1 file)

This test closes a window without waiting for it to fully initialize. Or something. It works right now because await takes two turns through the microtask queue; but bug 1495072 changes that, causing this test to fail quite reliably.

The test uses some custom code to wait for the new window, probably just
because the test is old. The fix is to use the BrowserUtils method instead.

Hmm. This patch has an issue. Now I get this:

[task 2019-02-02T02:21:54.309Z] 02:21:54 ERROR - TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/browser.xul]
[task 2019-02-02T02:21:54.309Z] 02:21:54 ERROR - TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | leaked 1 window(s) until shutdown [url = about:blank]
[task 2019-02-02T02:21:54.309Z] 02:21:54 INFO - TEST-INFO | browser/components/sessionstore/test/browser_aboutSessionRestore.js | windows(s) leaked: [pid = 5435] [serial = 17], [pid = 5435] [serial = 18]
[task 2019-02-02T02:21:54.313Z] 02:21:54 ERROR - TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | leaked 1 docShell(s) until shutdown
[task 2019-02-02T02:21:54.313Z] 02:21:54 INFO - TEST-INFO | browser/components/sessionstore/test/browser_aboutSessionRestore.js | docShell(s) leaked: [pid = 5435] [id = {98d5281b-e61b-4e9f-a2d5-a4b03f71d857}]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9680a762389f4132bdb1f552e12d193cb8f47e8a&selectedJob=225594801

Adding a call to BrowserTestUtils.removeTab(tab) seems to fix it for me locally.

Well, this now fails with an additional error message.

https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunning%2Cpending%2Crunnable&revision=062c8d2376f219984890a225cf8cb599215fa871&selectedJob=230366798

But now it passes for me locally, so I'm at a loss to fix it. florian, can you tell what's going on here?

Here's the version of the patch that's failing in that try run: https://hg.mozilla.org/try/rev/15beea7c47fbd8ddd3557054a09a03c84241ba6d

Flags: needinfo?(florian)

01:07:28 INFO - TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | Uncaught exception - [Exception... "Illegal value" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_aboutSessionRestore.js :: <TOP_LEVEL> :: line 34" data: no]

Line 34 https://hg.mozilla.org/try/annotate/15beea7c47fb/browser/components/sessionstore/test/browser_aboutSessionRestore.js#l34 is:
ok(view.isContainer(0), "first entry is the window");

I don't see how this would relate to your test. Note that the failures here are only on the TV jobs which are tier2 so:

  • it's possible these were pre-existing failures
  • you won't get backed out is this is the only failures.

I pushed to try to see if TV passed reliably for this test without your patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d3565565d867f675332c849e324d359eb23b393

(In reply to Florian Quèze [:florian] from comment #4)

I pushed to try to see if TV passed reliably for this test without your patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d3565565d867f675332c849e324d359eb23b393

This confirms that the TV failures you are seeing in your try run are unrelated to your patch and already existed. Both the Linux x64 debug leak, and the NS_ERROR_ILLEGAL_VALUE errors on OS X opt.

Flags: needinfo?(florian)
Attachment #9040940 - Attachment description: Bug 1524726 - Fix race condition in a SessionStore test. r?florian → Bug 1524726 - Fix a race condition in a SessionStore test. r?florian
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efec51b1aaf2
Fix a race condition in a SessionStore test. r=florian
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Assignee: nobody → jorendorff
You need to log in before you can comment on or make changes to this bug.