Bug 24449 - Bubble Spinner game page crashes
Summary: Bubble Spinner game page crashes
Status: RESOLVED DUPLICATE of bug 23736
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P1 Normal
Assignee: Cameron Zwarich (cpst)
URL: http://www.addictinggames.com/bubbles...
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2009-03-07 22:04 PST by Cameron Zwarich (cpst)
Modified: 2009-03-09 02:35 PDT (History)
2 users (show)

See Also:


Attachments
Partial reduction (1.02 KB, text/html)
2009-03-07 22:55 PST, Cameron Zwarich (cpst)
no flags Details
Assertion failure stack trace (2.95 KB, text/plain)
2009-03-07 23:16 PST, Cameron Zwarich (cpst)
no flags Details
Reduction (577 bytes, text/html)
2009-03-08 00:18 PST, Cameron Zwarich (cpst)
no flags Details
Simpler reduction (364 bytes, text/html)
2009-03-08 00:37 PST, Cameron Zwarich (cpst)
no flags Details
Proposed patch (466 bytes, patch)
2009-03-08 03:17 PDT, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff
Revised proposed patch (1.22 KB, patch)
2009-03-08 03:18 PDT, Cameron Zwarich (cpst)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron Zwarich (cpst) 2009-03-07 22:04:58 PST
The page <http://www.addictinggames.com/bubblespinner.html> crashes with wildly different backtraces. It has nothing to do with playing the game, and some say it even happens with a "Click to Flash" input manager hack.
Comment 1 Cameron Zwarich (cpst) 2009-03-07 22:08:26 PST
This also crashes with plugins disabled.
Comment 2 Cameron Zwarich (cpst) 2009-03-07 22:20:56 PST
I can reproduce with a local copy and <base href=...>, so I should be able to make a reduction. It seems to crash a lot in DocLoader::setLoadInprogress() under Loader::Host::didFinishLoading() trying to get the Frame of a null document.
Comment 3 Cameron Zwarich (cpst) 2009-03-07 22:28:25 PST
The problem seems to be due to this piece of JS, because it goes away if I delete it:

$(function() {
                                                  setTimeout('update_ad_iframe("iframe3","http://ad.doubleclick.net/adj/ag.nol/puzzlesboards/runofsection_puzzlesboards;gw=puzzlesboards;sec_0=runofsection_puzzlesboards;!category=expand;u|gw-puzzlesboards|sec_0-runofsection_puzzlesboards|;",4,160,600,10 * 60 * 1000,"no")',1600);
				        });	

I imagine something is going wrong with loading the new content in the ad iframe.
Comment 4 Cameron Zwarich (cpst) 2009-03-07 22:55:25 PST
Created attachment 28396 [details]
Partial reduction

This is a small reduction, but it still uses the JS libraries on their site. The problem is caused by them having two timers to update the iframe set to the same timeout (presumedly due to undesirable code duplication on their part). These timers are made at different times during parsing, so they are a bit apart. It seems that canceling the request caused by the first timer and processing the request caused by the second timer. Presumedly, the crash is caused by using objects from the first partially completed load when processing the second load, but I am not sure.

I make this crash consistently for me by opening it up in a bunch of new tabs. Usually when I get to 4 or 5 it will crash.
Comment 5 Cameron Zwarich (cpst) 2009-03-07 23:16:14 PST
Created attachment 28398 [details]
Assertion failure stack trace

I was waiting on a fresh debug build. It turns out my reduction consistently causes an assertion failure.
Comment 6 Cameron Zwarich (cpst) 2009-03-08 00:18:03 PST
Created attachment 28400 [details]
Reduction

Here is a simple reduction for the assertion failure. It still loads some content from the network, but it is not much and is essential to the reduction.

There is no problem with setting the src of an iframe in quick succession with timers. However, when it is done using a javascript: URL that returns a constant string for the source of the iframe that then references a script file over the network, it causes the assertion failure. I don't really understand why, but I will look into it.
Comment 7 Mark Rowe (bdash) 2009-03-08 00:25:26 PST
<rdar://problem/6658053>
Comment 8 Cameron Zwarich (cpst) 2009-03-08 00:37:17 PST
Created attachment 28401 [details]
Simpler reduction

Here is a simpler reduction involving only one external resource, the SquirrelFish PNG. ;-)
Comment 9 Cameron Zwarich (cpst) 2009-03-08 00:47:52 PST
Comment on attachment 28401 [details]
Simpler reduction

Strange, this simpler reduction no longer works for me.
Comment 10 Cameron Zwarich (cpst) 2009-03-08 01:52:52 PST
The problem is that loading a JS URL doesn't cancel pending requests. Adding a call to stopLoading() in FrameLoader::executeIfJavaScriptURL() fixes the bug, because of this section of code in stopLoading():

            cache()->loader()->cancelRequests(docLoader);

However, this is probably not the right change. Hopefully someone more familiar with FrameLoader can let me know what to do here.
Comment 11 Cameron Zwarich (cpst) 2009-03-08 03:17:09 PDT
Created attachment 28404 [details]
Proposed patch

Here's a patch that fixes the problem. It uses stopAllLoaders(), because that seems to be what is used in this situation.

I am confident that I can make a DRT HTTP test that reproduces this bug, but it might take me a while, so I am putting the patch up for review first.
Comment 12 Cameron Zwarich (cpst) 2009-03-08 03:17:58 PDT
Comment on attachment 28404 [details]
Proposed patch

Doh. I posted my old diff. ;-)
Comment 13 Cameron Zwarich (cpst) 2009-03-08 03:18:33 PDT
Created attachment 28405 [details]
Revised proposed patch
Comment 14 Alexey Proskuryakov 2009-03-09 02:19:59 PDT
See also: bug 23736 (duplicate?)

The fix looks reasonable, but it's no good without a test.
Comment 15 Cameron Zwarich (cpst) 2009-03-09 02:35:13 PDT
I am marking this a dupe of bug 23736, because my patch here also fixes that bug and the discussion there is both longer and involving more people.

*** This bug has been marked as a duplicate of 23736 ***
Comment 16 Cameron Zwarich (cpst) 2009-03-09 02:35:47 PDT
Comment on attachment 28405 [details]
Revised proposed patch

Clearing review because it would be confusing on a dupe.