Bug 13584 – <script> code wrongly assumes requests can't fail

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

This must be a recent regression:

This code:

void HTMLScriptElement::parseMappedAttribute(MappedAttribute *attr)
{
    const QualifiedName& attrName = attr->name();
    if (attrName == srcAttr) {
        if (m_evaluated || m_cachedScript || m_createdByParser || !inDocument())
            return;

        // FIXME: Evaluate scripts in viewless documents.
        // See http://bugs.webkit.org/show_bug.cgi?id=5727
        if (!document()->frame())
            return;
    
        const AtomicString& url = attr->value();
        if (!url.isEmpty()) {
            m_cachedScript = document()->docLoader()->requestScript(url, getAttribute(charsetAttr));
            m_cachedScript->ref(this);
        }
    } 

Assumes that requestScript() will never return 0.  That doesn't seem to be the case looking at requestScript.

I'm not sure what the test case looks like yet, but I'm certain it's possible to make one.


#0	0x0126df72 in WebCore::HTMLScriptElement::parseMappedAttribute at HTMLScriptElement.cpp:84
#1	0x0121ea97 in WebCore::StyledElement::attributeChanged at StyledElement.cpp:178
#2	0x0122448d in WebCore::NamedAttrMap::addAttribute at NamedAttrMap.cpp:287
#3	0x01227fd0 in WebCore::Element::setAttribute at Element.cpp:473
#4	0x01228181 in WebCore::Element::setAttribute at Element.cpp:154
#5	0x0126e128 in WebCore::HTMLScriptElement::setSrc at HTMLScriptElement.cpp:333
#6	0x012b4d61 in WebCore::JSHTMLScriptElement::putValueProperty at JSHTMLScriptElement.cpp:180
#7	0x015793d6 in KJS::lookupPut<WebCore::JSHTMLScriptElement> at lookup.h:252
#8	0x01579418 in KJS::lookupPut<WebCore::JSHTMLScriptElement, KJS::JSHTMLElement> at lookup.h:268
#9	0x012b5083 in WebCore::JSHTMLScriptElement::put at JSHTMLScriptElement.cpp:151
#10	0x005479c2 in KJS::AssignDotNode::evaluate at nodes.cpp:1498
#11	0x00541f0f in KJS::ExprStatementNode::execute at nodes.cpp:1723
#12	0x0053f0e7 in KJS::SourceElementsNode::execute at nodes.cpp:2522
#13	0x00517f1c in KJS::BlockNode::execute at nodes.cpp:1699
#14	0x0053d3c0 in KJS::GlobalFuncImp::callAsFunction at function.cpp:803
#15	0x0051b3b6 in KJS::JSObject::call at object.cpp:97
#16	0x00544e3b in KJS::FunctionCallResolveNode::evaluate at nodes.cpp:694
#17	0x00541f0f in KJS::ExprStatementNode::execute at nodes.cpp:1723
#18	0x0053f1f2 in KJS::SourceElementsNode::execute at nodes.cpp:2528
#19	0x00517f1c in KJS::BlockNode::execute at nodes.cpp:1699
#20	0x00517fe7 in KJS::DeclaredFunctionImp::execute at function.cpp:317
#21	0x00532fc9 in KJS::FunctionImp::callAsFunction at function.cpp:104
#22	0x0051b3b6 in KJS::JSObject::call at object.cpp:97
#23	0x00544e3b in KJS::FunctionCallResolveNode::evaluate at nodes.cpp:694
#24	0x00541f0f in KJS::ExprStatementNode::execute at nodes.cpp:1723
#25	0x0053f1f2 in KJS::SourceElementsNode::execute at nodes.cpp:2528
#26	0x00517f1c in KJS::BlockNode::execute at nodes.cpp:1699
#27	0x00517fe7 in KJS::DeclaredFunctionImp::execute at function.cpp:317
#28	0x00532fc9 in KJS::FunctionImp::callAsFunction at function.cpp:104
#29	0x0051b3b6 in KJS::JSObject::call at object.cpp:97
#30	0x00544e3b in KJS::FunctionCallResolveNode::evaluate at nodes.cpp:694
#31	0x00541f0f in KJS::ExprStatementNode::execute at nodes.cpp:1723
#32	0x0053f0e7 in KJS::SourceElementsNode::execute at nodes.cpp:2522
#33	0x00517f1c in KJS::BlockNode::execute at nodes.cpp:1699
#34	0x00517fe7 in KJS::DeclaredFunctionImp::execute at function.cpp:317
#35	0x00532fc9 in KJS::FunctionImp::callAsFunction at function.cpp:104
#36	0x0051b3b6 in KJS::JSObject::call at object.cpp:97
#37	0x0123d07a in KJS::JSAbstractEventListener::handleEvent at kjs_events.cpp:123
#38	0x010c8c18 in WebCore::Document::handleWindowEvent at Document.cpp:2341
#39	0x012097e7 in WebCore::EventTargetNode::dispatchWindowEvent at EventTargetNode.cpp:337
#40	0x010cb057 in WebCore::Document::implicitClose at Document.cpp:1382
#41	0x0137a8c3 in WebCore::FrameLoader::checkEmitLoadEvent at FrameLoader.cpp:1187
#42	0x01384a14 in WebCore::FrameLoader::checkCompleted at FrameLoader.cpp:1144
#43	0x013859be in WebCore::FrameLoader::finishedParsing at FrameLoader.cpp:1094
#44	0x010c6b26 in WebCore::Document::finishedParsing at Document.cpp:3414
#45	0x0101b86f in WebCore::HTMLParser::finished at HTMLParser.cpp:1411
#46	0x010201bf in WebCore::HTMLTokenizer::end at HTMLTokenizer.cpp:1501
#47	0x0102057f in WebCore::HTMLTokenizer::finish at HTMLTokenizer.cpp:1541
#48	0x010c53c0 in WebCore::Document::finishParsing at Document.cpp:1530
#49	0x01387240 in WebCore::FrameLoader::endIfNotLoading at FrameLoader.cpp:964
#50	0x01387281 in WebCore::FrameLoader::end at FrameLoader.cpp:948
#51	0x0138a1f2 in WebCore::DocumentLoader::finishedLoading at DocumentLoader.cpp:315
#52	0x01381bec in WebCore::FrameLoader::finishedLoading at FrameLoader.cpp:2571
#53	0x0138bf99 in WebCore::MainResourceLoader::didFinishLoading at MainResourceLoader.cpp:300
#54	0x0138d7c6 in WebCore::ResourceLoader::didFinishLoading at ResourceLoader.cpp:335
#55	0x0136c5d9 in -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] at ResourceHandleMac.mm:369
#56	0x92854d74 in -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback]
#57	0x92852e19 in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]
#58	0x92852ab5 in _sendCallbacks
#59	0x9082bf92 in CFRunLoopRunSpecific
#60	0x9082bace in CFRunLoopRunInMode
#61	0x92ddc8d8 in RunCurrentEventLoopInMode
#62	0x92ddbfe2 in ReceiveNextEventCommon
#63	0x92ddbe39 in BlockUntilNextEventMatchingListInMode
#64	0x93282465 in _DPSNextEvent
#65	0x93282056 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
#66	0x00006f96 in ??
#67	0x9327bddb in -[NSApplication run]
#68	0x9326fd2f in NSApplicationMain
#69	0x0005f7de in ??
#70	0x0005f6f9 in ??

[Eric Seidel (no email)]

This is as of r21245

[Eric Seidel (no email)]

These seem to be the only cases in which Cache::requestResource would return 0:

    if (resource->type() != type)
        return 0;

#if USE(LOW_BANDWIDTH_DISPLAY)
    // addLowBandwidthDisplayRequest() returns true if requesting CSS or JS during low bandwidth display.
    // Here, return 0 to not block parsing or layout.
    if (docLoader->frame() && docLoader->frame()->loader()->addLowBandwidthDisplayRequest(resource))
        return 0;
#endif

All others would return a resource object (or would have called ASSERT earlier).

[Eric Seidel (no email)]

The test case for http://bugs.webkit.org/show_bug.cgi?id=13541 attachment: http://bugs.webkit.org/attachment.cgi?id=14266 actually causes this crash.  It can be reduced further.

[Darin Adler]

<rdar://problem/5183698>

[Alexey Proskuryakov]

There's one more instance of the same issue in HTMLScriptElement::insertedIntoDocument().

[Alexey Proskuryakov]

Created attachment 14357 [details]
proposed fix

[Darin Adler]

Comment on attachment 14357 [details]
proposed fix

Don't we need to fire an errorEvent if we get back 0?

[Darin Adler]

Comment on attachment 14357 [details]
proposed fix

We do need to fire the error event.

[Alexey Proskuryakov]

Created attachment 14371 [details]
proposed fix

Dispatches errorEvent.

[Darin Adler]

Comment on attachment 14371 [details]
proposed fix

r=me

[Alexey Proskuryakov]

Committed revision 21275.