Bug 12353 - REGRESSION: Crash on load (mutation event dispatch under the image element constructor deletes the element)
Summary: REGRESSION: Crash on load (mutation event dispatch under the image element co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Nobody
URL: http://eigenclass.org/hiki.rb?method+...
Keywords: HasReduction, InRadar, Regression
: 12658 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-01-21 08:47 PST by Alexander Kellett
Modified: 2007-03-02 16:14 PST (History)
4 users (show)

See Also:


Attachments
Debug stack trace (r19008) (6.66 KB, text/plain)
2007-01-21 09:00 PST, David Kilzer (:ddkilzer)
no flags Details
Reduction (will crash) (281 bytes, text/html)
2007-01-21 11:18 PST, mitz
no flags Details
Protect the image element from deletion during mutation event dispatch (3.95 KB, patch)
2007-01-21 11:32 PST, mitz
darin: review+
Details | Formatted Diff | Diff
Another reduction (will crash) (236 bytes, text/html)
2007-01-21 12:18 PST, David Kilzer (:ddkilzer)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kellett 2007-01-21 08:47:28 PST
http://eigenclass.org/hiki.rb?method+arguments+via+introspection crashes on load
Comment 1 Alexander Kellett 2007-01-21 08:48:38 PST
1   com.apple.WebCore        	0x01218edd WebCore::Element::setAttribute(WebCore::QualifiedName const&, WebCore::StringImpl*, int&) + 55
2   com.apple.WebCore        	0x012190c1 WebCore::Element::setAttribute(WebCore::QualifiedName const&, WebCore::String const&) + 47
3   com.apple.WebCore        	0x0127a7b9 WebCore::HTMLImageElement::setHeight(int) + 59
4   com.apple.WebCore        	0x01230d1c KJS::ImageConstructorImp::construct(KJS::ExecState*, KJS::List const&) + 264
5   com.apple.JavaScriptCore 	0x0012cb35 KJS::NewExprNode::evaluate(KJS::ExecState*) + 533
6   com.apple.JavaScriptCore 	0x0012aae9 KJS::VarDeclNode::evaluate(KJS::ExecState*) + 55
7   com.apple.JavaScriptCore 	0x0012aa5a KJS::VarDeclListNode::evaluate(KJS::ExecState*) + 42
8   com.apple.JavaScriptCore 	0x00130a3b KJS::VarStatementNode::execute(KJS::ExecState*) + 117
9   com.apple.JavaScriptCore 	0x0013385b KJS::SourceElementsNode::execute(KJS::ExecState*) + 163
10  com.apple.JavaScriptCore 	0x00130b0d KJS::BlockNode::execute(KJS::ExecState*) + 67
11  com.apple.JavaScriptCore 	0x00130d9a KJS::IfNode::execute(KJS::ExecState*) + 270
12  com.apple.JavaScriptCore 	0x0013395d KJS::SourceElementsNode::execute(KJS::ExecState*) + 421
13  com.apple.JavaScriptCore 	0x00130b0d KJS::BlockNode::execute(KJS::ExecState*) + 67
Comment 2 David Kilzer (:ddkilzer) 2007-01-21 08:56:39 PST
Does not crash on shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037).
Comment 3 David Kilzer (:ddkilzer) 2007-01-21 09:00:36 PST
Created attachment 12584 [details]
Debug stack trace (r19008)

Stack trace from locally-built debug build of WebKit r19008.
Comment 4 David Kilzer (:ddkilzer) 2007-01-21 11:04:38 PST
The bug happens in urchin.js in this JavaScript code:

 if ((_userv==1 || _userv==2) && _uSP()) {
  var i2=new Image(1,1); // Safari dies executing this line
  i2.src=_ugifpath2+"?"+"utmwv="+_uwv+s+"&utmac="+_uacct+"&utmcc="+_uGCS();
  i2.onload=function() { _uVoid(); }
 }

When HTMLImageElement::setHeight() is called, it eventually reaches Element::attributes(bool readonly), and the call to updateStyleAttributeIfNeeded() in that method appears to simply "step off the the deep end" as if it doesn't know where to find that method.

Comment 5 mitz 2007-01-21 11:18:24 PST
Created attachment 12585 [details]
Reduction (will crash)
Comment 6 mitz 2007-01-21 11:32:51 PST
Created attachment 12586 [details]
Protect the image element from deletion during mutation event dispatch

I don't know if it makes sense for the calls to setWidth and setHeight to on the newly created element to even fire DOM mutation events, but this patch ensures that the element survives it, and in any case it makes sense to take ownership of the element before doing anything with it.
Comment 7 David Kilzer (:ddkilzer) 2007-01-21 12:18:50 PST
Created attachment 12587 [details]
Another reduction (will crash)

A different reduction based on the original web page.  The patch in Attachment 12586 [details] also fixes this reduction.
Comment 8 Darin Adler 2007-01-21 13:53:05 PST
Comment on attachment 12586 [details]
Protect the image element from deletion during mutation event dispatch

An equally good way to fix this would be to put the newly-created image element into a RefPtr; that's the model we're trying to move to. Reference-counted objects won't have an initial floating state -- instead they go straight into a PassRefPtr.

But this fix is also good. r=me
Comment 9 Mark Rowe (bdash) 2007-01-21 16:02:48 PST
<rdar://problem/4944599>
Comment 10 Darin Adler 2007-01-21 17:27:56 PST
Committed revision 19018.
Comment 11 Kirby White 2007-03-02 16:14:53 PST
*** Bug 12658 has been marked as a duplicate of this bug. ***