Title: [220551] trunk
Revision
220551
Author
n_w...@apple.com
Date
2017-08-10 15:15:49 -0700 (Thu, 10 Aug 2017)

Log Message

AX: crash at WebCore::AccessibilityObject::supportsARIALiveRegion() const + 24
https://bugs.webkit.org/show_bug.cgi?id=175340
<rdar://problem/33782159>

Reviewed by Chris Fleizach.

Source/WebCore:

The issue here is that we manualy set the parent object of the AccessibilitySVGRoot object
and there are chances that the parent doesn't detach it properly during the parent's destroying
process. Accessing the stale parent object will lead to a crash.
Fixed this by making the parent object a weak pointer so we don't access an invalid memory. 

Test: accessibility/add-children-pseudo-element.html

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::AccessibilityRenderObject):
* accessibility/AccessibilityRenderObject.h:
(WebCore::AccessibilityRenderObject::createWeakPtr):
* accessibility/AccessibilitySVGRoot.cpp:
(WebCore::AccessibilitySVGRoot::AccessibilitySVGRoot):
(WebCore::AccessibilitySVGRoot::setParent):
(WebCore::AccessibilitySVGRoot::parentObject const):
* accessibility/AccessibilitySVGRoot.h:

LayoutTests:

* accessibility/add-children-pseudo-element-expected.txt: Added.
* accessibility/add-children-pseudo-element.html: Added.
* accessibility/resources/svg-circle.svg: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (220550 => 220551)


--- trunk/LayoutTests/ChangeLog	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/LayoutTests/ChangeLog	2017-08-10 22:15:49 UTC (rev 220551)
@@ -1,3 +1,15 @@
+2017-08-10  Nan Wang  <n_w...@apple.com>
+
+        AX: crash at WebCore::AccessibilityObject::supportsARIALiveRegion() const + 24
+        https://bugs.webkit.org/show_bug.cgi?id=175340
+        <rdar://problem/33782159>
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/add-children-pseudo-element-expected.txt: Added.
+        * accessibility/add-children-pseudo-element.html: Added.
+        * accessibility/resources/svg-circle.svg: Added.
+
 2017-08-10  Chris Dumez  <cdu...@apple.com>
 
         [Beacon] Do connect-src CSP check on redirects as well

Added: trunk/LayoutTests/accessibility/add-children-pseudo-element-expected.txt (0 => 220551)


--- trunk/LayoutTests/accessibility/add-children-pseudo-element-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/add-children-pseudo-element-expected.txt	2017-08-10 22:15:49 UTC (rev 220551)
@@ -0,0 +1,12 @@
+Language Email 
+Make sure that we are updating the render block flow element's children correctly.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS element.childrenCount is 3
+PASS element.childrenCount is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/accessibility/add-children-pseudo-element.html (0 => 220551)


--- trunk/LayoutTests/accessibility/add-children-pseudo-element.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/add-children-pseudo-element.html	2017-08-10 22:15:49 UTC (rev 220551)
@@ -0,0 +1,70 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+
+<style>
+.pseudo::after {
+  content: url(resources/svg-circle.svg);
+  width: 18px;
+  height: 20px;
+  position: absolute;
+  margin-top: 6px;
+  right: 6px
+}
+
+.pseudo.hidden::after {
+    content: ' ';
+}
+</style>
+
+<body id="body">
+
+<div id="container">
+<div id="test" style="float : left;">
+<span>Language</span>
+<input id="test">
+</div>
+
+<div id="float" style="display: inline-block;" class="pseudo">
+<span required="" style="color: rgb(194, 0, 0);">Email</span>
+<input type="text" required="" aria-required="true" value="" _onkeyup_="hidePseudo();">
+</div>
+
+</div>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("Make sure that we are updating the render block flow element's children correctly.");
+
+    if (window.accessibilityController) {
+        var element = accessibilityController.accessibleElementById("float");
+        shouldBe("element.childrenCount", "3");
+        
+        eventSender.keyDown('\t');
+        eventSender.keyDown('\t');
+        
+        shouldBe("element.childrenCount", "2");
+        showPseudo();
+        
+        function hidePseudo() {
+            document.getElementById("float").className += "hidden"
+        }
+        
+        function showPseudo() {
+            document.getElementById("float").className = "pseudo";
+        }
+                
+        // Make sure getting the attributes of its children won't cause crash
+        element.attributesOfChildren();
+    }
+
+</script>
+
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/accessibility/resources/svg-circle.svg (0 => 220551)


--- trunk/LayoutTests/accessibility/resources/svg-circle.svg	                        (rev 0)
+++ trunk/LayoutTests/accessibility/resources/svg-circle.svg	2017-08-10 22:15:49 UTC (rev 220551)
@@ -0,0 +1,7 @@
+<svg viewBox="0 0 95 50"
+    xmlns="http://www.w3.org/2000/svg">
+    <g stroke="green" fill="white" stroke-width="5">
+        <circle cx="25" cy="25" r="15"/>
+        <text>circle</text>
+    </g>
+</svg>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (220550 => 220551)


--- trunk/Source/WebCore/ChangeLog	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/Source/WebCore/ChangeLog	2017-08-10 22:15:49 UTC (rev 220551)
@@ -1,3 +1,28 @@
+2017-08-10  Nan Wang  <n_w...@apple.com>
+
+        AX: crash at WebCore::AccessibilityObject::supportsARIALiveRegion() const + 24
+        https://bugs.webkit.org/show_bug.cgi?id=175340
+        <rdar://problem/33782159>
+
+        Reviewed by Chris Fleizach.
+
+        The issue here is that we manualy set the parent object of the AccessibilitySVGRoot object
+        and there are chances that the parent doesn't detach it properly during the parent's destroying
+        process. Accessing the stale parent object will lead to a crash.
+        Fixed this by making the parent object a weak pointer so we don't access an invalid memory. 
+
+        Test: accessibility/add-children-pseudo-element.html
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::AccessibilityRenderObject):
+        * accessibility/AccessibilityRenderObject.h:
+        (WebCore::AccessibilityRenderObject::createWeakPtr):
+        * accessibility/AccessibilitySVGRoot.cpp:
+        (WebCore::AccessibilitySVGRoot::AccessibilitySVGRoot):
+        (WebCore::AccessibilitySVGRoot::setParent):
+        (WebCore::AccessibilitySVGRoot::parentObject const):
+        * accessibility/AccessibilitySVGRoot.h:
+
 2017-08-10  Chris Dumez  <cdu...@apple.com>
 
         [Beacon] Do connect-src CSP check on redirects as well

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (220550 => 220551)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2017-08-10 22:15:49 UTC (rev 220551)
@@ -109,6 +109,7 @@
 AccessibilityRenderObject::AccessibilityRenderObject(RenderObject* renderer)
     : AccessibilityNodeObject(renderer->node())
     , m_renderer(renderer)
+    , m_weakPtrFactory(this)
 {
 #ifndef NDEBUG
     m_renderer->setHasAXObject(true);

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h (220550 => 220551)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2017-08-10 22:15:49 UTC (rev 220551)
@@ -31,6 +31,7 @@
 #include "AccessibilityNodeObject.h"
 #include "LayoutRect.h"
 #include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
     
@@ -198,6 +199,8 @@
     AccessibilityRole roleValueForMSAA() const override;
 
     String passwordFieldValue() const override;
+    
+    WeakPtr<AccessibilityRenderObject> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
 
 protected:
     explicit AccessibilityRenderObject(RenderObject*);
@@ -217,6 +220,7 @@
     RenderObject* m_renderer;
 
 private:
+    WeakPtrFactory<AccessibilityRenderObject> m_weakPtrFactory;
     bool isAccessibilityRenderObject() const final { return true; }
     void ariaListboxSelectedChildren(AccessibilityChildrenVector&);
     void ariaListboxVisibleChildren(AccessibilityChildrenVector&);

Modified: trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.cpp (220550 => 220551)


--- trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.cpp	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.cpp	2017-08-10 22:15:49 UTC (rev 220551)
@@ -35,7 +35,6 @@
 
 AccessibilitySVGRoot::AccessibilitySVGRoot(RenderObject* renderer)
     : AccessibilitySVGElement(renderer)
-    , m_parent(nullptr)
 {
 }
 
@@ -47,6 +46,14 @@
 {
     return adoptRef(*new AccessibilitySVGRoot(renderer));
 }
+
+void AccessibilitySVGRoot::setParent(AccessibilityRenderObject *parent)
+{
+    if (parent)
+        m_parent = parent->createWeakPtr();
+    else
+        m_parent = nullptr;
+}
     
 AccessibilityObject* AccessibilitySVGRoot::parentObject() const
 {
@@ -53,7 +60,7 @@
     // If a parent was set because this is a remote SVG resource, use that
     // but otherwise, we should rely on the standard render tree for the parent.
     if (m_parent)
-        return m_parent;
+        return m_parent.get();
     
     return AccessibilitySVGElement::parentObject();
 }

Modified: trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.h (220550 => 220551)


--- trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.h	2017-08-10 22:08:34 UTC (rev 220550)
+++ trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.h	2017-08-10 22:15:49 UTC (rev 220551)
@@ -29,6 +29,7 @@
 #pragma once
 
 #include "AccessibilitySVGElement.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -37,7 +38,7 @@
     static Ref<AccessibilitySVGRoot> create(RenderObject*);
     virtual ~AccessibilitySVGRoot();
     
-    void setParent(AccessibilityObject* parent) { m_parent = parent; }
+    void setParent(AccessibilityRenderObject*);
 
 private:
     explicit AccessibilitySVGRoot(RenderObject*);
@@ -45,7 +46,7 @@
     AccessibilityObject* parentObject() const override;
     bool isAccessibilitySVGRoot() const override { return true; }
 
-    AccessibilityObject* m_parent;
+    WeakPtr<AccessibilityRenderObject> m_parent;
     AccessibilityRole roleValue() const override { return GroupRole; }
 };
     
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to