Bug 174560

Summary: REGRESSION(r218910): Crash when password field changes to text field
Product: WebKit Reporter: Daniel Bates <dbates>
Component: AccessibilityAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, buildbot, cfleizach, commit-queue, dmazzoni, d_russell, jcraig, jdiggs, n_wang, rniwa, samuel_white, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Test case
none
Fixes the bug
none
Patch for landing commit-queue: commit-queue-

Description Daniel Bates 2017-07-15 21:37:59 PDT 
Created attachment 315592 [details]
Test case

Perform the following:

1. Open the attached test case.
2. Click the password field to focus it.

Then WebKit will crash in AXObjectCache::textMarkerDataForFirstPositionInTextControl() because we deference nullptr returned by AXObjectCache::getOrCreate().

<rdar://problem/33261681>
Comment 1 Daniel Bates 2017-07-15 21:38:37 PDT 
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
  * frame #0: 0x0000000104622f9c WebCore`WebCore::AccessibilityObject::axObjectID(this=0x0000000000000000) const at AccessibilityObject.h:795
    frame #1: 0x00000001047f223f WebCore`WebCore::AXObjectCache::textMarkerDataForFirstPositionInTextControl(this=0x000000011b99f380, textControl=0x000000011b9dd380) at AXObjectCache.cpp:2199
    frame #2: 0x000000010720ca7f WebCore`::-[WebAccessibilityObjectWrapper textMarkerForFirstPositionInTextControl:](self=0x0000610000000cf0, _cmd="textMarkerForFirstPositionInTextControl:", textControl=0x000000011b9dd380) at WebAccessibilityObjectWrapperMac.mm:703
    frame #3: 0x0000000104818930 WebCore`WebCore::addTextMarkerFor(change=2 key/value pairs, object=0x000000011b9ed5a0, textControl=0x000000011b9dd380) at AXObjectCacheMac.mm:420
    frame #4: 0x00000001048184ba WebCore`NSDictionary* WebCore::textReplacementChangeDictionary<WebCore::HTMLTextFormControlElement>(object=0x000000011b9ed5a0, type=AXTextEditTypeInsert, string={ length = 10, contents = 'Click here' }, markerTarget=0x000000011b9dd380) at AXObjectCacheMac.mm:438
    frame #5: 0x00000001048182d1 WebCore`WebCore::AXObjectCache::postTextReplacementPlatformNotificationForTextControl(this=0x000000011b99f380, object=0x000000011b9ed5a0, deletedText={ length = 0, contents = '' }, insertedText={ length = 10, contents = 'Click here' }, textControl=0x000000011b9dd380) at AXObjectCacheMac.mm:495
    frame #6: 0x00000001047ed6b3 WebCore`WebCore::AXObjectCache::postTextReplacementNotificationForTextControl(this=0x000000011b99f380, textControl=0x000000011b9dd380, deletedText={ length = 0, contents = '' }, insertedText={ length = 10, contents = 'Click here' }) at AXObjectCache.cpp:1308
    frame #7: 0x0000000105415833 WebCore`WebCore::HTMLTextFormControlElement::setInnerTextValue(this=0x000000011b9dd380, value={ length = 10, contents = 'Click here' }) at HTMLTextFormControlElement.cpp:582
    frame #8: 0x000000010708050c WebCore`WebCore::TextFieldInputType::updateInnerTextValue(this=0x000000011b95baa8) at TextFieldInputType.cpp:576
    frame #9: 0x000000010535df29 WebCore`WebCore::HTMLInputElement::updateType(this=0x000000011b9dd380) at HTMLInputElement.cpp:516
    frame #10: 0x000000010535f0eb WebCore`WebCore::HTMLInputElement::parseAttribute(this=0x000000011b9dd380, name=0x00007fff5fbfda28, value={ length = 4, contents = 'text' }) at HTMLInputElement.cpp:692
    frame #11: 0x0000000104f0cbf9 WebCore`WebCore::Element::attributeChanged(this=0x000000011b9dd380, name=0x00007fff5fbfda28, oldValue={ length = 8, contents = 'password' }, newValue={ length = 4, contents = 'text' }, (null)=ModifiedDirectly) at Element.cpp:1333
    frame #12: 0x0000000106e79e0f WebCore`WebCore::StyledElement::attributeChanged(this=0x000000011b9dd380, name=0x00007fff5fbfda28, oldValue={ length = 8, contents = 'password' }, newValue={ length = 4, contents = 'text' }, reason=ModifiedDirectly) at StyledElement.cpp:90
    frame #13: 0x0000000104f18eb4 WebCore`WebCore::Element::didModifyAttribute(this=0x000000011b9dd380, name=0x00007fff5fbfda28, oldValue={ length = 8, contents = 'password' }, newValue={ length = 4, contents = 'text' }) at Element.cpp:3376
    frame #14: 0x0000000104f0c7b9 WebCore`WebCore::Element::setAttributeInternal(this=0x000000011b9dd380, index=0, name=0x00007fff5fbfda98, newValue={ length = 4, contents = 'text' }, inSynchronizationOfLazyAttribute=NotInSynchronizationOfLazyAttribute) at Element.cpp:1290
    frame #15: 0x0000000104f0c491 WebCore`WebCore::Element::setAttribute(this=0x000000011b9dd380, localName={ length = 4, contents = 'type' }, value={ length = 4, contents = 'text' }) at Element.cpp:1237
    frame #16: 0x0000000105a7e2f7 WebCore`WebCore::jsElementPrototypeFunctionSetAttributeBody(state=0x00007fff5fbfdcb0, castedThis=0x000000012066c0c0, throwScope=0x00007fff5fbfdc38) at JSElement.cpp:1893
    frame #17: 0x0000000105a7324e WebCore`long long WebCore::IDLOperation<WebCore::JSElement>::call<&(state=0x00007fff5fbfdcb0, operationName="setAttribute")), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*) at JSDOMOperation.h:53
    frame #18: 0x0000000105a72fdc WebCore`WebCore::jsElementPrototypeFunctionSetAttribute(state=0x00007fff5fbfdcb0) at JSElement.cpp:1899
    frame #19: 0x00005f4f670038e8
    frame #20: 0x000000011173d183 JavaScriptCore`llint_entry at LowLevelInterpreter.asm:795
    frame #21: 0x00000001117357e7 JavaScriptCore`llintPCRangeStart at LowLevelInterpreter64.asm:256
    frame #22: 0x00000001115229fe JavaScriptCore`JSC::JITCode::execute(this=0x00000001251db500, vm=0x0000000120500000, protoCallFrame=0x00007fff5fbfdf08) at JITCode.cpp:81
    frame #23: 0x00000001114d2af5 JavaScriptCore`JSC::Interpreter::executeCall(this=0x000000011b9fcc68, callFrame=0x00000001206e00e8, function=0x0000000120678a90, callType=JS, callData=0x00007fff5fbfe520, thisValue=JSValue @ 0x00007fff5fbfe020, args=0x00007fff5fbfe408) at Interpreter.cpp:971
    frame #24: 0x0000000110c9f7f8 JavaScriptCore`JSC::call(exec=0x00000001206e00e8, functionObject=JSValue @ 0x00007fff5fbfe0a0, callType=JS, callData=0x00007fff5fbfe520, thisValue=JSValue @ 0x00007fff5fbfe098, args=0x00007fff5fbfe408) at CallData.cpp:40
    frame #25: 0x0000000110c9f909 JavaScriptCore`JSC::call(exec=0x00000001206e00e8, functionObject=JSValue @ 0x00007fff5fbfe190, callType=JS, callData=0x00007fff5fbfe520, thisValue=JSValue @ 0x00007fff5fbfe188, args=0x00007fff5fbfe408, returnedException=0x00007fff5fbfe430) at CallData.cpp:47
    frame #26: 0x0000000110c9fb8d JavaScriptCore`JSC::profiledCall(exec=0x00000001206e00e8, reason=Other, functionObject=JSValue @ 0x00007fff5fbfe220, callType=JS, callData=0x00007fff5fbfe520, thisValue=JSValue @ 0x00007fff5fbfe218, args=0x00007fff5fbfe408, returnedException=0x00007fff5fbfe430) at CallData.cpp:66
    frame #27: 0x00000001057d0b5b WebCore`WebCore::JSMainThreadExecState::profiledCall(exec=0x00000001206e00e8, reason=Other, functionObject=JSValue @ 0x00007fff5fbfe2b0, callType=JS, callData=0x00007fff5fbfe520, thisValue=JSValue @ 0x00007fff5fbfe2a8, args=0x00007fff5fbfe408, returnedException=0x00007fff5fbfe430) at JSMainThreadExecState.h:72
    frame #28: 0x0000000105aa6f19 WebCore`WebCore::JSEventListener::handleEvent(this=0x000000011b9e1b60, scriptExecutionContext=0x000000011b92e000, event=0x000000011b987258) at JSEventListener.cpp:155
    frame #29: 0x0000000104f73ac6 WebCore`WebCore::EventTarget::fireEventListeners(this=0x000000011b9dd380, event=0x000000011b987258, listeners={ size = 1, capacity = 0 }) at EventTarget.cpp:264
    frame #30: 0x0000000104f7368e WebCore`WebCore::EventTarget::fireEventListeners(this=0x000000011b9dd380, event=0x000000011b987258) at EventTarget.cpp:209
    frame #31: 0x00000001066209b1 WebCore`WebCore::Node::handleLocalEvents(this=0x000000011b9dd380, event=0x000000011b987258) at Node.cpp:2368
    frame #32: 0x0000000104f416fb WebCore`WebCore::EventContext::handleLocalEvents(this=0x000000011b9e77f8, event=0x000000011b987258) const at EventContext.cpp:54
    frame #33: 0x0000000104f419ca WebCore`WebCore::MouseOrFocusEventContext::handleLocalEvents(this=0x000000011b9e77f8, event=0x000000011b987258) const at EventContext.cpp:85
    frame #34: 0x0000000104f423f8 WebCore`WebCore::dispatchEventInDOM(event=0x000000011b987258, path=0x00007fff5fbfe928) at EventDispatcher.cpp:105
    frame #35: 0x0000000104f41e97 WebCore`WebCore::EventDispatcher::dispatchEvent(node=0x000000011b9e1958, event=0x000000011b987258) at EventDispatcher.cpp:163
    frame #36: 0x0000000106620a0d WebCore`WebCore::Node::dispatchEvent(this=0x000000011b9e1958, event=0x000000011b987258) at Node.cpp:2382
    frame #37: 0x0000000104f062bf WebCore`WebCore::Element::dispatchMouseEvent(this=0x000000011b9e1958, platformEvent=0x00007fff5fbfeed0, eventType={ length = 7, contents = 'mouseup' }, detail=1, relatedTarget=0x0000000000000000) at Element.cpp:285
    frame #38: 0x0000000104f4b21c WebCore`WebCore::EventHandler::dispatchMouseEvent(this=0x000000011b9f1600, eventType={ length = 7, contents = 'mouseup' }, targetNode=0x000000011b9e1958, (null)=true, clickCount=1, platformMouseEvent=0x00007fff5fbfeed0, setUnder=false) at EventHandler.cpp:2553
    frame #39: 0x0000000104f4dbe3 WebCore`WebCore::EventHandler::handleMouseReleaseEvent(this=0x000000011b9f1600, platformMouseEvent=0x00007fff5fbfeed0) at EventHandler.cpp:2077
    frame #40: 0x0000000104f5cdb8 WebCore`WebCore::EventHandler::mouseUp(this=0x000000011b9f1600, event=0x0000600000121180, correspondingPressureEvent=0x0000000000000000) at EventHandlerMac.mm:547
    frame #41: 0x0000000103c8b507 WebKitLegacy`::-[WebHTMLView mouseUp:](self=0x0000600000169c00, _cmd="mouseUp:", event=0x0000600000121180) at WebHTMLView.mm:4777
    frame #42: 0x00007fffbc60fb0a AppKit`-[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 1544
    frame #43: 0x00007fffbc60f136 AppKit`-[NSWindow(NSEventRouting) sendEvent:] + 541
    frame #44: 0x00007fffbc493835 AppKit`-[NSApplication(NSEvent) sendEvent:] + 1145
    frame #45: 0x00007fffbbd0e98b AppKit`-[NSApplication run] + 1002
    frame #46: 0x00007fffbbcd9372 AppKit`NSApplicationMain + 1237
    frame #47: 0x0000000100008e09 MiniBrowser`main(argc=5, argv=0x00007fff5fbff808) at main.m:32
    frame #48: 0x00007fffd4034235 libdyld.dylib`start + 1
    frame #49: 0x00007fffd4034235 libdyld.dylib`start + 1
(lldb)
Comment 2 Daniel Bates 2017-07-15 21:42:16 PDT 
(In reply to Daniel Bates from comment #0)
> Created attachment 315592 [details]
> Test case
> 
> Perform the following:
> 
> 1. Open the attached test case.
> 2. Click the password field to focus it.
> 

I forgot to mention that these steps assume that VoiceOver is enabled. You can enable VoiceOver by pressing Command-F5 or by opening System Preferences > Accessibility, click Voice Over and then click Enable VoiceOver.
Comment 3 Ryosuke Niwa 2017-07-16 18:35:43 PDT 
I'll work on this.
Comment 4 Ryosuke Niwa 2017-07-18 00:05:58 PDT 
Thanks for the reduction, Dan. This was very useful.
Comment 5 Ryosuke Niwa 2017-07-18 00:51:00 PDT 
Created attachment 315779 [details]
Fixes the bug
Comment 6 Daniel Bates 2017-07-18 09:42:33 PDT 
Comment on attachment 315779 [details]
Fixes the bug

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

> LayoutTests/accessibility/mac/input-type-change-crash-2.html:12
> +    testRunner.dumpAsText();
> +    accessibilityController.enableEnhancedAccessibility(true);
> +    internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();

This test will not run in MiniBrowser or Safari as it makes use of DRT/WKTR functionality that is not available unconditionally. I always find it convenient when a layout test can be run by hand with manual instructions.
Comment 7 Ryosuke Niwa 2017-07-18 13:45:45 PDT 
Created attachment 315829 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-07-18 14:24:10 PDT 
Comment on attachment 315829 [details]
Patch for landing

Rejecting attachment 315829 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 315829, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
    -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 219625 = f8c2d45d56c37002ff86dcf361215ec48c163462
r219626 = b56b9a00db8cda658322dbe07cd66cd444b6900f
r219627 = 31d1684934f780819588e68c0071e4c98ba0a08b
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/4143846
Comment 9 Ryosuke Niwa 2017-07-18 16:08:33 PDT 
Committed r219638: <http://trac.webkit.org/changeset/219638>