Bug 14027 – ASSERT in becomeFirstResponder (focus) code when hitting tab in an XML document

• Component: XML
• Status: NEW

[Eric Seidel (no email)]

Infinite loop in focus code when hitting tab in an XML document

See the attached test case.  This reproduces in DRT every time.  I expect it should reproduce in Safari as well, given the proper combination of tabs and modifier keys.

If this is DRT only it could perhaps be downgraded to a P2.

[Eric Seidel (no email)]

Created attachment 14897 [details]
test case (doens't seem to crash if served as HTML)

[Eric Seidel (no email)]

Adding sullivan and thatcher to the CC as they both touched the line of code in question.

[Eric Seidel (no email)]

Created attachment 14898 [details]
slightly better test (crashes under DRT)

[Eric Seidel (no email)]

The backtrace for the crash:

#0	0x0026426c in -[WebView becomeFirstResponder] at WebView.mm:2251
#1	0x932cc483 in -[NSWindow makeFirstResponder:]
#2	0x93380fef in -[NSWindow selectKeyViewFollowingView:]
#3	0x002951d8 in WebChromeClient::takeFocus at WebChromeClient.mm:119
#4	0x013efedb in WebCore::Chrome::takeFocus at Chrome.cpp:85
#5	0x0143227f in WebCore::FocusController::advanceFocus at FocusController.cpp:155
#6	0x0143253c in WebCore::FocusController::advanceFocus at FocusController.cpp:112
#7	0x002477d2 in -[WebHTMLView becomeFirstResponder] at WebHTMLView.mm:3142
#8	0x932cc483 in -[NSWindow makeFirstResponder:]
#9	0x9344fcd3 in -[NSClipView becomeFirstResponder]
#10	0x932cc483 in -[NSWindow makeFirstResponder:]
#11	0x9344fc34 in -[NSScrollView becomeFirstResponder]
#12	0x932cc483 in -[NSWindow makeFirstResponder:]
#13	0x0025a8f3 in -[WebFrameView becomeFirstResponder] at WebFrameView.mm:391
#14	0x932cc483 in -[NSWindow makeFirstResponder:]
#15	0x002643a8 in -[WebView becomeFirstResponder] at WebView.mm:2275
#16	0x932cc483 in -[NSWindow makeFirstResponder:]
#17	0x93380fef in -[NSWindow selectKeyViewFollowingView:]
#18	0x002951d8 in WebChromeClient::takeFocus at WebChromeClient.mm:119
#19	0x013efedb in WebCore::Chrome::takeFocus at Chrome.cpp:85
#20	0x0143227f in WebCore::FocusController::advanceFocus at FocusController.cpp:155
#21	0x0143253c in WebCore::FocusController::advanceFocus at FocusController.cpp:112
#22	0x014083e1 in WebCore::EventHandler::defaultTabEventHandler at EventHandler.cpp:1681
#23	0x01409463 in WebCore::EventHandler::defaultKeyboardEventHandler at EventHandler.cpp:1419
#24	0x01239c99 in WebCore::EventTargetNode::defaultEventHandler at EventTargetNode.cpp:583
#25	0x01237fc4 in WebCore::EventTargetNode::dispatchGenericEvent at EventTargetNode.cpp:267
#26	0x01239909 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:308
#27	0x01239985 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:292
#28	0x01409350 in WebCore::EventHandler::defaultKeyboardEventHandler at EventHandler.cpp:1409
#29	0x01239c99 in WebCore::EventTargetNode::defaultEventHandler at EventTargetNode.cpp:583
#30	0x01237fc4 in WebCore::EventTargetNode::dispatchGenericEvent at EventTargetNode.cpp:267
#31	0x01239909 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:308
#32	0x01239985 in WebCore::EventTargetNode::dispatchEvent at EventTargetNode.cpp:292
#33	0x01238f3a in WebCore::EventTargetNode::dispatchKeyEvent at EventTargetNode.cpp:370
#34	0x014081a4 in WebCore::EventHandler::keyEvent at EventHandler.cpp:1375
#35	0x01405265 in WebCore::EventHandler::keyEvent at EventHandlerMac.mm:138
#36	0x0023f128 in -[WebHTMLView keyDown:] at WebHTMLView.mm:3419
#37	0x00004dad in -[EventSendingController keyDown:withModifiers:] at EventSendingController.m:367
#38	0x90a5ac56 in objc_msgSendv
#39	0x927f53b2 in -[NSInvocation invoke]
#40	0x003e1486 in KJS::Bindings::ObjcInstance::invokeMethod at objc_instance.mm:187
#41	0x003dcfd5 in KJS::RuntimeMethod::callAsFunction at runtime_method.cpp:89
#42	0x0041e24e in KJS::JSObject::call at object.cpp:98
#43	0x0044773d in KJS::FunctionCallDotNode::evaluate at nodes.cpp:790
#44	0x00444e93 in KJS::ExprStatementNode::execute at nodes.cpp:1723
#45	0x00444d9d in KJS::IfNode::execute at nodes.cpp:1742
#46	0x00442176 in KJS::SourceElementsNode::execute at nodes.cpp:2528
#47	0x0041adb4 in KJS::BlockNode::execute at nodes.cpp:1699
#48	0x0043f301 in KJS::Interpreter::evaluate at interpreter.cpp:365
#49	0x0127901f in WebCore::KJSProxy::evaluate at kjs_proxy.cpp:78
#50	0x013df4bb in WebCore::FrameLoader::executeScript at FrameLoader.cpp:712
#51	0x0102cd6f in WebCore::XMLTokenizer::endElementNs at XMLTokenizer.cpp:753
#52	0x0102ce07 in endElementNsHandler at XMLTokenizer.cpp:985
#53	0x91bff515 in xmlParseNotationDecl
#54	0x91be4d86 in xmlParseChunk
#55	0x0102be90 in WebCore::XMLTokenizer::write at XMLTokenizer.cpp:566
#56	0x013d4075 in WebCore::FrameLoader::write at FrameLoader.cpp:927
#57	0x013d41a7 in WebCore::FrameLoader::addData at FrameLoader.cpp:1583
#58	0x01102a01 in -[WebCoreFrameBridge addData:] at WebCoreFrameBridge.mm:288
#59	0x01105b9c in -[WebCoreFrameBridge receivedData:textEncodingName:] at WebCoreFrameBridge.mm:1427
#60	0x00233245 in -[WebHTMLRepresentation receivedData:withDataSource:] at WebHTMLRepresentation.mm:173
#61	0x0022e73b in -[WebDataSource(WebInternal) _receivedData:] at WebDataSource.mm:176
#62	0x002917c5 in WebFrameLoaderClient::committedLoad at WebFrameLoaderClient.mm:658
#63	0x013d0f6b in WebCore::FrameLoader::committedLoad at FrameLoader.cpp:3027
#64	0x013e1acd in WebCore::DocumentLoader::commitLoad at DocumentLoader.cpp:347
#65	0x013e1b26 in WebCore::DocumentLoader::receivedData at DocumentLoader.cpp:359
#66	0x013d0465 in WebCore::FrameLoader::receivedData at FrameLoader.cpp:2039
#67	0x013e33f0 in WebCore::MainResourceLoader::addData at MainResourceLoader.cpp:133
#68	0x013e5599 in WebCore::ResourceLoader::didReceiveData at ResourceLoader.cpp:208
#69	0x013e3725 in WebCore::MainResourceLoader::didReceiveData at MainResourceLoader.cpp:289
#70	0x013e51a0 in WebCore::ResourceLoader::didReceiveData at ResourceLoader.cpp:330
#71	0x013c0100 in -[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:] at ResourceHandleMac.mm:351
#72	0x92854afa in -[NSURLConnection(NSURLConnectionInternal) _sendDidReceiveDataCallback]
#73	0x92852ddb in -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks]
#74	0x92852ab5 in _sendCallbacks
#75	0x9082bf92 in CFRunLoopRunSpecific
#76	0x9082bace in CFRunLoopRunInMode
#77	0x92823d3a in -[NSRunLoop runMode:beforeDate:]
#78	0x0000b09d in runTest at DumpRenderTree.m:1458
#79	0x00007008 in dumpRenderTree at DumpRenderTree.m:531
#80	0x000071e6 in main at DumpRenderTree.m:572

[John Sullivan]

Bug title says "Infinite loop in focus code", but stack trace shows a crash that doesn't seem to involve an infinite loop.

[Eric Seidel (no email)]

There is an ASSERT (which is where it crashes) to prevent the infinite loop.  If the ASSERT were not there, it would infinite loop.  Looking at it again: in release builds, there is no ASSERT and it just bails out, so it won't ever crash.  Meaning this can be a p2.

There is a comment in the code where this ASSERT is:
        // Fix for unrepro infinite recursion reported in radar 4448181. If we hit this assert on
        // a debug build, we should figure out what causes the problem and do a better fix.
Well... I guess we've just made 4448181 reproducible. :)

[John Sullivan]

That's good news.

This isn't very high priority at the moment, so if it's getting in the way of layout tests or something like that, we could change the ASSERT to some kind of log.

[Eric Seidel (no email)]

I agree.  I added you as CC in case you happened to recognize the code or had some brilliant inspiration for a fix.  I'm certain you all have plenty more important things to be working on at current (with WWDC around the corner).  The SVG change which exposed this bug to me can still land, and this bug will eventually get fixed and landed at a later time.

[David Kilzer (:ddkilzer)]

Per Comment #6:

<rdar://problem/4448181> (closed)
<rdar://problem/5125128> (follow-up)

(Such interesting radar numbers!)

[John Sullivan]

I'm glad you cc:ed me on this, and I'm very glad to have a reproducible case. Hopefully we'll have time to figure out exactly what's going on in the not too distant future.

[Eric Seidel (no email)]

When this finally is fixed the layout test from bug 8823 should be landed as well.

[David Kilzer (:ddkilzer)]

See also Bug 13299.

[jay]

what is the delay?

I'm anxious that keyboard navigation be implemented, but it seems that this has not been resolved.

cheers.