Bug 12353 – REGRESSION: Crash on load (mutation event dispatch under the image element constructor deletes the element)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Alexander Kellett]

http://eigenclass.org/hiki.rb?method+arguments+via+introspection crashes on load

[Alexander Kellett]

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

[David Kilzer (:ddkilzer)]

Does not crash on shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037).

[David Kilzer (:ddkilzer)]

Created attachment 12584 [details]
Debug stack trace (r19008)

Stack trace from locally-built debug build of WebKit r19008.

[David Kilzer (:ddkilzer)]

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.

Created attachment 12585 [details]
Reduction (will crash)

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.

[David Kilzer (:ddkilzer)]

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.

[Darin Adler]

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

[Mark Rowe (bdash)]

<rdar://problem/4944599>

[Darin Adler]

Committed revision 19018.

[Kirby White]

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