AX: crash at WebCore::Range::selectNodeContents(WebCore::Node*, int&)
authorn_wang@apple.com <n_wang@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Feb 2016 03:04:20 +0000 (03:04 +0000)
committern_wang@apple.com <n_wang@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Feb 2016 03:04:20 +0000 (03:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154018

Reviewed by Chris Fleizach.

Source/WebCore:

Sometimes rangeForUnorderedCharacterOffsets call is accessing derefed node objects
and leading to a crash. Fixed it by checking isNodeInUse before creating the CharacterOffset
object.

Test: accessibility/text-marker/text-marker-range-stale-node-crash.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::visiblePositionForTextMarkerData):
(WebCore::AXObjectCache::characterOffsetForTextMarkerData):
(WebCore::AXObjectCache::traverseToOffsetInRange):
* accessibility/AXObjectCache.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper rangeForTextMarkerRange:]):
(characterOffsetForTextMarker):
(-[WebAccessibilityObjectWrapper characterOffsetForTextMarker:]):
(textMarkerForVisiblePosition):

LayoutTests:

* accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt: Added.
* accessibility/text-marker/text-marker-range-stale-node-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196287 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

index af43c2f94211eaf95ac4e743e4aa0cbd06247e51..11d8d242c445ef4bf7303a9b02896590b81c9a1f 100644 (file)
@@ -1,3 +1,13 @@
+2016-02-08  Nan Wang  <n_wang@apple.com>
+
+        AX: crash at WebCore::Range::selectNodeContents(WebCore::Node*, int&)
+        https://bugs.webkit.org/show_bug.cgi?id=154018
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt: Added.
+        * accessibility/text-marker/text-marker-range-stale-node-crash.html: Added.
+
 2016-02-08  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Zooming in on the timeline graph does not increase its time resolution from minutes
diff --git a/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt b/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt
new file mode 100644 (file)
index 0000000..cb623ad
--- /dev/null
@@ -0,0 +1,11 @@
+someContent
+This tests that we create a text marker range from a stale node won't lead to crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS text.textMarkerRangeLength(markerRange) is 4
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html b/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html
new file mode 100644 (file)
index 0000000..9c8c4d0
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+
+<div id="text" contenteditable="true">text</div>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("This tests that we create a text marker range from a stale node won't lead to crash.");
+
+    if (window.accessibilityController) {
+        
+        var text = accessibilityController.accessibleElementById("text");
+        var textElement = document.getElementById("text");
+        var markerRange = text.textMarkerRangeForElement(text);
+        var startMarker = text.startTextMarkerForTextMarkerRange(markerRange);
+        var endMarker = text.endTextMarkerForTextMarkerRange(markerRange);
+        
+        markerRange = text.textMarkerRangeForMarkers(startMarker, endMarker);
+        shouldBe("text.textMarkerRangeLength(markerRange)", "4");
+        
+        // Change the node hierarchy.
+        textElement.innerHTML = "";
+        var textnode = document.createTextNode("someContent");
+        textElement.appendChild(textnode);
+        
+        // Making a range from the old markers won't crash.
+        markerRange = text.textMarkerRangeForMarkers(startMarker, endMarker);
+    }
+
+</script>
+
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
\ No newline at end of file
index 0d95d6bc5d6e43f156673fb35c1e6d53aed94fc9..8ba28bba1571bd05cc3e95b6116b93c6fe3d3619 100644 (file)
@@ -1,3 +1,27 @@
+2016-02-08  Nan Wang  <n_wang@apple.com>
+
+        AX: crash at WebCore::Range::selectNodeContents(WebCore::Node*, int&)
+        https://bugs.webkit.org/show_bug.cgi?id=154018
+
+        Reviewed by Chris Fleizach.
+
+        Sometimes rangeForUnorderedCharacterOffsets call is accessing derefed node objects
+        and leading to a crash. Fixed it by checking isNodeInUse before creating the CharacterOffset
+        object.
+
+        Test: accessibility/text-marker/text-marker-range-stale-node-crash.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::visiblePositionForTextMarkerData):
+        (WebCore::AXObjectCache::characterOffsetForTextMarkerData):
+        (WebCore::AXObjectCache::traverseToOffsetInRange):
+        * accessibility/AXObjectCache.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper rangeForTextMarkerRange:]):
+        (characterOffsetForTextMarker):
+        (-[WebAccessibilityObjectWrapper characterOffsetForTextMarker:]):
+        (textMarkerForVisiblePosition):
+
 2016-02-08  Andreas Kling  <akling@apple.com>
 
         [iOS] Throw away some unlinked code when navigating to a new page.
index 5a516ebd2b1ef67eb9ca8e85b265ba3cfc3a6431..a71482d8705e9a421f140704db64e01f1871b3aa 100644 (file)
@@ -1423,6 +1423,17 @@ VisiblePosition AXObjectCache::visiblePositionForTextMarkerData(TextMarkerData&
     return visiblePos;
 }
 
+CharacterOffset AXObjectCache::characterOffsetForTextMarkerData(TextMarkerData& textMarkerData)
+{
+    if (!isNodeInUse(textMarkerData.node))
+        return CharacterOffset();
+    
+    if (textMarkerData.ignored)
+        return CharacterOffset();
+    
+    return CharacterOffset(textMarkerData.node, textMarkerData.characterStartIndex, textMarkerData.characterOffset);
+}
+
 CharacterOffset AXObjectCache::traverseToOffsetInRange(RefPtr<Range>range, int offset, bool toNodeEnd, bool stayWithinRange)
 {
     if (!range)
index bc044c5a6bd7a9cea2ac3a51976bcdf750719319..90bc7db8aff8bcf3519ce41cddadc849660504ce 100644 (file)
@@ -185,6 +185,7 @@ public:
     // Text marker utilities.
     void textMarkerDataForVisiblePosition(TextMarkerData&, const VisiblePosition&);
     VisiblePosition visiblePositionForTextMarkerData(TextMarkerData&);
+    CharacterOffset characterOffsetForTextMarkerData(TextMarkerData&);
     void textMarkerDataForCharacterOffset(TextMarkerData&, Node&, int, bool toNodeEnd = false);
     void startOrEndTextMarkerDataForRange(TextMarkerData&, RefPtr<Range>, bool);
     AccessibilityObject* accessibilityObjectForTextMarkerData(TextMarkerData&);
index 985e756aea0c4f35c6de51a2e91fdfa255676424..ecc0bd4092b16b918fbb5b4d1344c27b4c72e963 100644 (file)
@@ -900,16 +900,21 @@ static id textMarkerForCharacterOffset(AXObjectCache* cache, Node& node, int off
     return cache->rangeForUnorderedCharacterOffsets(startCharacterOffset, endCharacterOffset);
 }
 
-- (CharacterOffset)characterOffsetForTextMarker:(id)textMarker
+static CharacterOffset characterOffsetForTextMarker(AXObjectCache* cache, CFTypeRef textMarker)
 {
-    if (!textMarker || isTextMarkerIgnored(textMarker))
+    if (!cache || !textMarker)
         return CharacterOffset();
     
     TextMarkerData textMarkerData;
     if (!wkGetBytesFromAXTextMarker(textMarker, &textMarkerData, sizeof(textMarkerData)))
         return CharacterOffset();
     
-    return CharacterOffset(textMarkerData.node, textMarkerData.characterStartIndex, textMarkerData.characterOffset);
+    return cache->characterOffsetForTextMarkerData(textMarkerData);
+}
+
+- (CharacterOffset)characterOffsetForTextMarker:(id)textMarker
+{
+    return characterOffsetForTextMarker(m_object->axObjectCache(), textMarker);
 }
 
 static id textMarkerForVisiblePosition(AXObjectCache* cache, const VisiblePosition& visiblePos)