NEW 51673
chrome.dll!WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks ReadAV@NULL (920f5522b6092d55727c9be058273715) 
https://bugs.webkit.org/show_bug.cgi?id=51673
Summary chrome.dll!WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks ReadAV...
Berend-Jan Wever
Reported 2010-12-28 03:42:20 PST
Created attachment 77546 [details] Repro Repro: <body onload="document.body.outerHTML= '<input autofocus=1/><button>x<d>';"><style> id: chrome.dll!WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks ReadAV@NULL (920f5522b6092d55727c9be058273715) description: Attempt to read from unallocated NULL pointer+0xC in chrome.dll!WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks application: Chromium 10.0.623.0 stack: chrome.dll!WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks chrome.dll!WebCore::RenderBlock::addChildIgnoringContinuation chrome.dll!WebCore::RenderBlock::addChild chrome.dll!WebCore::Node::createRendererIfNeeded chrome.dll!WebCore::Text::attach chrome.dll!WebCore::ContainerNode::attach chrome.dll!WebCore::Element::attach chrome.dll!WebCore::HTMLFormControlElement::attach chrome.dll!WebCore::ContainerNode::attach chrome.dll!WebCore::Element::attach chrome.dll!WebCore::ContainerNode::replaceChild chrome.dll!WebCore::HTMLElement::setOuterHTML chrome.dll!WebCore::HTMLElementInternal::outerHTMLAttrSetter chrome.dll!v8::internal::JSObject::SetPropertyWithCallback chrome.dll!v8::internal::JSObject::SetProperty chrome.dll!v8::internal::JSObject::SetProperty chrome.dll!v8::internal::StoreIC::Store chrome.dll!v8::internal::StoreIC_Miss chrome.dll!v8::internal::Invoke chrome.dll!v8::internal::Execution::Call ...
Attachments
Repro (83 bytes, text/html)
2010-12-28 03:42 PST, Berend-Jan Wever 		no flags
Details
Patch (3.44 KB, patch)
2011-01-05 16:59 PST, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (3.45 KB, patch)
2011-01-05 17:14 PST, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (4.71 KB, patch)
2011-01-10 15:10 PST, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Berend-Jan Wever
Comment 1 2010-12-28 03:43:06 PST
Chromium bug: http://code.google.com/p/chromium/issues/detail?id=68141
Emil A Eklund
Comment 2 2011-01-05 16:59:16 PST
Created attachment 78067 [details] Patch
Eric Seidel (no email)
Comment 3 2011-01-05 17:01:52 PST
Comment on attachment 78067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78067&action=review > WebCore/html/HTMLFormControlElement.cpp:152 > + queuePostAttachCallback(focusElementCallback, this); postAttachCallback is sooooo lame. Are we sure there is no other way? > LayoutTests/fast/dom/HTMLInputElement/outerhtml-autofocus-crash.html:4 > +var lc = window.layoutTestController; > +if (lc) > + lc.dumpAsText(); Why use a local here?
Emil A Eklund
Comment 4 2011-01-05 17:13:04 PST
> postAttachCallback is sooooo lame. Are we sure there is no other way? Yeah, it's not pretty. Was the only approach I could come up with that would work though. At least it moves the focus call (and the layout pass triggered by it) out of attach. > Why use a local here? Good point, I'll change it.
Eric Seidel (no email)
Comment 5 2011-01-05 17:14:08 PST
My memory is a bit foggy, but I remember spending time this fall removing postAttachCallback users. So I'm very sad to see one added.
Emil A Eklund
Comment 6 2011-01-05 17:14:33 PST
Created attachment 78075 [details] Patch
James Robinson
Comment 7 2011-01-05 17:16:54 PST
Comment on attachment 78075 [details] Patch What happens if script queries the currently focused element? Will it get a stale value here until the post attach callbacks run?
Emil A Eklund
Comment 8 2011-01-05 17:20:47 PST
> What happens if script queries the currently focused element? > Will it get a stale value here until the post attach callbacks run? It would. I'll try to figure out another way to make this work.
Eric Seidel (no email)
Comment 9 2011-01-05 17:23:31 PST
I believe postAttachCallbacks are run before recalcStyle or attach() returns. I don't think we normally run javascript during style recalc or attach. (See Document.cpp and Element.cpp, resumePostAttachCallbacks() calls). so I don't think a JS race is a concern.
Emil A Eklund
Comment 10 2011-01-05 17:47:49 PST
This is related to bug 44449 <https://bugs.webkit.org/show_bug.cgi?id=44449> as that bug is also caused by calling focus during attach.
Emil A Eklund
Comment 11 2011-01-10 15:10:34 PST
Created attachment 78456 [details] Patch Updated to head and added another test (based on test case from bug 44449). This approach looks like the best option to me, the only alternative I could think of would be to modify Element::focus to take a bool noLayout option. I'd be happy to go down that route if you think it would be better.
Adam Barth
Comment 12 2011-01-11 15:06:18 PST
Comment on attachment 78456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78456&action=review > Source/WebCore/html/HTMLFormControlElement.cpp:152 > - focus(); > + queuePostAttachCallback(focusElementCallback, this); Do we need to hold a reference to |this| to ensure it doesn't get deleted? What if JavaScript calls focus on another done after this line of code executes but before the post-attach callback... I'm just a bit skeptical of this approach, but I don't have anything better to suggest.
Note You need to log in before you can comment on or make changes to this bug.