scrapinghub/splash — pull request #49 by andresp99999 (2013-10-13 22:12:25+00:00)
Title: Fix for issue: https://github.com/scrapinghub/splash/issues/47

I looked into issue: https://github.com/scrapinghub/splash/issues/47, after some debugging I think I know why those pages are failing. When loading those pages in a browser we can see the following requests:

1. The original page starts loading (the html and all its resources)
2. A redirect is triggered from javascript (window.location is set to a new url).
3. The original page loading is cancelled and the target page starts loading.
4. The target page finished loading.

This flow is the reason why the loadFinished signal is called twice inside Splash. The first call to loadFinished has the ok parameter set to False (since the original page didn't loaded completely) and the second call to loadFinished has the ok parameter set to True.

The fix I am proposing in this PR is the following:

1. Add a new variable 'loading' to the web_page object. This is set to True when the loadStarted signal is called and it is set to False when the loadFinished signal is called.
2. When a loadFinished signal is called when a ok parameter set to False (like in the redirection issue) a timer is set to check the loading variable in LOAD_FINISHED_DELAY millis.
3. When the timer is called if a redirect was issued then 'loading' is True (set by the loadStarted of the target request) and nothing is done. If 'loading' is False then we assume the original page loading failed and errback is called.

I tested this fix with the urls in the ticket and they now work fine. I tried to add a unit test but it turned out to be tricky to simulate the exact error condition (if the original page loads too quickly then the first loadFinished is called with True, since the original page loads before it is cancelled by the javascript redirect).

A couple of additional comments:

I set the default value of LOAD_FINISHED_DELAY to 250 millis, based on my test this delay works fine with the pages I tested.

As part of this fix I also removed the remaining like referenced from: https://github.com/scrapinghub/splash/issues/33.