Safari always crashes on http://bbc.co.uk when VoiceOver enabled
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jun 2011 22:37:07 +0000 (22:37 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jun 2011 22:37:07 +0000 (22:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=61886

Reviewed by Darin Adler.

This crash can happen on webpages that remove an element from the DOM when the element receives focus.
When AppKit goes to post a notification to inform VoiceOver the focus has changed, it asks for the AXFocusedUIElement.
However by posting that notification, a render tree update is performed. This causes the element to disappear, but
AppKit still has a handle to it and continues to try to reference it. When the autorelease pool pops, the reference goes bad.

To fix, the root element, the AccessibilityScrollView, needs to implement updateBackingStore(), otherwise this method
will not be called in time.

No test could be created because to cause it depends on an internal AppKit mechanism
that is only triggered remotely through the accessibility runtime.

* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::updateBackingStore):
* accessibility/AccessibilityObject.h:
* accessibility/AccessibilityRenderObject.cpp:
* accessibility/AccessibilityRenderObject.h:

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

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

index f67609c..ec51819 100644 (file)
@@ -1,3 +1,27 @@
+2011-06-01  Chris Fleizach  <cfleizach@apple.com>
+
+        Reviewed by Darin Adler.
+
+        Safari always crashes on http://bbc.co.uk when VoiceOver enabled
+        https://bugs.webkit.org/show_bug.cgi?id=61886
+
+        This crash can happen on webpages that remove an element from the DOM when the element receives focus.
+        When AppKit goes to post a notification to inform VoiceOver the focus has changed, it asks for the AXFocusedUIElement.
+        However by posting that notification, a render tree update is performed. This causes the element to disappear, but
+        AppKit still has a handle to it and continues to try to reference it. When the autorelease pool pops, the reference goes bad.
+
+        To fix, the root element, the AccessibilityScrollView, needs to implement updateBackingStore(), otherwise this method 
+        will not be called in time.
+
+        No test could be created because to cause it depends on an internal AppKit mechanism
+        that is only triggered remotely through the accessibility runtime.
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::updateBackingStore):
+        * accessibility/AccessibilityObject.h:
+        * accessibility/AccessibilityRenderObject.cpp:
+        * accessibility/AccessibilityRenderObject.h:
+
 2011-06-01  David Carson  <dacarson@apple.com>
 
         Reviewed by Antti Koivisto.
index f27140a..6b370bc 100644 (file)
@@ -717,6 +717,13 @@ unsigned AccessibilityObject::doAXLineForIndex(unsigned index)
     return lineForPosition(visiblePositionForIndex(index, false));
 }
     
+void AccessibilityObject::updateBackingStore()
+{
+    // Updating the layout may delete this object.
+    if (Document* document = this->document())
+        document->updateLayoutIgnorePendingStylesheets();
+}
+
 Document* AccessibilityObject::document() const
 {
     FrameView* frameView = documentFrameView();
index cacd19d..e003153 100644 (file)
@@ -583,7 +583,7 @@ public:
 
     // allows for an AccessibilityObject to update its render tree or perform
     // other operations update type operations
-    virtual void updateBackingStore() { }
+    void updateBackingStore();
     
 protected:
     AXID m_id;
index c12a3ea..b880e1d 100644 (file)
@@ -3630,15 +3630,6 @@ void AccessibilityRenderObject::setAccessibleName(String& name)
         static_cast<Element*>(domNode)->setAttribute(aria_labelAttr, name);
 }
     
-void AccessibilityRenderObject::updateBackingStore()
-{
-    if (!m_renderer)
-        return;
-
-    // Updating layout may delete m_renderer and this object.
-    m_renderer->document()->updateLayoutIgnorePendingStylesheets();
-}
-
 static bool isLinkable(const AccessibilityRenderObject& object)
 {
     if (!object.renderer())
index 53a354b..d136b8a 100644 (file)
@@ -245,8 +245,6 @@ public:
     virtual String doAXStringForRange(const PlainTextRange&) const;
     virtual IntRect doAXBoundsForRange(const PlainTextRange&) const;
     
-    virtual void updateBackingStore();
-
     virtual String stringValueForMSAA() const;
     virtual String stringRoleForMSAA() const;
     virtual String nameForMSAA() const;