Title: [196287] trunk
Revision
196287
Author
n_w...@apple.com
Date
2016-02-08 19:04:20 -0800 (Mon, 08 Feb 2016)

Log Message

AX: crash at WebCore::Range::selectNodeContents(WebCore::Node*, int&)
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (196286 => 196287)


--- trunk/LayoutTests/ChangeLog	2016-02-09 02:22:27 UTC (rev 196286)
+++ trunk/LayoutTests/ChangeLog	2016-02-09 03:04:20 UTC (rev 196287)
@@ -1,3 +1,13 @@
+2016-02-08  Nan Wang  <n_w...@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  <pecor...@apple.com>
 
         Web Inspector: Zooming in on the timeline graph does not increase its time resolution from minutes

Added: trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt (0 => 196287)


--- trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash-expected.txt	2016-02-09 03:04:20 UTC (rev 196287)
@@ -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
+

Added: trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html (0 => 196287)


--- trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/text-marker/text-marker-range-stale-node-crash.html	2016-02-09 03:04:20 UTC (rev 196287)
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</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=""
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (196286 => 196287)


--- trunk/Source/WebCore/ChangeLog	2016-02-09 02:22:27 UTC (rev 196286)
+++ trunk/Source/WebCore/ChangeLog	2016-02-09 03:04:20 UTC (rev 196287)
@@ -1,3 +1,27 @@
+2016-02-08  Nan Wang  <n_w...@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  <akl...@apple.com>
 
         [iOS] Throw away some unlinked code when navigating to a new page.

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (196286 => 196287)


--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2016-02-09 02:22:27 UTC (rev 196286)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp	2016-02-09 03:04:20 UTC (rev 196287)
@@ -1423,6 +1423,17 @@
     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)

Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (196286 => 196287)


--- trunk/Source/WebCore/accessibility/AXObjectCache.h	2016-02-09 02:22:27 UTC (rev 196286)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h	2016-02-09 03:04:20 UTC (rev 196287)
@@ -185,6 +185,7 @@
     // 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&);

Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (196286 => 196287)


--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2016-02-09 02:22:27 UTC (rev 196286)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2016-02-09 03:04:20 UTC (rev 196287)
@@ -900,18 +900,23 @@
     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)
 {
     ASSERT(cache);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to