Title: [137178] trunk/Source/WebCore
Revision
137178
Author
[email protected]
Date
2012-12-10 10:36:25 -0800 (Mon, 10 Dec 2012)

Log Message

AX: Crash in NSAccessibilityUnignoredDescendant
https://bugs.webkit.org/show_bug.cgi?id=104468

Reviewed by Anders Carlsson.

This crash occurred because AccessibilityScrollView stopped retaining it's ScrollView pointer.

Unfortunately, there was still a case that didn't correctly clear that variable.

That case was when a Frame had a new FrameView associated with it. The old FrameView didn't inform the
accessibility object cache that it should be removed. As a result, when the FrameView deallocated and
did try to remove, it used the wrong AXObjectCache because it was no longer correctly hooked into the
document tree.

No new tests. Existing tests will stop crashing.

* page/Frame.cpp:
(WebCore::Frame::setView): change the name of the method that tells the FrameView to detach
* page/FrameView.cpp:
(WebCore::FrameView::~FrameView):
(WebCore::FrameView::removeFromAXObjectCache): add a method that can be called in multiple places
(WebCore::FrameView::isFrameView):
(WebCore::FrameView::prepareForDetach): detaches scrollbars and accessibility
* page/FrameView.h:
(FrameView):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (137177 => 137178)


--- trunk/Source/WebCore/ChangeLog	2012-12-10 18:22:28 UTC (rev 137177)
+++ trunk/Source/WebCore/ChangeLog	2012-12-10 18:36:25 UTC (rev 137178)
@@ -1,3 +1,31 @@
+2012-12-10  Chris Fleizach  <[email protected]>
+
+        AX: Crash in NSAccessibilityUnignoredDescendant
+        https://bugs.webkit.org/show_bug.cgi?id=104468
+
+        Reviewed by Anders Carlsson.
+
+        This crash occurred because AccessibilityScrollView stopped retaining it's ScrollView pointer.
+
+        Unfortunately, there was still a case that didn't correctly clear that variable. 
+
+        That case was when a Frame had a new FrameView associated with it. The old FrameView didn't inform the 
+        accessibility object cache that it should be removed. As a result, when the FrameView deallocated and 
+        did try to remove, it used the wrong AXObjectCache because it was no longer correctly hooked into the 
+        document tree.
+
+        No new tests. Existing tests will stop crashing.
+ 
+        * page/Frame.cpp:
+        (WebCore::Frame::setView): change the name of the method that tells the FrameView to detach
+        * page/FrameView.cpp:
+        (WebCore::FrameView::~FrameView):
+        (WebCore::FrameView::removeFromAXObjectCache): add a method that can be called in multiple places
+        (WebCore::FrameView::isFrameView):
+        (WebCore::FrameView::prepareForDetach): detaches scrollbars and accessibility
+        * page/FrameView.h:
+        (FrameView):
+
 2012-12-10  Mark Pilgrim  <[email protected]>
 
         [Chromium] Move getPluginsList out of PlatformSupport

Modified: trunk/Source/WebCore/page/Frame.cpp (137177 => 137178)


--- trunk/Source/WebCore/page/Frame.cpp	2012-12-10 18:22:28 UTC (rev 137177)
+++ trunk/Source/WebCore/page/Frame.cpp	2012-12-10 18:36:25 UTC (rev 137178)
@@ -255,7 +255,7 @@
     // from messing with the view such that its scroll bars won't be torn down.
     // FIXME: We should revisit this.
     if (m_view)
-        m_view->detachCustomScrollbars();
+        m_view->prepareForDetach();
 
     // Prepare for destruction now, so any unload event handlers get run and the DOMWindow is
     // notified. If we wait until the view is destroyed, then things won't be hooked up enough for

Modified: trunk/Source/WebCore/page/FrameView.cpp (137177 => 137178)


--- trunk/Source/WebCore/page/FrameView.cpp	2012-12-10 18:22:28 UTC (rev 137177)
+++ trunk/Source/WebCore/page/FrameView.cpp	2012-12-10 18:36:25 UTC (rev 137178)
@@ -234,9 +234,7 @@
         m_actionScheduler->clear();
     }
     
-    if (AXObjectCache::accessibilityEnabled() && axObjectCache())
-        axObjectCache()->remove(this);
-    
+    removeFromAXObjectCache();
     resetScrollbars();
 
     // Custom scrollbars should already be destroyed at this point
@@ -299,7 +297,13 @@
     m_disableRepaints = 0;
 }
 
-bool FrameView::isFrameView() const 
+void FrameView::removeFromAXObjectCache()
+{
+    if (AXObjectCache::accessibilityEnabled() && axObjectCache())
+        axObjectCache()->remove(this);
+}
+
+bool FrameView::isFrameView() const
 { 
     return true; 
 }
@@ -351,6 +355,14 @@
             setMarginHeight(marginHeight);
     }
 }
+    
+void FrameView::prepareForDetach()
+{
+    detachCustomScrollbars();
+    // When the view is no longer associated with a frame, it needs to be removed from the ax object cache
+    // right now, otherwise it won't be able to reach the topDocument()'s axObject cache later.
+    removeFromAXObjectCache();
+}
 
 void FrameView::detachCustomScrollbars()
 {

Modified: trunk/Source/WebCore/page/FrameView.h (137177 => 137178)


--- trunk/Source/WebCore/page/FrameView.h	2012-12-10 18:22:28 UTC (rev 137177)
+++ trunk/Source/WebCore/page/FrameView.h	2012-12-10 18:36:25 UTC (rev 137178)
@@ -156,6 +156,7 @@
 
     void resetScrollbars();
     void resetScrollbarsAndClearContentsSize();
+    void prepareForDetach();
     void detachCustomScrollbars();
     virtual void recalculateScrollbarOverlayStyle();
 
@@ -470,6 +471,7 @@
 
     virtual AXObjectCache* axObjectCache() const;
     void notifyWidgetsInAllFrames(WidgetNotification);
+    void removeFromAXObjectCache();
     
     static double sCurrentPaintTimeStamp; // used for detecting decoded resource thrash in the cache
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to