Bug 10837 – REGRESSION: Yahoo New Charts Crashes WebKit.

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[David Scheck]

The URL works in Safari but not Webkit.  

WebKit-SVN-r16342.dmg

[Mark Rowe (bdash)]

Confirmed with r16342.

[Mark Rowe (bdash)]

Created attachment 10524 [details]
Crash log

ASSERTION FAILED: p (WebCore/bindings/js/kjs_window.cpp:378 static KJS::JSValue* KJS::Window::retrieve(WebCore::Frame*))

Created attachment 10530 [details]
Reduction (will assert or crash)

The crash happens when trying to access the location property of a document that is no longer associated with a frame. I think the document's URL can be a good fallback in this case.

*** Bug 10955 has been marked as a duplicate of this bug. ***

[Andrew Wellington]

Created attachment 10677 [details]
Proposed patch

Patch to return null in line with Firefox

[Andrew Wellington]

Comment on attachment 10677 [details]
Proposed patch

Oi! Who put those tabs in there... :-/

[Andrew Wellington]

Created attachment 10678 [details]
Proposed patch without the tabs

[Eric Seidel (no email)]

Comment on attachment 10678 [details]
Proposed patch without the tabs

Looks good to me.  Too bad this logic can't be put in the DOM c++ implementation class.  That would sure make later auto-generation of JSHTMLDocument easier.

Curious.  I wonder if this is broken in the Obj-C version of this wrapper?

Otherwise it looks fine.  r=me.

[Alexey Proskuryakov]

> Too bad this logic can't be put in the DOM c++ implementation class.

Cannot the check be inside retrieveWindow() itself?

[Darin Adler]

(In reply to comment #10)
> > Too bad this logic can't be put in the DOM c++ implementation class.
> 
> Cannot the check be inside retrieveWindow() itself?

Yes. retrieveWindow should return 0 in this case. And we can change the jsUndefined() to jsNull().

We should make that change instead of this one.

[Darin Adler]

(In reply to comment #9)
> (From update of attachment 10678 [details] [edit])
> Too bad this logic can't be put in the DOM C++ implementation class.

This logic *can* be put into the DOM C++ implementation class. I'm going to do that.

[Darin Adler]

(In reply to comment #12)
> (In reply to comment #9)
> > (From update of attachment 10678 [details] [edit] [edit])
> > Too bad this logic can't be put in the DOM C++ implementation class.
> 
> This logic *can* be put into the DOM C++ implementation class. I'm going to do
> that.

Actually, it can't because there is no C++ implementation for this method yet. So the patch is fine as-is. Also, I think the frame nil check is a good solution -- no need to change Window::retrieve just to fix this, although we could consider making that change later.

[Mark Rowe (bdash)]

Landed in r16881.