761600 - Stale elements do not work since CPG (compartment per global) has been landed

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect, P2)

Description

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

On bug 610258 I have added a feature to better handle stale elements for content pages. So we simply cached the window in the element class. With the GPC landing that's not possible anymore because we would work on a dead object after a reload of the tab.

Probably we should get rid of this handling at all and backout the patch on bug 610258 for master. Tests really should re-instantiate elements after a page load.

Clint, what do you think?

Comment 1

[cmtalbert]

I'm with you, I think we should remove the ability to re-use stale elements and force the test to recreate them. But it would be nice if the framework could let you know when an element goes stale.

Comment 2

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

Lets bring this back to the triage meeting today, given that it is a regression from Mozmill 1.5.

Comment 3

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

We mixed up the bug assignments on Monday. That one can actually be done by Johannes while I have to work on bug 760720.

Comment 4

[Johannes Vogel (:nebelhom)]

Ok, with :whimboo 's help I have done several changes similar to the ones for bug 610258 .

Unfortunately, this has not completely solved the issue yet and I have a strange situation, now.

Here is the link to the current commits on my github fork

https://github.com/Nebelhom/mozmill/commits/bug-761600-fix

When running stale_elements.js as is on an aurora build with

mozmill -b ./firefox-aurora/firefox/firefox-bin -t mutt/mutt/tests/js/elementslib/stale_element.js

it passes (Please note the change of assert.ok to simply elem.getNode() )

However, when uncommenting the following line:

https://github.com/Nebelhom/mozmill/blob/bug-761600-fix/mozmill/mozmill/extension/resource/driver/mozelement.js#l87

the test times out. Henrik suggested to not cache the document, but the defaultView in this._view to fix the issue, but instead the addition seems to have a rather drastic. I don't understand why the mere addition of a variable definition would have such a drastic effect...

Could anyone of you tell me the reason for this behaviour and/or how to fix it? Thanks

Johannes

Comment 5

[Johannes Vogel (:nebelhom)]

Ok, an error that seems to occur during the above mentioned procedure is:

Timestamp: 04/24/2013 11:50:25 PM
Error: [Exception... "'TypeError: args.document is undefined' when calling method: [nsITimerCallback::notify]"  nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

Therefore args["document"].defaultView would understandably not work.

Not sure, how to circumvent this problem just now. Suggestions are welcome.

Comment 6

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

Attachment: Patch v1

With the class hierarchy fixed in bug 860658 this was an easy modification. We simply also cache the window and each time we want to access the element, we check if the document is valid. If not we retrieve the element and cache it again.

Keep in mind that this will not work for Elem nodes because we do not have any way to retrieve it again after the document has been replaced. In this case we return null for getNode() and don't throw the dead object error. Which is already very helpful.

Comment 7

[cmtalbert]

Comment on attachment 744920 [details] [diff] [review]
Patch v1

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

I think these changes look fine.

Comment 8

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

Pushed to master:
https://github.com/mozilla/mozmill/commit/1706822f1d7b336a98b6622e8148a761e840f1a8