Some tests fail with "can't access dead object" since patch on bug 695480 has landed

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vladmaniac, Assigned: whimboo)

Tracking

({regression})

unspecified
All
Mac OS X
regression
Dependency tree / graph

Firefox Tracking Flags

(firefox12 fixed, firefox13 fixed, firefox14 fixed, firefox15 affected, firefox-esr10 fixed, status1.9.2 wontfix)

Details

(Whiteboard: [mozmill-test-failure][lib][qa-], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Version: 
Firefox 15.0a1 (15.0a1, en-US, 20120502030505)

Mozmill: 
1.5.9 

Error: 
handleWindow@resource://mozmill/stdlib/securable-module.js -> file:///tmp/tmpDtvfJC.mozmill-tests/lib/utils.js:412 @resource://mozmill/modules/frame.js -> file:///tmp/tmpDtvfJC.mozmill-tests/tests/functional/testPrivateBrowsing/testDisabledElements.js:38 @resource://mozmill/modules/frame.js:557 @resource://mozmill/modules/frame.js:626 @resource://mozmill/modules/frame.js:669 @resource://mozmill/modules/frame.js:506 @resource://mozmill/modules/frame.js:681 @resource://jsbridge/modules/server.js:179 @resource://jsbridge/modules/server.js:183 @resource://jsbridge/modules/server.js:283 

Tests affected:
http://hg.mozilla.org/qa/mozmill-tests/file/4d11b76a73ae/tests/functional/testPrivateBrowsing/testDisabledElements.js
http://hg.mozilla.org/qa/mozmill-tests/file/4d11b76a73ae/tests/functional/testPrivateBrowsing/testDownloadManagerClosed.js

I suspect this has something to do with bug 695480

There are no local investigations at the moment, but I am on it as we speak.
(Reporter)

Updated

5 years ago
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
(Reporter)

Updated

5 years ago
(Reporter)

Comment 1

5 years ago
The failure is reproducible locally
(Assignee)

Updated

5 years ago
OS: Linux → All
Hardware: x86 → All
Whiteboard: [mozmill-test-failure]
(Assignee)

Comment 2

5 years ago
Please also check other platforms where even other tests could fail with the same failure message.
Blocks: 695480
Keywords: regression
Summary: Mozmill test failure /testPrivateBrowsing/testDownloadManagerClosed.js and /testPrivateBrowsing/testDisabledElements.js | "can't access dead object" → Some tests fail with "can't access dead object" since patch on bug 695480 has landed
(Reporter)

Comment 3

5 years ago
The reason why we fail is explained in this pastebin: 

http://themaniak.pastebin.mozilla.org/1613688 

We are accessing the window object after the window has been closed, and then the object is dead. 
I'm going to also add bug 628576 and bug 639870 to dependencies, because they will have to consider this change. 
I am wondering how we are going to test for the closing event if the window object is dead? 
Do we have an event listener there?
OS: All → Linux
Hardware: All → x86
(Reporter)

Updated

5 years ago
Depends on: 628576, 639870
(Assignee)

Comment 4

5 years ago
Please never explain stuff in a pastebin. Always add an attachment if necessary to a bug but better do it wisely in a comment.
(Reporter)

Comment 5

5 years ago
(In reply to Henrik Skupin (:whimboo) from comment #4)
> Please never explain stuff in a pastebin. Always add an attachment if
> necessary to a bug but better do it wisely in a comment.

Ok. Well please consider comment 3 as an explanation (what is after the pastebin link which has the code snippet). The comment also contains my concerns at the moment
(Assignee)

Comment 6

5 years ago
Complete list of tests which are affected:

functional/testPrivateBrowsing/testCloseWindow.js
functional/testPrivateBrowsing/testDisabledElements.js 
functional/testPrivateBrowsing/testDownloadManagerClosed.js

When we have landed the patch and it works I will port the code to the new API and Mozmill core.
Assignee: vlad.mozbugs → hskupin
Blocks: 628576
No longer depends on: 628576, 639870
OS: Linux → All
Hardware: x86 → All
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][lib]
(Assignee)

Comment 7

5 years ago
Created attachment 620651 [details] [diff] [review]
More robust window closing code (v1)

So it appears that we have to silently fail in some cases. Exactly when the test itself closes the window but doesn't specify the flag for handleWindow. In those circumstances our code still wants to force closing the window and fails because the window object is not available anymore.
Attachment #620651 - Flags: review?(dave.hunt)
Comment on attachment 620651 [details] [diff] [review]
More robust window closing code (v1)

># HG changeset patch
># Parent 4d11b76a73ae0a801fc5ea8b5eddf714f9663731
># User Henrik Skupin <hskupin@mozilla.com>
>Bug 751242 - Make handleWindow code more robust when the test already closed the window. r=dhunt
>
>diff --git a/lib/utils.js b/lib/utils.js
>--- a/lib/utils.js
>+++ b/lib/utils.js
>+      } catch (e if e instanceof TypeError) {
>+        // The test itself has already been destroyed the window. Also the object
>+        // is not available anymore because it has been marked as dead. We can fail
>+        // silently.
>+      }
>+

Minor nit: Should this comment read "The test itself has already destroyed the window"? The "been" makes it unclear to me.

r=me with the above comment clarified
Attachment #620651 - Flags: review?(dave.hunt) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 620654 [details] [diff] [review]
More robust window closing code (v2)

Fixed the minor review nit. Will land it now.
Attachment #620651 - Attachment is obsolete: true
Attachment #620654 - Flags: review+
(Assignee)

Comment 10

5 years ago
Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/6f1bbe8238a7

Once we get a clean test-run we should backport this to older branches too.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status1.9.2: --- → wontfix
status-firefox-esr10: --- → affected
status-firefox12: --- → affected
status-firefox13: --- → affected
status-firefox14: --- → affected
status-firefox15: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 11

5 years ago
No more test failures are visible. I'm going to land this change for the other branches.

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/457f311f0b9d (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/59fd6a49b5fc (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/53216b03d6e7 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/746170de8030 (esr10)
status-firefox-esr10: affected → fixed
status-firefox12: affected → fixed
status-firefox13: affected → fixed
status-firefox14: affected → fixed
(Reporter)

Comment 12

5 years ago
Now we are failing with this 

http://mozmill-release.blargon7.com/#/functional/report/c2b72632f20450b6d99d14c70981dfb9 

I think this needs to be filed separately in another bug, as a regression to this patch?
(Assignee)

Comment 13

5 years ago
I don't think this is a regression. Please wait for todays tests before filing any new bug. Keep in mind that Mozmill was broken in the last 2 days due to bug 751424. Version 1.5.12 has been released right now.
(Reporter)

Comment 14

5 years ago
I agree now that my local test has finished, thanks Henrik for clarifications

http://mozmill-crowd.blargon7.com/db/c2b72632f20450b6d99d14c70983f971
(Assignee)

Updated

5 years ago
Depends on: 753757
Seems to fail again on Mac with Nightly and can be reproduced locally.
Status: RESOLVED → REOPENED
status-firefox15: fixed → affected
OS: All → Mac OS X
Resolution: FIXED → ---
(Assignee)

Comment 16

5 years ago
How long are they failing again? Please always provide a link to the failure report.
Sorry, on CI it stopped failing in 25th of May.
http://mozmill-ci.blargon7.com/#/functional/failure?branch=All&platform=All&from=2012-05-25&test=%2FtestPrivateBrowsing%2FtestCloseWindow.js&func=testCloseWindow.js%3A%3AtestCloseWindow

Though I can reproduce it locally.
Tried to find something useful, but I think it's machine dependant. I used a sleep call and things solved. It is something related to the tab content not loading fast enough when switching tabs.
Do we still want this fixed given that I can reproduce only on 1 machine?
(Assignee)

Comment 19

5 years ago
(In reply to Remus Pop (:RemusPop) from comment #18)
> Tried to find something useful, but I think it's machine dependant. I used a
> sleep call and things solved. It is something related to the tab content not
> loading fast enough when switching tabs.

Not sure what you mean by that. Where have you added a sleep call? Please tell us more details how you tried to find a workaround at least for the time being.

> Do we still want this fixed given that I can reproduce only on 1 machine?

It doesn't matter on how many machines it can be reproduced. It's failing and needs a fix.
After this line I inserted a sleep(2000).
http://hg.mozilla.org/qa/mozmill-tests/file/73b913d2b1f4/tests/functional/testPrivateBrowsing/testCloseWindow.js#l87

The failure complains about a new elementslib when executing this._view = _document.defaultView;
So I think the DOM is not ready when executing line 88: var tab = controller.tabs.getTab(i);
(Assignee)

Comment 21

5 years ago
(In reply to Remus Pop (:RemusPop) from comment #20)
> After this line I inserted a sleep(2000).
> http://hg.mozilla.org/qa/mozmill-tests/file/73b913d2b1f4/tests/functional/
> testPrivateBrowsing/testCloseWindow.js#l87

This is a property so you might want to experiment even more and add the sleep call within the corresponding tabBrowser.selectedIndex setter.

> The failure complains about a new elementslib when executing this._view =
> _document.defaultView;
> So I think the DOM is not ready when executing line 88: var tab =
> controller.tabs.getTab(i);

The former window which was existent at the beginning of the test doesn't exist anymore. So both the controller and tabbrowser have invalid cached windows and should probably recreated after the window has been reopened by leaving the private browsing mode.
(Assignee)

Comment 22

5 years ago
Given that this is a test specific failure please file a new bug for it.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][lib] → [mozmill-test-failure][lib][qa-]
You need to log in before you can comment on or make changes to this bug.