475797 - waitForPageLoad isn't fully reliable (e.g. for slow connections)

Status: VERIFIED FIXED

(Testing Graveyard :: Mozmill, defect, P1)

Description

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

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090127 Minefield/3.2a1pre ID:20090127020417 and MozMill 1.0.2

During the creation of the functional test for MSN.com (bug 475719) I run into the problem that waitForPageLoad returns immediately while the page has been finished loading around 20 seconds later. As result each element I want to access doesn't exist and the test fails. For now I have to wait for each of the elements I want to use before running an assertNode against.

For future use this function should be 99.99% reliable and return when the page has definitely been finished loading. I don't know anything about the current way it works but it's not an observer, right?

Comment 1

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

Same happened to me lately while running the existing content tests (Amazon and MSN). Probably this is caused by slow DNS queries. While the statusbar is still claiming "Looking up for msn.com" the function returns and caused the test to fail. We really should take DNS resolving into account. Otherwise all content related tests could fail.

I'll mark this as P1 because it only happens under special circumstances. But it is still a bad behavior.

Comment 2

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

Boris, would there be a neat observer we could use for MozMill to better recognize when a page has been completed loading? Right now we fail constantly when DNS resolving takes too long or the connection is too slow.

Here is what we have right now for MozMill:
http://code.google.com/p/mozmill/source/browse/trunk/mozmill/extension/resource/modules/controller.js#748

Comment 3

[Boris Zbarsky [:bzbarsky]]

Uh....  Is there a reason to not use, say, the onload event?  Or DOMContentLoaded if you really only need the DOM?

Comment 4

[Mikeal Rogers [:mikeal] (mrogers@)]

we need to wait for all the rendering to finish not just for the DOM to be ready, onload and DOMContentLoaded fire too early for that.

we also need to protect against the case that waitForPageLoad gets called on a page that is already done loading.

Comment 5

[Boris Zbarsky [:bzbarsky]]

What do you mean by "all the rendering to finish", exactly?  onload always makes sure that layout is completely up to date before it fires, though it might not wait for painting.  So if it's firing too early for you... when do you expect your code to run, exactly?

> we also need to protect against the case that waitForPageLoad gets called on a
> page that is already done loading.

On trunk, you can use document.readyState, no?

Comment 6

[Mikeal Rogers [:mikeal] (mrogers@)]

Adam, you want to look in to this on Minefield and add some code to check for it when it's there.

We can keep the current code around for pre 3.6 Firefox builds.

Comment 7

[Adam Christian]

Using document.readyState and improving the pre 3.6 code should take a couple hours but should be pretty solid.

Comment 8

[Adam Christian]

Just check in some code in Committed revision 495. That should make this work better, can anyone verify?

Comment 9

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

That works perfect! The changeset r495 fixed it:
http://code.google.com/p/mozmill/source/detail?r=495

Comment 10

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

Sorry, that was too early. One of our tests fail. As talked with Adam this page is loading too fast and we don't catch the unload part and we will wait forever.

Comment 11

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

See bug 497543. There is a WIP patch attached which also watches for page loads. Probably it can help us to fix our issue.

Comment 12

[Adam Christian]

Now using DOMContentLoaded and using the window onunload to remove the attribute making for what appears to be a very accurate waitForPageLoad.

Committed revision 501.

Comment 13

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

That change looks interesting. I did a quick look and eventually we still have problem when the page will load too fast. I discovered this by running the following test which is located in our Mozmill repository:

http://hg.mozilla.org/qa/mozmill-tests/file/b0eaf9e85a67/firefox/testPrivateBrowsing/testTabRestoration.js

Adam, when you run it please check the following. After we have entered the Private Browsing mode the Google webpage gets loaded. After that we have a long delay of about 20s. Why does this happen?

Comment 14

[Adam Christian]

Okay I think I finally squashed this one, for real. I am using a listener to detect the loads, and before unloads and setting and unsetting the flags, all of my tests worked this time as expected without hitting any timeouts on long or short page loads.

Comment 15

[Adam Christian]

Marking fixed.

Comment 16

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

Wow, that's pretty cool. Thanks Adam. Verified fixed with:
http://code.google.com/p/mozmill/source/detail?r=507