Bug 7363 – REGRESSION (r12872): Repro crash when clicking the Quick Reply box in Gmail

• Component: New Bugs
• Status: RESOLVED FIXED

[musti]

Steps:
Logon to GMail
Click on any message in Inbox
Click on the Quick Reply box to enter a reply
You see the input box with Check Spelling, then WebKit crashes.

Crash Report below:
Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   com.apple.WebCore        	0x01180b20 WebCore::DocumentImpl::setFocusNode(KXMLCore::PassRefPtr<WebCore::NodeImpl>) + 400
1   com.apple.WebCore        	0x010b0cd4 WebCore::ElementImpl::focus() + 116
2   com.apple.WebCore        	0x01064b10 KJS::HTMLElementFunction::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 1136
3   com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
4   com.apple.JavaScriptCore 	0x0012badc KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) + 524
5   com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
6   com.apple.JavaScriptCore 	0x001337d8 KJS::SourceElementsNode::execute(KJS::ExecState*) + 488
7   com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
8   com.apple.JavaScriptCore 	0x0013330c KJS::TryNode::execute(KJS::ExecState*) + 108
9   com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
10  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
11  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
12  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
13  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
14  com.apple.JavaScriptCore 	0x0012c27c KJS::FunctionCallResolveNode::evaluate(KJS::ExecState*) + 524
15  com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
16  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
17  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
18  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
19  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
20  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
21  com.apple.JavaScriptCore 	0x0012c27c KJS::FunctionCallResolveNode::evaluate(KJS::ExecState*) + 524
22  com.apple.JavaScriptCore 	0x00130468 KJS::ExprStatementNode::execute(KJS::ExecState*) + 104
23  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
24  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
25  com.apple.JavaScriptCore 	0x00130744 KJS::IfNode::execute(KJS::ExecState*) + 484
26  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
27  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
28  com.apple.JavaScriptCore 	0x00130744 KJS::IfNode::execute(KJS::ExecState*) + 484
29  com.apple.JavaScriptCore 	0x001337d8 KJS::SourceElementsNode::execute(KJS::ExecState*) + 488
30  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
31  com.apple.JavaScriptCore 	0x0013330c KJS::TryNode::execute(KJS::ExecState*) + 108
32  com.apple.JavaScriptCore 	0x001336ec KJS::SourceElementsNode::execute(KJS::ExecState*) + 252
33  com.apple.JavaScriptCore 	0x00130398 KJS::BlockNode::execute(KJS::ExecState*) + 152
34  com.apple.JavaScriptCore 	0x001183f8 KJS::DeclaredFunctionImp::execute(KJS::ExecState*) + 56
35  com.apple.JavaScriptCore 	0x00117d30 KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 464
36  com.apple.JavaScriptCore 	0x00139ec4 KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) + 100
37  com.apple.WebCore        	0x010766dc KJS::ScheduledAction::execute(KJS::Window*) + 236
38  com.apple.WebCore        	0x010769d4 KJS::WindowQObject::timerFired(KJS::DOMWindowTimer*) + 148
39  com.apple.WebCore        	0x012d2164 WebCore::TimerBase::fireTimers(double, KXMLCore::Vector<WebCore::TimerBase*, (unsigned long)0> const&) + 324
40  com.apple.WebCore        	0x012d2210 WebCore::TimerBase::sharedTimerFired() + 112
41  com.apple.CoreFoundation 	0x90770aec __CFRunLoopDoTimer + 184
42  com.apple.CoreFoundation 	0x9075d464 __CFRunLoopRun + 1680
43  com.apple.CoreFoundation 	0x9075ca18 CFRunLoopRunSpecific + 268
44  com.apple.HIToolbox      	0x9318f1e0 RunCurrentEventLoopInMode + 264
45  com.apple.HIToolbox      	0x9318e874 ReceiveNextEventCommon + 380
46  com.apple.HIToolbox      	0x9318e6e0 BlockUntilNextEventMatchingListInMode + 96
47  com.apple.AppKit         	0x9366c104 _DPSNextEvent + 384
48  com.apple.AppKit         	0x9366bdc8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
49  com.apple.Safari         	0x000072d4 0x1000 + 25300
50  com.apple.AppKit         	0x9366830c -[NSApplication run] + 472
51  com.apple.AppKit         	0x93758e68 NSApplicationMain + 452
52  com.apple.Safari         	0x0005cfdc 0x1000 + 376796
53  com.apple.Safari         	0x0005ce80 0x1000 + 376448

Created attachment 6612 [details]
Test case (crasher)

To see the problem, open the test case and click where indicated. The second time you click, Safari will crash.

This is a regression from r12872 (the fix for <rdar://problem/4315673> and <rdar://problem/4447009>). The crash happens when the element losing focus no longer has a renderer. The fix may be as simple as adding a null check for r right here in DocumentImpl::setFocusNode:

        // Dispatch a change event for text fields or textareas that have been edited
        RenderObject *r = static_cast<RenderObject*>(oldFocusNode.get()->renderer());
        if ((r->isTextArea() || r->isTextField()) && r->isEdited()) {
            oldFocusNode->dispatchHTMLEvent(changeEvent, true, false);
            r->setEdited(false);
        }

However I'm not sure about the implications of not dispatching the changeEvent in this case.

[Darin Adler]

The code quoted here has another problem, too. After calling dispatchHTMLEvent, the renderer may be gone, so we can't just call r->setEdited(false) without first re-getting that renderer from the DOM node.

[Graham Dennis]

*** Bug 7398 has been marked as a duplicate of this bug. ***

[Graham Dennis]

Created attachment 6641 [details]
automatic testcase (caused by detached node)

This is a second testcase where the lack of a renderer is caused by a detached node instead of 'display: none' styling.

[Vicki Murley]

I've attached the patch that fixes this bug, which Adele reviewed and I'll commit shortly.  

The second test case here ("automatic testcase"), exhibits a second bug.  Focusing a text field from an onload handler fails, b/c the node doesn't have a renderer when onload fires.  "Automatic testcase" no longer crashes, but the text field isn't focused as it should be.  This issue has already been reported in another bug (http://bugzilla.opendarwin.org/show_bug.cgi?id=7405).  The attached patch doesn't aim to fix this second bug.

[Vicki Murley]

Created attachment 6650 [details]
patch

[Vicki Murley]

I haven't committed a test case for this yet b/c tests that use eventSender are hanging from time to time (see Justin's comment in the Changelog).  I'm seeing this hang in the test case I'm working on.

[Alice Liu]

<rdar://problem/4462712>