Bug 154018

Summary: AX: crash at WebCore::Range::selectNodeContents(WebCore::Node*, int&)
Product: WebKit Reporter: Nan Wang <n_wang>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, n_wang, ryanhaddad, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch
cfleizach: review+
patch
commit-queue: commit-queue-
patch none

Description Nan Wang 2016-02-08 16:30:15 PST 
5   com.apple.WebCore             	0x7fff9eb912f4 WebCore::Range::selectNodeContents(WebCore::Node*, int&) + 36 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7602.1.17/dom/Node.h:412)
6   com.apple.WebCore             	0x7fff9ecf58eb WebCore::AXObjectCache::rangeForNodeContents(WebCore::Node*) + 75 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7602.1.17/accessibility/AXObjectCache.cpp:1531)
7   com.apple.WebCore             	0x7fff9ecf5be8 WebCore::AXObjectCache::rangeForUnorderedCharacterOffsets(WebCore::CharacterOffset const&, WebCore::CharacterOffset const&) + 312 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7602.1.17/accessibility/AXObjectCache.cpp:1561)
8   com.apple.WebCore             	0x7fff9fa449b1 -[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:] + 9249 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebCore/WebCore-7602.1.17/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4032)

Seems selectNodeContents is accessing some garbage data.
Comment 1 Radar WebKit Bug Importer 2016-02-08 16:32:43 PST 
<rdar://problem/24559206>
Comment 2 Nan Wang 2016-02-08 16:52:43 PST 
Created attachment 270894 [details]
patch
Comment 3 chris fleizach 2016-02-08 16:55:59 PST 
Comment on attachment 270894 [details]
patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:1586
> +    if (nodeIsDerefed(characterOffset1.node) || nodeIsDerefed(characterOffset2.node))

can we use our nodeInUse cache to handle this case? seems like we should be doing that when creating the CharacterOffsets too
Comment 4 Nan Wang 2016-02-08 17:37:54 PST 
Comment on attachment 270894 [details]
patch

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

>> Source/WebCore/accessibility/AXObjectCache.cpp:1586
>> +    if (nodeIsDerefed(characterOffset1.node) || nodeIsDerefed(characterOffset2.node))
> 
> can we use our nodeInUse cache to handle this case? seems like we should be doing that when creating the CharacterOffsets too

Good point, will do.
Comment 5 Nan Wang 2016-02-08 17:38:58 PST 
Created attachment 270899 [details]
patch

review comments.
Comment 6 chris fleizach 2016-02-08 17:40:50 PST 
Comment on attachment 270899 [details]
patch

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

> Source/WebCore/ChangeLog:5
> +        <rdar://problem/24559206>

don't include rdar numbers (unless there is some new dictate to include them)

> LayoutTests/ChangeLog:5
> +        <rdar://problem/24559206>

don't include rdar numbers

> LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html:29
> +        textElement.innerHTML="";

textElement.innerHTML = "";
Comment 7 Nan Wang 2016-02-08 17:45:53 PST 
Created attachment 270901 [details]
patch

Addressed minor issues.
Comment 8 chris fleizach 2016-02-08 17:49:22 PST 
Comment on attachment 270901 [details]
patch

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

> LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html:29
> +        textElement.innerHTML= "";

still need another space before HTML=
Comment 9 WebKit Commit Bot 2016-02-08 17:56:25 PST 
Comment on attachment 270901 [details]
patch

Rejecting attachment 270901 [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', 'validate-changelog', '--check-oops', '--non-interactive', 270901, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/802367
Comment 10 Nan Wang 2016-02-08 17:58:28 PST 
Created attachment 270902 [details]
patch

This one should be good.
Comment 11 WebKit Commit Bot 2016-02-08 19:04:22 PST 
Comment on attachment 270902 [details]
patch

Clearing flags on attachment: 270902

Committed r196287: <http://trac.webkit.org/changeset/196287>
Comment 12 WebKit Commit Bot 2016-02-08 19:04:27 PST 
All reviewed patches have been landed.  Closing bug.