Bug 46672

Summary: Crash (preceded by assertion) in Node::document when running plugins/document-open.html in WebKit2 on Windows
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Plug-insAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ggaren, jhoneycutt
Priority: P2 Keywords: InRadar, LayoutTestFailure
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Make PluginView retain its HTMLPlugInElement andersca: review+

Description Adam Roben (:aroben) 2010-09-27 16:44:49 PDT
To reproduce:

1. run-webkit-tests -2 plugins/embed-attributes-style.html

You'll hit an assertion failure in Node::document:

ASSERT(m_document || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument()));

Here's the backtrace:

WebKit!WebCore::Node::document+0x68 [h:\cyghome\dev\build_webkit\include\webcore\node.h @ 357]
WebKit!WebKit::PluginView::frame+0x19 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\pluginview.cpp @ 284]
WebKit!WebKit::PluginView::evaluate+0x8f [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\pluginview.cpp @ 783]
WebKit!WebKit::NetscapePlugin::evaluate+0x3b [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapeplugin.cpp @ 167]
WebKit!WebKit::NPN_Evaluate+0x78 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapebrowserfuncs.cpp @ 589]
npTestNetscapePlugin!notifyTestCompletion+0x4e [h:\cyghome\dev\webkit\opensource\webkittools\dumprendertree\testnetscapeplugin\pluginobject.cpp @ 723]
npTestNetscapePlugin!testDocumentOpen+0x238 [h:\cyghome\dev\webkit\opensource\webkittools\dumprendertree\testnetscapeplugin\pluginobject.cpp @ 766]
npTestNetscapePlugin!DocumentOpenInDestroyStream::NPP_DestroyStream+0x25 [h:\cyghome\dev\webkit\opensource\webkittools\dumprendertree\testnetscapeplugin\tests\documentopenindestroystream.cpp @ 46]
npTestNetscapePlugin!NPP_DestroyStream+0x172 [h:\cyghome\dev\webkit\opensource\webkittools\dumprendertree\testnetscapeplugin\main.cpp @ 365]
WebKit!WebKit::NetscapePlugin::NPP_DestroyStream+0x3e [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapeplugin.cpp @ 233]
WebKit!WebKit::NetscapePluginStream::stop+0x240 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapepluginstream.cpp @ 330]
WebKit!WebKit::NetscapePluginStream::didFinishLoading+0x30 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapepluginstream.cpp @ 83]
WebKit!WebKit::NetscapePlugin::streamDidFinishLoading+0x38 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\netscape\netscapeplugin.cpp @ 455]
WebKit!WebKit::PluginView::Stream::didFinishLoading+0x73 [h:\cyghome\dev\webkit\opensource\webkit2\webprocess\plugins\pluginview.cpp @ 219]
WebKit!WebCore::NetscapePlugInStreamLoader::didFinishLoading+0x5e [h:\cyghome\dev\webkit\opensource\webcore\loader\netscapepluginstreamloader.cpp @ 103]
WebKit!WebCore::ResourceLoader::didFinishLoading+0x27 [h:\cyghome\dev\webkit\opensource\webcore\loader\resourceloader.cpp @ 446]
WebKit!WebCore::didFinishLoading+0xa1 [h:\cyghome\dev\webkit\opensource\webcore\platform\network\cf\resourcehandlecfnet.cpp @ 244]
Comment 1 Adam Roben (:aroben) 2010-09-27 16:47:37 PDT
<rdar://problem/8484208>
Comment 2 Adam Roben (:aroben) 2010-10-25 12:42:04 PDT
Turns out it's plugins/document-open.html that triggers this assertion.
Comment 3 Adam Roben (:aroben) 2010-10-25 12:42:17 PDT
*** Bug 46712 has been marked as a duplicate of this bug. ***
Comment 4 Adam Roben (:aroben) 2010-10-25 12:58:00 PDT
This fails on Mac, too, if you run the test enough times in a row.
Comment 5 Adam Roben (:aroben) 2010-10-25 13:08:04 PDT
The assertion is firing because PluginView::m_pluginElement->m_document is null. Before the call to NPRuntimeObjectMap::evaluate, it is not null. It becomes null beneath that call when JavaScript code is being compiled!
Comment 6 Adam Roben (:aroben) 2010-10-25 13:08:55 PDT
Here's the backtrace when m_document becomes null:


 	msvcr80.dll!memcpy(unsigned char * dst=0x020e6eb0, unsigned char * src=0x0205e700, unsigned long count=84)  Line 188	Asm
 	JavaScriptCore.dll!WTF::VectorMover<1,JSC::Instruction>::move(const JSC::Instruction * src=0x0205e700, const JSC::Instruction * srcEnd=0x0205e754, JSC::Instruction * dst=0x020e6eb0)  Line 158 + 0x14 bytes	C++
 	JavaScriptCore.dll!WTF::VectorTypeOperations<JSC::Instruction>::move(const JSC::Instruction * src=0x0205e700, const JSC::Instruction * srcEnd=0x0205e754, JSC::Instruction * dst=0x020e6eb0)  Line 255 + 0x11 bytes	C++
 	JavaScriptCore.dll!WTF::Vector<JSC::Instruction,0>::reserveCapacity(unsigned int newCapacity=27)  Line 873 + 0x16 bytes	C++
 	JavaScriptCore.dll!WTF::Vector<JSC::Instruction,0>::expandCapacity(unsigned int newMinCapacity=22)  Line 789	C++
 	JavaScriptCore.dll!WTF::Vector<JSC::Instruction,0>::expandCapacity<enum JSC::OpcodeID const >(unsigned int newMinCapacity=22, const JSC::OpcodeID * ptr=0x0012f360)  Line 827	C++
 	JavaScriptCore.dll!WTF::Vector<JSC::Instruction,0>::append<enum JSC::OpcodeID>(const JSC::OpcodeID & val=op_call)  Line 967 + 0x18 bytes	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitOpcode(JSC::OpcodeID opcodeID=op_call)  Line 688	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitCall(JSC::OpcodeID opcodeID=op_call, JSC::RegisterID * dst=0x044e6700, JSC::RegisterID * func=0x044e6700, JSC::CallArguments & callArguments={...}, unsigned int divot=49, unsigned int startOffset=38, unsigned int endOffset=2)  Line 1661	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitCall(JSC::RegisterID * dst=0x044e6700, JSC::RegisterID * func=0x044e6700, JSC::CallArguments & callArguments={...}, unsigned int divot=49, unsigned int startOffset=38, unsigned int endOffset=2)  Line 1593	C++
 	JavaScriptCore.dll!JSC::FunctionCallDotNode::emitBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * dst=0x044e6700)  Line 430 + 0x55 bytes	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitNode(JSC::RegisterID * dst=0x044e6700, JSC::Node * n=0x044e4490)  Line 217 + 0x17 bytes	C++
 	JavaScriptCore.dll!JSC::ExprStatementNode::emitBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * dst=0x044e6700)  Line 1415	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitNode(JSC::RegisterID * dst=0x044e6700, JSC::Node * n=0x044e44b8)  Line 217 + 0x17 bytes	C++
 	JavaScriptCore.dll!JSC::LabelNode::emitBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * dst=0x044e6700)  Line 1901 + 0x13 bytes	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::emitNode(JSC::RegisterID * dst=0x044e6700, JSC::Node * n=0x044e44c8)  Line 217 + 0x17 bytes	C++
 	JavaScriptCore.dll!JSC::SourceElements::emitBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * dst=0x044e6700)  Line 1370 + 0x1e bytes	C++
 	JavaScriptCore.dll!JSC::ScopeNode::emitStatementsBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * dst=0x044e6700)  Line 1999	C++
 	JavaScriptCore.dll!JSC::ProgramNode::emitBytecode(JSC::BytecodeGenerator & generator={...}, JSC::RegisterID * __formal=0x00000000)  Line 2011	C++
 	JavaScriptCore.dll!JSC::BytecodeGenerator::generate()  Line 144 + 0x1b bytes	C++
 	JavaScriptCore.dll!JSC::ProgramExecutable::compileInternal(JSC::ExecState * exec=0x020aae40, JSC::ScopeChainNode * scopeChainNode=0x020c8ea0)  Line 162	C++
 	JavaScriptCore.dll!JSC::ProgramExecutable::compile(JSC::ExecState * exec=0x020aae40, JSC::ScopeChainNode * scopeChainNode=0x020c8ea0)  Line 250 + 0x10 bytes	C++
 	JavaScriptCore.dll!JSC::evaluate(JSC::ExecState * exec=0x020aae40, JSC::ScopeChain & scopeChain={...}, const JSC::SourceCode & source={...}, JSC::JSValue thisValue={...})  Line 56 + 0x1c bytes	C++
 	WebKit.dll!WebKit::NPRuntimeObjectMap::evaluate(NPObject * npObject=0x02072680, const WTF::String & scriptString={...}, _NPVariant * result=0x0012f85c)  Line 199 + 0x4f bytes	C++
>	WebKit.dll!WebKit::PluginView::evaluate(NPObject * npObject=0x02072680, const WTF::String & scriptString={...}, _NPVariant * result=0x0012f85c, bool allowPopups=false)  Line 797 + 0x1a bytes	C++
 	WebKit.dll!WebKit::NetscapePlugin::evaluate(NPObject * npObject=0x02072680, const WTF::String & scriptString={...}, _NPVariant * result=0x0012f85c)  Line 168 + 0x2c bytes	C++
 	WebKit.dll!WebKit::NPN_Evaluate(_NPP * npp=0x0212a3cc, NPObject * npObject=0x02072680, _NPString * script=0x0012f84c, _NPVariant * result=0x0012f85c)  Line 600 + 0x1b bytes	C++
 	npTestNetscapePlugin.dll!notifyTestCompletion(_NPP * npp=0x0212a3cc, NPObject * object=0x02072680)  Line 723 + 0x20 bytes	C++
 	npTestNetscapePlugin.dll!testDocumentOpen(_NPP * npp=0x0212a3cc)  Line 766 + 0x10 bytes	C++
 	npTestNetscapePlugin.dll!DocumentOpenInDestroyStream::NPP_DestroyStream(_NPStream * __formal=0x0205dc80, _NPStream * __formal=0x0205dc80)  Line 46 + 0xc bytes	C++
 	npTestNetscapePlugin.dll!NPP_DestroyStream(_NPP * instance=0x0212a3cc, _NPStream * stream=0x0205dc80, short reason=0)  Line 377 + 0x1e bytes	C++
 	WebKit.dll!WebKit::NetscapePlugin::NPP_DestroyStream(_NPStream * stream=0x0205dc80, short reason=0)  Line 254 + 0x2f bytes	C++
 	WebKit.dll!WebKit::NetscapePluginStream::stop(short reason=0)  Line 330	C++
 	WebKit.dll!WebKit::NetscapePluginStream::didFinishLoading()  Line 83	C++
 	WebKit.dll!WebKit::NetscapePlugin::streamDidFinishLoading(unsigned __int64 streamID=1)  Line 481	C++
 	WebKit.dll!WebKit::PluginView::Stream::didFinishLoading(WebCore::NetscapePlugInStreamLoader * __formal=0x020c69d0)  Line 222 + 0x2b bytes	C++
 	WebKit.dll!WebCore::NetscapePlugInStreamLoader::didFinishLoading(double finishTime=0.00000000000000000)  Line 103 + 0x21 bytes	C++
 	WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x020adda8, double finishTime=0.00000000000000000)  Line 421 + 0x18 bytes	C++
 	WebKit.dll!WebCore::didFinishLoading(_CFURLConnection * conn=0x020e1c40, const void * clientInfo=0x020adda8)  Line 244 + 0x26 bytes	C++
Comment 7 Adam Roben (:aroben) 2010-10-25 13:09:16 PDT
Why is JSC overwriting m_document when compiling code?
Comment 8 Adam Roben (:aroben) 2010-10-25 13:14:51 PDT
Geoff wondered if the PluginView was getting destroyed before the overwriting happened. I tested to see if the PluginView destructor runs before m_document is overwritten, and it does not.
Comment 9 Adam Roben (:aroben) 2010-10-25 13:15:18 PDT
The JS being evaluated is "javascript:window.layoutTestController.notifyDone()"
Comment 10 Adam Roben (:aroben) 2010-10-25 13:27:07 PDT
I just reproed this again.

memcpy is being called with these parameters:

memcpy(dst=0x020372c8, src=0x0205e760, count=84)

&m_pluginElement->m_document is 0x020372e4, which is only 27 bytes after 0x020372c8.
Comment 11 Adam Roben (:aroben) 2010-10-25 13:33:00 PDT
The Vector is expanding from a capacity if 21 elements (84 bytes) to 27 elements (108 bytes).
Comment 12 Adam Roben (:aroben) 2010-10-25 13:50:01 PDT
Geoff helped me to figure out that m_pluginElement has been destroyed even before PluginView::evaluate is called. That's the issue here.
Comment 13 Adam Roben (:aroben) 2010-10-25 13:53:21 PDT
m_pluginElement is getting destroyed due to document.open being called. It's likely that we're hitting the crash that this test is intended to expose.
Comment 14 Adam Roben (:aroben) 2010-10-25 14:10:31 PDT
It looks like WebKit1 doesn't depend on the HTMLPlugInElement when NPN_Evaluate is called (at least not directly).
Comment 15 Adam Roben (:aroben) 2010-10-28 07:26:02 PDT
(In reply to comment #14)
> It looks like WebKit1 doesn't depend on the HTMLPlugInElement when NPN_Evaluate is called (at least not directly).

In fact, by the time NPN_Evaluate is called in WebKit1 the whole PluginView has been destroyed.
Comment 16 Adam Roben (:aroben) 2010-10-28 08:00:28 PDT
In WebKit2, the PluginView is kept alive by NPRuntimeObjectMap.
Comment 17 Adam Roben (:aroben) 2010-10-28 08:22:51 PDT
Anders suggested that PluginView should retain its HTMLPlugInElement, just like WebBaseNetscapePluginView does. I tried that, and it fixes the test. The HTMLPlugInElement does not get leaked, either.
Comment 18 Adam Roben (:aroben) 2010-10-28 09:10:02 PDT
Created attachment 72194 [details]
Make PluginView retain its HTMLPlugInElement
Comment 19 Adam Roben (:aroben) 2010-10-28 09:17:38 PDT
Committed r70781: <http://trac.webkit.org/changeset/70781>