753757 - Mozmill test failure /testPrivateBrowsing/testDownloadManagerClosed.js | "Window has been closed. "

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Maniac Vlad Florin (:vladmaniac)]

Firefox build: 
Firefox 14.0a2 (14.0a2, en-US, 20120510042005)

Build from:
http://hg.mozilla.org/releases/mozilla-aurora/rev/56cc5a3c4316

Report:
http://mozmill-ci.blargon7.com/#/functional/report/c2b72632f20450b6d99d14c709b34990

Error:
TimeoutError@resource://mozmill/modules/utils.js:447 waitFor@resource://mozmill/modules/utils.js:485 handleWindow@resource://mozmill/stdlib/securable-module.js -> file:///var/folders/vq/bds4whmx4jg5n19nm5p_9xfr0000gp/T/tmpQDZQwC.mozmill-tests/lib/utils.js:404 @resource://mozmill/stdlib/securable-module.js -> file:///var/folders/vq/bds4whmx4jg5n19nm5p_9xfr0000gp/T/tmpQDZQwC.mozmill-tests/lib/downloads.js:332 @resource://mozmill/modules/frame.js -> file:///var/folders/vq/bds4whmx4jg5n19nm5p_9xfr0000gp/T/tmpQDZQwC.mozmill-tests/tests/functional/testPrivateBrowsing/testDownloadManagerClosed.js:64 @resource://mozmill/modules/frame.js:563 @resource://mozmill/modules/frame.js:632 @resource://mozmill/modules/frame.js:675 @resource://mozmill/modules/frame.js:512 @resource://mozmill/modules/frame.js:687 @resource://jsbridge/modules/server.js:184 @resource://jsbridge/modules/server.js:188 @resource://jsbridge/modules/server.js:288

Mozmill version:
1.5.12

Test location:
http://hg.mozilla.org/qa/mozmill-tests/file/e01e2f9ad8db/tests/functional/testPrivateBrowsing/testDownloadManagerClosed.js

Comment 1

[Maniac Vlad Florin (:vladmaniac)]

This is reproducible locally also with Firefox 15

Comment 2

[Remus Pop (:RemusPop)]

The problem is in:
var closed = false;
        window.addEventListener("unload", function () { closed = true; }, false);
        window.close();
        mozmill.utils.waitFor(function () {
          return closed;
        }, "Window has been closed.");

So my guess is that the event listener gets destroyed when we close the window, so the event won't fire and set closed to true.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We probably should compare the window count. But window.close() should always close the window. So not sure if we should test this at all.

Comment 4

[Remus Pop (:RemusPop)]

We should check what events are available and see if we can use/adapt one.

Comment 5

[Maniac Vlad Florin (:vladmaniac)]

Top failure report 

http://mozmill-ci.blargon7.com/#/functional/report/c2b72632f20450b6d99d14c709b423b0

http://mozmill-ci.blargon7.com/#/functional/failure?branch=All&platform=All&from=2012-05-03&to=2012-05-10&test=%2FtestPrivateBrowsing%2FtestDownloadManagerClosed.js&func=testDownloadManagerClosed.js%3A%3AtestDownloadManagerClosed

Comment 6

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Remus Pop (:RemusPop) from comment #4)
> We should check what events are available and see if we can use/adapt one.

A bunch of events here 
http://mxr.mozilla.org/mozilla-central/search?string=window.addEventListener

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There will be no events. So just compare the window count.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I will take it since no-one is assigned yet.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Make use of windowMap (v1)

We simply can make use of the windowMap now. Whoever comes first, please review so that we can get it landed ASAP. Thanks.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Looks like to be a regression from bug 751242 across all branches.

Comment 11

[Remus Pop (:RemusPop)]

Tested on Nightly in Linux and it fails at testPreferences/testPaneRetention with:
ERROR | Test Failure: {"exception": {"message": "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendKeyEvent]", "name": "NS_ERROR_FAILURE", "lineNumber": 339}}

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is because of bug 754225 and which we will have to fix with a new 1.5 release. So I would say that we go ahead and land this. For a temporary fix I could include an additional sleep of about 100ms which will not be noticeable but should also fix the failure.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Make use of windowMap (v2)

Updated patch with the additional sleep call.

Comment 14

[Remus Pop (:RemusPop)]

Comment on attachment 623099 [details] [diff] [review]
Make use of windowMap (v2)

Looks good and works fine. Please land.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Updated patch for check-in

Sorry, there were some left over code in the test itself I had to remove.

Comment 16

[Remus Pop (:RemusPop)]

Comment on attachment 623102 [details] [diff] [review]
Updated patch for check-in

Looks good to me.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/e614325a27dc

Lets wait for results before backporting the patch.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Also landed on all other branches:
http://hg.mozilla.org/qa/mozmill-tests/rev/89bd7909031c (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/786f16e1e29a (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/98c8eca1d0ae (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/b1cd00663a2f (esr10)

At best we should now be on 0 failures across branches. Lets look for tomorrows results.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Not sure why but there are still some failures lingering around:
http://mozmill-ci.blargon7.com/#/functional/report/c2b72632f20450b6d99d14c709e7c5dd

Seems to be intermittent orange. I will have to further check what's going on here.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Really silly mistake we did here. I will upload a patch in a minute.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Really wait for window closed

Comment 22

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 624539 [details] [diff] [review]
Really wait for window closed

r+ of course!

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Really wait for window closed (v2)

In fact we can even remove the sleep call now! \o/

Comment 24

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 624542 [details] [diff] [review]
Really wait for window closed (v2)

It just keeps getting better!

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/e31f0088a48b

I will trigger a test-run for each platform to confirm it is fixed.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Failure is not visible anymore. Lets get it landed on other branches:

http://hg.mozilla.org/qa/mozmill-tests/rev/29891037c4f8 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/7389712a0abb (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/1d38824b338b (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/5536f0fd76a6 (esr10)