Changeset 220551 in webkit

Timestamp:

Aug 10, 2017 3:15:49 PM (7 years ago)

Author:

n_wang@apple.com

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/add-children-pseudo-element-expected.txt (added)
• LayoutTests/accessibility/add-children-pseudo-element.html (added)
• LayoutTests/accessibility/resources/svg-circle.svg (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityRenderObject.cpp (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityRenderObject.h (modified) (3 diffs)
• Source/WebCore/accessibility/AccessibilitySVGRoot.cpp (modified) (3 diffs)
• Source/WebCore/accessibility/AccessibilitySVGRoot.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-08-10  Nan Wang  <n_wang@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  <cdumez@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-08-10  Nan Wang  <n_wang@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  <cdumez@apple.com>
 

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -110 +110 @@
     : AccessibilityNodeObject(renderer->node())
     , m_renderer(renderer)
+    , m_weakPtrFactory(this)
 {
 #ifndef NDEBUG

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h

@@ -32 +32 @@
 #include "LayoutRect.h"
 #include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -199 +200 @@
 
     String passwordFieldValue() const override;
+    
+    WeakPtr<AccessibilityRenderObject> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
 
 protected:
@@ -218 +221 @@
 
 private:
+    WeakPtrFactory<AccessibilityRenderObject> m_weakPtrFactory;
     bool isAccessibilityRenderObject() const final { return true; }
     void ariaListboxSelectedChildren(AccessibilityChildrenVector&);

trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.cpp

@@ -36 +36 @@
 AccessibilitySVGRoot::AccessibilitySVGRoot(RenderObject* renderer)
     : AccessibilitySVGElement(renderer)
-    , m_parent(nullptr)
 {
 }
@@ -48 +47 @@
     return adoptRef(*new AccessibilitySVGRoot(renderer));
 }
+
+void AccessibilitySVGRoot::setParent(AccessibilityRenderObject *parent)
+{
+    if (parent)
+        m_parent = parent->createWeakPtr();
+    else
+        m_parent = nullptr;
+}
     
 AccessibilityObject* AccessibilitySVGRoot::parentObject() const
@@ -54 +61 @@
     // 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();

trunk/Source/WebCore/accessibility/AccessibilitySVGRoot.h

@@ -30 +30 @@
 
 #include "AccessibilitySVGElement.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -38 +39 @@
     virtual ~AccessibilitySVGRoot();
     
-    void setParent(AccessibilityObject* parent) { m_parent = parent; }
+    void setParent(AccessibilityRenderObject*);
 
 private:
@@ -46 +47 @@
     bool isAccessibilitySVGRoot() const override { return true; }
 
-    AccessibilityObject* m_parent;
+    WeakPtr<AccessibilityRenderObject> m_parent;
     AccessibilityRole roleValue() const override { return GroupRole; }
 };