AX: crash at AccessibilityRenderObject::remoteSVGRootElement const
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 07:54:41 +0000 (07:54 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 07:54:41 +0000 (07:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158098

Reviewed by Joanmarie Diggs.

What looks like happens here is that when a document is torn down and we try to detach, we end up creating an accessibility element during detachment phase.
So instead of just clearing the callback pointer on an existing AXObject, we make a new object and access properties of an object being deallocated.

I tried very hard to make a test but it looks like this can really only be triggered during document tear down which also tears down the AXObjectCache. I didn't
have luck reproducing because of that.

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::remoteSVGElementHitTest):
(WebCore::AccessibilityRenderObject::isSVGImage):
(WebCore::AccessibilityRenderObject::detachRemoteSVGRoot):
(WebCore::AccessibilityRenderObject::remoteSVGRootElement):
(WebCore::AccessibilityRenderObject::addRemoteSVGChildren):
* accessibility/AccessibilityRenderObject.h:

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

Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.h

index bf0ca07..9c92944 100644 (file)
@@ -1,3 +1,24 @@
+2016-05-26  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: crash at AccessibilityRenderObject::remoteSVGRootElement const
+        https://bugs.webkit.org/show_bug.cgi?id=158098
+
+        Reviewed by Joanmarie Diggs.
+
+        What looks like happens here is that when a document is torn down and we try to detach, we end up creating an accessibility element during detachment phase.
+        So instead of just clearing the callback pointer on an existing AXObject, we make a new object and access properties of an object being deallocated.
+
+        I tried very hard to make a test but it looks like this can really only be triggered during document tear down which also tears down the AXObjectCache. I didn't
+        have luck reproducing because of that.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::remoteSVGElementHitTest):
+        (WebCore::AccessibilityRenderObject::isSVGImage):
+        (WebCore::AccessibilityRenderObject::detachRemoteSVGRoot):
+        (WebCore::AccessibilityRenderObject::remoteSVGRootElement):
+        (WebCore::AccessibilityRenderObject::addRemoteSVGChildren):
+        * accessibility/AccessibilityRenderObject.h:
+
 2016-05-25  Antti Koivisto  <antti@apple.com>
 
         Invalidate style for newly added nodes in Node::insertedInto
index e9190dd..5199294 100644 (file)
@@ -2255,7 +2255,7 @@ AccessibilityObject* AccessibilityRenderObject::accessibilityImageMapHitTest(HTM
 
 AccessibilityObject* AccessibilityRenderObject::remoteSVGElementHitTest(const IntPoint& point) const
 {
-    AccessibilityObject* remote = remoteSVGRootElement();
+    AccessibilityObject* remote = remoteSVGRootElement(Create);
     if (!remote)
         return nullptr;
     
@@ -2968,16 +2968,16 @@ void AccessibilityRenderObject::addTextFieldChildren()
     
 bool AccessibilityRenderObject::isSVGImage() const
 {
-    return remoteSVGRootElement();
+    return remoteSVGRootElement(Create);
 }
     
 void AccessibilityRenderObject::detachRemoteSVGRoot()
 {
-    if (AccessibilitySVGRoot* root = remoteSVGRootElement())
+    if (AccessibilitySVGRoot* root = remoteSVGRootElement(Retrieve))
         root->setParent(nullptr);
 }
 
-AccessibilitySVGRoot* AccessibilityRenderObject::remoteSVGRootElement() const
+AccessibilitySVGRoot* AccessibilityRenderObject::remoteSVGRootElement(CreationChoice createIfNecessary) const
 {
     if (!is<RenderImage>(m_renderer))
         return nullptr;
@@ -3009,11 +3009,11 @@ AccessibilitySVGRoot* AccessibilityRenderObject::remoteSVGRootElement() const
     AXObjectCache* cache = frame.document()->axObjectCache();
     if (!cache)
         return nullptr;
-    AccessibilityObject* rootSVGObject = cache->getOrCreate(rendererRoot);
+    AccessibilityObject* rootSVGObject = createIfNecessary == Create ? cache->getOrCreate(rendererRoot) : cache->get(rendererRoot);
 
     // In order to connect the AX hierarchy from the SVG root element from the loaded resource
     // the parent must be set, because there's no other way to get back to who created the image.
-    ASSERT(rootSVGObject);
+    ASSERT(!createIfNecessary || rootSVGObject);
     if (!is<AccessibilitySVGRoot>(*rootSVGObject))
         return nullptr;
     
@@ -3022,7 +3022,7 @@ AccessibilitySVGRoot* AccessibilityRenderObject::remoteSVGRootElement() const
     
 void AccessibilityRenderObject::addRemoteSVGChildren()
 {
-    AccessibilitySVGRoot* root = remoteSVGRootElement();
+    AccessibilitySVGRoot* root = remoteSVGRootElement(Create);
     if (!root)
         return;
     
index 5e2f363..ab68ec9 100644 (file)
@@ -245,7 +245,8 @@ private:
     
     bool isSVGImage() const;
     void detachRemoteSVGRoot();
-    AccessibilitySVGRoot* remoteSVGRootElement() const;
+    enum CreationChoice { Create, Retrieve };
+    AccessibilitySVGRoot* remoteSVGRootElement(CreationChoice createIfNecessary) const;
     AccessibilityObject* remoteSVGElementHitTest(const IntPoint&) const;
     void offsetBoundingBoxForRemoteSVGElement(LayoutRect&) const;
     bool supportsPath() const override;