Bug 106361 – [Mac] svg/custom/text-use-click-crash.xhtml added by r139029 hits assertion in enclosingTextFormControl

• Component: Forms
• Status: RESOLVED FIXED

[Ryosuke Niwa]

http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r139078%20(4404)/results.html

Application Specific Information:
CRASHING TEST: svg/custom/text-use-click-crash.xhtml

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00000001102263f6 WebCore::enclosingTextFormControl(WebCore::Position const&) + 150 (HTMLTextFormControlElement.cpp:641)
1   com.apple.WebCore             	0x00000001100638f5 WebCore::FrameSelection::isInPasswordField() const + 37 (FrameSelection.cpp:1674)
2   com.apple.WebKit              	0x000000010f2e3190 isInPasswordField(WebCore::Frame*) + 48 (WebHTMLView.mm:3788)
3   com.apple.WebKit              	0x000000010f2ee353 -[WebHTMLView(WebNSTextInputSupport) _updateSecureInputState] + 275 (WebHTMLView.mm:6006)
4   com.apple.WebKit              	0x000000010f2ee552 -[WebHTMLView(WebNSTextInputSupport) _updateSelectionForInputManager] + 258 (WebHTMLView.mm:6040)
5   com.apple.WebKit              	0x000000010f2e973b -[WebHTMLView(WebInternal) _selectionChanged] + 43 (WebHTMLView.mm:5051)
6   com.apple.WebKit              	0x000000010f27f8d7 WebEditorClient::respondToChangedSelection(WebCore::Frame*) + 167 (WebEditorClient.mm:295)
7   com.apple.WebCore             	0x000000010ff06717 WebCore::Editor::notifyComponentsOnChangedSelection(WebCore::VisibleSelection const&, unsigned int) + 87 (Editor.cpp:485)
8   com.apple.WebCore             	0x000000010ff13ab5 WebCore::Editor::respondToChangedSelection(WebCore::VisibleSelection const&, unsigned int) + 2053 (Editor.cpp:2854)
9   com.apple.WebCore             	0x000000011005a6ad WebCore::FrameSelection::setSelection(WebCore::VisibleSelection const&, unsigned int, WebCore::FrameSelection::CursorAlignOnScroll, WebCore::TextGranularity) + 909 (FrameSelection.cpp:314)
10  com.apple.WebCore             	0x0000000110064b60 WebCore::FrameSelection::setSelection(WebCore::VisibleSelection const&, WebCore::TextGranularity) + 48 (FrameSelection.h:153)
11  com.apple.WebCore             	0x000000011005b421 WebCore::FrameSelection::setNonDirectionalSelectionIfNeeded(WebCore::VisibleSelection const&, WebCore::TextGranularity, WebCore::FrameSelection::EndPointsAdjustmentMode) + 801 (FrameSelection.cpp:250)
12  com.apple.WebCore             	0x000000010ff5cb29 WebCore::EventHandler::updateSelectionForMouseDrag(WebCore::HitTestResult const&) + 2137 (EventHandler.cpp:872)
13  com.apple.WebCore             	0x000000010ff5b7d0 WebCore::EventHandler::handleMouseDraggedEvent(WebCore::MouseEventWithHitTestResults const&) + 704 (EventHandler.cpp:739)
14  com.apple.WebCore             	0x000000010ff614da WebCore::EventHandler::handleMouseMoveEvent(WebCore::PlatformMouseEvent const&, WebCore::HitTestResult*, bool) + 2074 (EventHandler.cpp:1759)
15  com.apple.WebCore             	0x000000010ff70780 WebCore::EventHandler::mouseDragged(NSEvent*) + 128 (EventHandlerMac.mm:488)
16  com.apple.WebKit              	0x000000010f2e1fd0 -[WebHTMLView mouseDragged:] + 320 (WebHTMLView.mm:3627)
17  DumpRenderTree                	0x000000010df761b9 -[EventSendingController mouseMoveToX:Y:] + 1593 (EventSendingController.mm:487)
18  com.apple.CoreFoundation      	0x00007fff8f6e863c __invoking___ + 140
19  com.apple.CoreFoundation      	0x00007fff8f6e84d7 -[NSInvocation invoke] + 263
20  DumpRenderTree                	0x000000010df76993 +[EventSendingController replaySavedEvents] + 195 (EventSendingController.mm:579)
21  DumpRenderTree                	0x000000010df7562e -[EventSendingController mouseUp:withModifiers:] + 366 (EventSendingController.mm:404)
22  com.apple.CoreFoundation      	0x00007fff8f6e863c __invoking___ + 140
23  com.apple.CoreFoundation      	0x00007fff8f6e84d7 -[NSInvocation invoke] + 263
24  com.apple.WebCore             	0x0000000110b06b93 JSC::Bindings::ObjcInstance::invokeObjcMethod(JSC::ExecState*, JSC::Bindings::ObjcMethod*) + 1923 (objc_instance.mm:320)
25  com.apple.WebCore             	0x0000000110b063f0 JSC::Bindings::ObjcInstance::invokeMethod(JSC::ExecState*, JSC::RuntimeMethod*) + 304 (objc_instance.mm:232)
26  com.apple.WebCore             	0x0000000110ef57a2 JSC::callRuntimeMethod(JSC::ExecState*) + 530 (runtime_method.cpp:115)
27  com.apple.JavaScriptCore      	0x000000010e450779 JSC::LLInt::handleHostCall(JSC::ExecState*, JSC::Instruction*, JSC::JSValue, JSC::CodeSpecializationKind) + 329 (LLIntSlowPaths.cpp:1362)
28  com.apple.JavaScriptCore      	0x000000010e45169c JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 92 (LLIntSlowPaths.cpp:1406)
29  com.apple.JavaScriptCore      	0x000000010e451626 JSC::LLInt::genericCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind) + 246 (LLIntSlowPaths.cpp:1462)
30  com.apple.JavaScriptCore      	0x000000010e44e71c llint_slow_path_call + 60 (LLIntSlowPaths.cpp:1468)
31  com.apple.JavaScriptCore      	0x000000010e4573b1 llint_op_call + 184
32  com.apple.JavaScriptCore      	0x000000010e338cf4 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::JSGlobalData*) + 84 (JITCode.h:134)
33  com.apple.JavaScriptCore      	0x000000010e33577f JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 4735 (Interpreter.cpp:983)
34  com.apple.JavaScriptCore      	0x000000010e1b8903 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) + 483 (Completion.cpp:75)
35  com.apple.WebCore             	0x00000001106b3bb2 WebCore::JSMainThreadExecState::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) + 82 (JSMainThreadExecState.h:77)
36  com.apple.WebCore             	0x0000000110f10123 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld*) + 339 (ScriptController.cpp:141)
37  com.apple.WebCore             	0x0000000110f10264 WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) + 68 (ScriptController.cpp:158)
38  com.apple.WebCore             	0x0000000110f282ea WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) + 746 (ScriptElement.cpp:304)
39  com.apple.WebCore             	0x0000000110f2707d WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) + 1693 (ScriptElement.cpp:242)
40  com.apple.WebCore             	0x00000001113b3845 WebCore::XMLDocumentParser::endElementNs() + 709 (XMLDocumentParserLibxml2.cpp:886)
41  com.apple.WebCore             	0x00000001113b4cb4 WebCore::endElementNsHandler(void*, unsigned char const*, unsigned char const*, unsigned char const*) + 68 (XMLDocumentParserLibxml2.cpp:1099)
42  libxml2.2.dylib               	0x00007fff8e5f885d xmlParseEndTag2 + 782
43  libxml2.2.dylib               	0x00007fff8e5fa760 xmlParseTryOrFinish + 2390
44  libxml2.2.dylib               	0x00007fff8e5f9b5a xmlParseChunk + 230
45  com.apple.WebCore             	0x00000001113b25eb WebCore::XMLDocumentParser::doWrite(WTF::String const&) + 363 (XMLDocumentParserLibxml2.cpp:662)
46  com.apple.WebCore             	0x00000001113af90c WebCore::XMLDocumentParser::append(WebCore::SegmentedString const&) + 188 (XMLDocumentParser.cpp:128)
47  com.apple.WebCore             	0x000000010fce5047 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) + 199 (DecodedDataDocumentParser.cpp:50)
48  com.apple.WebCore             	0x000000010fda6693 WebCore::DocumentWriter::addData(char const*, unsigned long) + 259 (DocumentWriter.cpp:222)
49  com.apple.WebCore             	0x000000010fd70507 WebCore::DocumentLoader::commitData(char const*, unsigned long) + 583 (DocumentLoader.cpp:357)
50  com.apple.WebKit              	0x000000010f28d1d3 -[WebFrame(WebInternal) _commitData:] + 211 (WebFrame.mm:826)
51  com.apple.WebKit              	0x000000010f2cb420 -[WebHTMLRepresentation receivedData:withDataSource:] + 128 (WebHTMLRepresentation.mm:186)
52  com.apple.WebKit              	0x000000010f26bb1a -[WebDataSource(WebInternal) _receivedData:] + 90 (WebDataSource.mm:215)
53  com.apple.WebKit              	0x000000010f29b581 WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 129 (WebFrameLoaderClient.mm:846)
54  com.apple.WebCore             	0x000000010fd705e0 WebCore::DocumentLoader::commitLoad(char const*, int) + 208 (DocumentLoader.cpp:319)
55  com.apple.WebCore             	0x000000010fd70adb WebCore::DocumentLoader::receivedData(char const*, int) + 59 (DocumentLoader.cpp:388)
56  com.apple.WebCore             	0x0000000110a1d13f WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int) + 1167 (MainResourceLoader.cpp:499)
57  com.apple.WebCore             	0x000000010fa4e388 WebCore::CachedRawResource::data(WTF::PassRefPtr<WebCore::ResourceBuffer>, bool) + 600 (CachedRawResource.cpp:70)
58  com.apple.WebCore             	0x00000001111040d5 WebCore::SubresourceLoader::sendDataToResource(char const*, int) + 405 (SubresourceLoader.cpp:253)
59  com.apple.WebCore             	0x000000011110432b WebCore::SubresourceLoader::didReceiveData(char const*, int, long long, bool) + 395 (SubresourceLoader.cpp:226)
60  com.apple.WebCore             	0x0000000110ebd75f WebCore::ResourceLoader::didReceiveData(WebCore::ResourceHandle*, char const*, int, int) + 143 (ResourceLoader.cpp:452)
61  com.apple.WebCore             	0x0000000110eba12a -[WebCoreResourceHandleAsDelegate connection:didReceiveData:lengthReceived:] + 298 (ResourceHandleMac.mm:785)
62  com.apple.Foundation          	0x00007fff8c606f58 __65-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:]_block_invoke_0 + 28
63  com.apple.Foundation          	0x00007fff8c606e9c -[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:] + 227
64  com.apple.Foundation          	0x00007fff8c606d98 -[NSURLConnectionInternal _withActiveConnectionAndDelegate:] + 63
65  com.apple.Foundation          	0x00007fff8c60992b _NSURLConnectionDidReceiveData_LengthReceived + 86
66  com.apple.CFNetwork           	0x00007fff94f2b7b1 ___delegate_didReceiveDataArray_block_invoke_0 + 132
67  com.apple.CFNetwork           	0x00007fff94f1e753 ___withDelegateAsync_block_invoke_0 + 90
68  com.apple.CFNetwork           	0x00007fff94fad2ca __block_global_1 + 28
69  com.apple.CoreFoundation      	0x00007fff8f692724 CFArrayApplyFunction + 68
70  com.apple.CFNetwork           	0x00007fff94f0fa6c RunloopBlockContext::perform() + 126
71  com.apple.CFNetwork           	0x00007fff94f0f94b MultiplexerSource::perform() + 221
72  com.apple.CoreFoundation      	0x00007fff8f674101 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
73  com.apple.CoreFoundation      	0x00007fff8f673a25 __CFRunLoopDoSources0 + 245
74  com.apple.CoreFoundation      	0x00007fff8f696dc5 __CFRunLoopRun + 789
75  com.apple.CoreFoundation      	0x00007fff8f6966b2 CFRunLoopRunSpecific + 290
76  com.apple.Foundation          	0x00007fff8c68489e -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 268
77  DumpRenderTree                	0x000000010df69809 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 5017 (DumpRenderTree.mm:1382)

[Ryosuke Niwa]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=svg%2Fcustom%2Ftext-use-click-crash.xhtml

[Ryosuke Niwa]

Added a test expectation in http://trac.webkit.org/changeset/139092.

[Hajime Morrita]

Created attachment 181879 [details]
Patch

[Darin Adler]

Comment on attachment 181879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181879&action=review

> Source/WebCore/ChangeLog:9
> +        In most case this assuption makes sense but it can be broken when isInPasswordField().

Typo: assuption

> Source/WebCore/ChangeLog:10
> +        This change checks precondiiton before invoking enclosingTextFormControl() there.

Typo: precondiiton

> Source/WebCore/editing/FrameSelection.cpp:1676
> +    if (!start().containerNode())
> +        return false;
>      HTMLTextFormControlElement* textControl = enclosingTextFormControl(start());

I think we should rethink and loosen the assertion in enclosingTextFormControl instead of doing this. The actual implementation of that function is perfectly fine, it’s just got an overzealous assertion in it.

I hate the idea of checking something here that’s also checked first thing in the enclosingTextFormControl function, just because the assertion in there is too strong.

Since Ryosuke wrote that assertion, I’d like him to comment on this.

[Ryosuke Niwa]

Comment on attachment 181879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181879&action=review

>> Source/WebCore/ChangeLog:9
>> +        In most case this assuption makes sense but it can be broken when isInPasswordField().
> 
> Typo: assuption

When do we have such an exotic selection start? There lies a deeper problem. start() should never be a position before/after a node that doesn't have any parent.
We must be not updating selection properly somewhere.

[Hajime Morrita]

(In reply to comment #5)
> (From update of attachment 181879 [details])
> When do we have such an exotic selection start? There lies a deeper problem. start() should never be a position before/after a node that doesn't have any parent.
> We must be not updating selection properly somewhere.

I need to investigate further.
The outline here is that changing @requestedFeatures causes re-attach(),
which causes SVG shadow tree reconstruction.
The selection was in the shadow tree and it looks that it's been staying there even after the reconstruction.

[Ryosuke Niwa]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 181879 [details] [details])
> > When do we have such an exotic selection start? There lies a deeper problem. start() should never be a position before/after a node that doesn't have any parent.
> > We must be not updating selection properly somewhere.
> 
> I need to investigate further.
> The outline here is that changing @requestedFeatures causes re-attach(),
> which causes SVG shadow tree reconstruction.
> The selection was in the shadow tree and it looks that it's been staying there even after the reconstruction.

That sounds like a problem shinyak in fixed in http://trac.webkit.org/changeset/139401. Maybe you guys can talk about it in person?

[Hajime Morrita]

> 
> That sounds like a problem shinyak in fixed in http://trac.webkit.org/changeset/139401. Maybe you guys can talk about it in person?

I tried with the latest build but it still crashes. 
Well, shinyak might be a better person to attack this :-/

[Shinya Kawanaka]

(In reply to comment #8)
> > 
> > That sounds like a problem shinyak in fixed in http://trac.webkit.org/changeset/139401. Maybe you guys can talk about it in person?
> 
> I tried with the latest build but it still crashes. 
> Well, shinyak might be a better person to attack this :-/

mmm. I'll take a look.

[Shinya Kawanaka]

https://bugs.webkit.org/show_bug.cgi?id=106533
https://bugs.webkit.org/show_bug.cgi?id=107089

This anticipation is realized...? I'm not sure yet.

[Shinya Kawanaka]

My investigation is...

When ASSERT hits, position.anchorNode() is /use/shadow-root/text#target, and anchorType is PositionIsBeforeAnchor.
Since SHADOW_DOM flag is not enabled in mac ports, position.containerNode() cannot be ShadowRoot. Instead, position.containerNode() returns null.

If I allowed Position to have ShadowRoot, this ASSERT was not triggered. So I believe this investigation is correct.

Here we have two options:
1) Allows Position to have ShadowRoot as container node.
2) Loose ASSERT condition in enclosingTextFormControl.

(1) will affect a lot of code, I would like to choose (2) this time.

[Shinya Kawanaka]

Created attachment 183148 [details]
Patch

[WebKit Review Bot]

Comment on attachment 183148 [details]
Patch

Clearing flags on attachment: 183148

Committed r139999: <http://trac.webkit.org/changeset/139999>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

<rdar://problem/12975228>