Title: [87856] trunk/Source/WebCore
Revision
87856
Author
[email protected]
Date
2011-06-01 15:37:07 -0700 (Wed, 01 Jun 2011)

Log Message

Safari always crashes on http://bbc.co.uk when VoiceOver enabled
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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (87855 => 87856)


--- trunk/Source/WebCore/ChangeLog	2011-06-01 21:57:05 UTC (rev 87855)
+++ trunk/Source/WebCore/ChangeLog	2011-06-01 22:37:07 UTC (rev 87856)
@@ -1,3 +1,27 @@
+2011-06-01  Chris Fleizach  <[email protected]>
+
+        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  <[email protected]>
 
         Reviewed by Antti Koivisto.

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (87855 => 87856)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2011-06-01 21:57:05 UTC (rev 87855)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2011-06-01 22:37:07 UTC (rev 87856)
@@ -717,6 +717,13 @@
     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();

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (87855 => 87856)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.h	2011-06-01 21:57:05 UTC (rev 87855)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h	2011-06-01 22:37:07 UTC (rev 87856)
@@ -583,7 +583,7 @@
 
     // 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;

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (87855 => 87856)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2011-06-01 21:57:05 UTC (rev 87855)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2011-06-01 22:37:07 UTC (rev 87856)
@@ -3630,15 +3630,6 @@
         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())

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h (87855 => 87856)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2011-06-01 21:57:05 UTC (rev 87855)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2011-06-01 22:37:07 UTC (rev 87856)
@@ -245,8 +245,6 @@
     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;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to