86035 2012-05-09 16:23:20 -0700 Crash in FrameView::windowClipRectForFrameOwner after r116371 2012-05-14 14:51:39 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 jchaffraix jchaffraix abarth eric tony oldest_to_newest 619991 0 jchaffraix 2012-05-09 16:23:20 -0700 Example stack-trace: 0x67258b37 [chrome.dll] - frameview.cpp:2573 WebCore::FrameView::windowClipRectForFrameOwner(WebCore::HTMLFrameOwnerElement const *,bool) 0x67258ae1 [chrome.dll] - frameview.cpp:2564 WebCore::FrameView::windowClipRect(bool) 0x67258bde [chrome.dll] - frameview.cpp:2587 WebCore::FrameView::windowClipRectForFrameOwner(WebCore::HTMLFrameOwnerElement const *,bool) 0x67cde64f [chrome.dll] - webplugincontainerimpl.cpp:718 WebKit::WebPluginContainerImpl::windowClipRect() 0x67cdee3f [chrome.dll] - webplugincontainerimpl.cpp:697 WebKit::WebPluginContainerImpl::calculateGeometry(WebCore::IntRect const &,WebCore::IntRect &,WebCore::IntRect &,WTF::Vector<WebCore::IntRect,0> &) 0x67cdef4e [chrome.dll] - webplugincontainerimpl.cpp:326 WebKit::WebPluginContainerImpl::reportGeometry() 0x67cde32c [chrome.dll] - webplugincontainerimpl.cpp:239 WebKit::WebPluginContainerImpl::setParent(WebCore::ScrollView *) 0x66c64205 [chrome.dll] - scrollview.cpp:74 WebCore::ScrollView::addChild(WTF::PassRefPtr<WebCore::Widget>) 0x671dd73f [chrome.dll] - renderwidget.cpp:91 WebCore::moveWidgetToParentSoon 0x671ddddb [chrome.dll] - renderwidget.cpp:219 WebCore::RenderWidget::setWidget(WTF::PassRefPtr<WebCore::Widget>) 0x671d673a [chrome.dll] - renderpart.cpp:55 WebCore::RenderPart::setWidget(WTF::PassRefPtr<WebCore::Widget>) 0x675a56f2 [chrome.dll] - subframeloader.cpp:383 WebCore::SubframeLoader::loadPlugin(WebCore::HTMLPlugInImageElement *,WebCore::KURL const &,WTF::String const &,WTF::Vector<WTF::String,0> const &,WTF::Vector<WTF::String,0> const &,bool) 0x675a4918 [chrome.dll] - subframeloader.cpp:131 WebCore::SubframeLoader::requestPlugin(WebCore::HTMLPlugInImageElement *,WebCore::KURL const &,WTF::String const &,WTF::Vector<WTF::String,0> const &,WTF::Vector<WTF::String,0> const &,bool) 0x675a4a43 [chrome.dll] - subframeloader.cpp:151 WebCore::SubframeLoader::requestObject(WebCore::HTMLPlugInImageElement *,WTF::String const &,WTF::AtomicString const &,WTF::String const &,WTF::Vector<WTF::String,0> const &,WTF::Vector<WTF::String,0> const &) 0x675efa5f [chrome.dll] - htmlembedelement.cpp:177 WebCore::HTMLEmbedElement::updateWidget(WebCore::PluginCreationOption) 0x67258913 [chrome.dll] - frameview.cpp:2290 WebCore::FrameView::updateWidget(WebCore::RenderEmbeddedObject *) 0x66b599f7 [chrome.dll] - frameview.cpp:2323 WebCore::FrameView::updateWidgets() 0x66b56390 [chrome.dll] - frameview.cpp:2376 WebCore::FrameView::performPostLayoutTasks() 0x66b396e2 [chrome.dll] - frameview.cpp:1166 WebCore::FrameView::layout(bool) 0x67258732 [chrome.dll] - frameview.cpp:2029 WebCore::FrameView::layoutTimerFired(WebCore::Timer<WebCore::FrameView> *) 0x66e8815b [chrome.dll] - timer.h:100 WebCore::Timer<WebCore::CachedImage>::fired() 0x66b68a42 [chrome.dll] - threadtimers.cpp:115 WebCore::ThreadTimers::sharedTimerFiredInternal() 0x66b688fc [chrome.dll] - threadtimers.cpp:93 WebCore::ThreadTimers::sharedTimerFired() 0x66a0ebde [chrome.dll] - timer.cc:179 base::Timer::RunScheduledTask() 0x66a0eb49 [chrome.dll] - timer.cc:44 base::BaseTimerTaskInternal::Run() 0x669c772d [chrome.dll] - message_loop.cc:458 MessageLoop::RunTask(base::PendingTask const &) 0x669c6c44 [chrome.dll] - message_loop.cc:685 MessageLoop::DoDelayedWork(base::TimeTicks *) 0x669d81cc [chrome.dll] - message_pump_default.cc:32 base::MessagePumpDefault::Run(base::MessagePump::Delegate *) 0x669c5fa6 [chrome.dll] - message_loop.cc:390 MessageLoop::RunHandler() 0x669c5f54 [chrome.dll] - message_loop.cc:300 MessageLoop::Run() 0x66a48893 [chrome.dll] - renderer_main.cc:271 RendererMain(content::MainFunctionParams const &) 0x669c242b [chrome.dll] - content_main_runner.cc:292 `anonymous namespace'::RunNamedProcessTypeMain(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,content::MainFunctionParams const &,content::ContentMainDelegate *) 0x669c23b0 [chrome.dll] - content_main_runner.cc:550 `anonymous namespace'::ContentMainRunnerImpl::Run() 0x669b44d7 [chrome.dll] - content_main.cc:35 content::ContentMain(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *,content::ContentMainDelegate *) 0x669b4462 [chrome.dll] - chrome_main.cc:28 ChromeMain 0x00ee7fa1 [chrome.exe] - client_util.cc:423 MainDllLoader::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *) 0x00ee72a4 [chrome.exe] - chrome_exe_main_win.cc:31 RunChrome(HINSTANCE__ *) 0x00ee730f [chrome.exe] - chrome_exe_main_win.cc:47 wWinMain 0x00f07788 [chrome.exe] - crt0.c:263 __tmainCRTStartup 0x75723676 [kernel32.dll] + 0x00013676] BaseThreadInitThunk 0x77d19f41 [ntdll.dll] + 0x00039f41] __RtlUserThreadStart 0x77d19f14 [ntdll.dll] + 0x00039f14] _RtlUserThreadStart Looking at the error and the code, it seems like it is possible for windowClipRect to get a NULL parentView. I don't have a reproduction case unfortunately. 620020 1 141050 jchaffraix 2012-05-09 16:41:32 -0700 Created attachment 141050 Proposed blind fix. Added a NULL-check. 620047 2 tony 2012-05-09 17:04:31 -0700 I'm not a good reviewer for this. Maybe Adam is? 620091 3 abarth 2012-05-09 17:29:00 -0700 Have you tried using the beforeload event on the <object> element to screw around with the DOM below FrameView::updateWidget ? 620092 4 141050 eric 2012-05-09 17:29:16 -0700 Comment on attachment 141050 Proposed blind fix. Added a NULL-check. Why is something calling windowClipRect w/o this being a child FrameView? 620093 5 eric 2012-05-09 17:30:18 -0700 Is WebKit::WebPluginContainerImpl::setParent calling this before actually setting the parent? Is somethign else executing to remove this widget from the hierarchy before this executes? 620583 6 141050 jchaffraix 2012-05-10 07:56:57 -0700 Comment on attachment 141050 Proposed blind fix. Added a NULL-check. (In reply to comment #3) > Have you tried using the beforeload event on the <object> element to screw around with the DOM below FrameView::updateWidget ? I haven't as I didn't know this code enough to create a test on the spot. Let me regroup and come back to you with hopefully a test case! > Is WebKit::WebPluginContainerImpl::setParent calling this before actually setting the parent? Is somethign else executing to remove this widget from the hierarchy before this executes? It's difficult for me to answer any question without a reproduction (I don't hide it's a blind fix based on the stack-trace). My understanding is that it's possible for |parentView| to be NULL (as mentioned in Document.h). The code prior to r116371 was doing NULL checking the enclosingLayer() which may explain how they go away with that. 623200 7 jchaffraix 2012-05-14 12:18:01 -0700 (In reply to comment #6) > (From update of attachment 141050 [details]) > (In reply to comment #3) > > Have you tried using the beforeload event on the <object> element to screw around with the DOM below FrameView::updateWidget ? > > I haven't as I didn't know this code enough to create a test on the spot. Let me regroup and come back to you with hopefully a test case! OK, unfortunately that didn't help (tried to kill one of our parent FrameView) nor did the Chromium crash reports so far. > > Is WebKit::WebPluginContainerImpl::setParent calling this before actually setting the parent? Looking at the code in WebPluginContainerImpl, that's not the case: Widget::setParent(view); if (view) reportGeometry(); (the crash is below reportGeometry) > Is somethign else executing to remove this widget from the hierarchy before this executes? I wonder about that. This is a big crasher on Youtube but it's very flaky (no video seems to reproduce the issue more than a couple of times). This may as well indicate a race condition in the Chromium code or just in WebKit. 623354 8 141050 hyatt 2012-05-14 14:41:54 -0700 Comment on attachment 141050 Proposed blind fix. Added a NULL-check. r=me 623361 9 141050 jchaffraix 2012-05-14 14:51:30 -0700 Comment on attachment 141050 Proposed blind fix. Added a NULL-check. Landed in http://trac.webkit.org/changeset/117005. 141050 2012-05-09 16:41:32 -0700 2012-05-14 14:51:30 -0700 Proposed blind fix. Added a NULL-check. bug-86035-20120509164131.patch text/plain 1693 jchaffraix U3VidmVyc2lvbiBSZXZpc2lvbjogMTE2NTM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDI2MzA1YTY5ZjczMDdj NjVjMDdlZDM4M2JlMWVjMWE1NWY5NjNjYy4uODZjMjAwZWYwMTM3MDY2YTM2MWY5ZjQ1NDY5ZTdk ZjkxZDcwY2RlMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTA1LTA5ICBKdWxp ZW4gQ2hhZmZyYWl4ICA8amNoYWZmcmFpeEB3ZWJraXQub3JnPgorCisgICAgICAgIENyYXNoIGlu IEZyYW1lVmlldzo6d2luZG93Q2xpcFJlY3RGb3JGcmFtZU93bmVyIGFmdGVyIHIxMTYzNzEKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg2MDM1CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWRkZWQgYSBOVUxM LWNoZWNrIGZvciB8cGFyZW50Vmlld3wgYXMgbm90aGluZyBndWFyYW50ZWVzIGl0IHRvIGJlCisg ICAgICAgIG5vbi1OVUxMLiBVbmZvcnR1bmF0ZWx5IG5vIHRlc3QgYXMgdGhpcyBpcyBhIGNyYXNo ZXIgSSBjb3VsZG4ndAorICAgICAgICByZXByb2R1Y2Ugb24gbXkgbWFjaGluZSBhbmQgdGhlIHVz ZXIgbG9ncyB3ZXJlIG5vdCBoZWxwZnVsLgorCisgICAgICAgICogcGFnZS9GcmFtZVZpZXcuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6RnJhbWVWaWV3Ojp3aW5kb3dDbGlwUmVjdCk6CisKIDIwMTIt MDUtMDkgIFRvbW15IFdpZGVuZmx5Y2h0ICA8dG9tbXl3QGdvb2dsZS5jb20+CiAKICAgICAgICAg TWVkaWFTdHJlYW0gQVBJOiBTZXNzaW9uRGVzY3JpcHRpb246OmFkZENhbmRpZGF0ZSBzaG91bGQg bm90IGNyYXNoIGZvciBtYWxmb3JtZWQgaW5wdXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L3BhZ2UvRnJhbWVWaWV3LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmNwcApp bmRleCA1NzI4NTM2MWYzNWMxY2Y5MzkwZTdjZDA2N2RjY2ViZDVmYzkyNTc3Li4wY2VmMGUxZTgx Y2FhNDkwMjcxYWZlMmQ1N2RiODMwNzVjZGUwOWU3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wYWdlL0ZyYW1lVmlldy5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9GcmFtZVZpZXcu Y3BwCkBAIC0yNTYxLDcgKzI1NjEsOCBAQCBJbnRSZWN0IEZyYW1lVmlldzo6d2luZG93Q2xpcFJl Y3QoYm9vbCBjbGlwVG9Db250ZW50cykgY29uc3QKICAgICAvLyBUYWtlIG91ciBvd25lciBlbGVt ZW50IGFuZCBnZXQgaXRzIGNsaXAgcmVjdC4KICAgICBIVE1MRnJhbWVPd25lckVsZW1lbnQqIG93 bmVyRWxlbWVudCA9IG1fZnJhbWUtPm93bmVyRWxlbWVudCgpOwogICAgIEZyYW1lVmlldyogcGFy ZW50VmlldyA9IG93bmVyRWxlbWVudC0+ZG9jdW1lbnQoKS0+dmlldygpOwotICAgIGNsaXBSZWN0 LmludGVyc2VjdChwYXJlbnRWaWV3LT53aW5kb3dDbGlwUmVjdEZvckZyYW1lT3duZXIob3duZXJF bGVtZW50LCB0cnVlKSk7CisgICAgaWYgKHBhcmVudFZpZXcpCisgICAgICAgIGNsaXBSZWN0Lmlu dGVyc2VjdChwYXJlbnRWaWV3LT53aW5kb3dDbGlwUmVjdEZvckZyYW1lT3duZXIob3duZXJFbGVt ZW50LCB0cnVlKSk7CiAgICAgcmV0dXJuIGNsaXBSZWN0OwogfQogCg==