Title: [195705] trunk/Source/WebCore
Revision
195705
Author
n_w...@apple.com
Date
2016-01-27 16:17:20 -0800 (Wed, 27 Jan 2016)

Log Message

AX: Crash in AccessibilityTableColumn::headerObject
https://bugs.webkit.org/show_bug.cgi?id=153553
<rdar://problem/23196278>

Reviewed by Chris Fleizach.

Webkit was crashing sometimes when we asked for column headers of a table.
The columns vector of the table was reset during the iteration when we
were asking for the headerObject of each column. The column's addChildren()
function calls elementRect() for each child cell and that sometimes causes
the parent table to reset its children.
Fixed it by caching the columns vector and moving out the elementRect() logic
from AccessibilityTalbeColumn::addChildren().

* accessibility/AccessibilityTable.cpp:
(WebCore::AccessibilityTable::columnHeaders):
(WebCore::AccessibilityTable::rowHeaders):
* accessibility/AccessibilityTableColumn.cpp:
(WebCore::AccessibilityTableColumn::elementRect):
(WebCore::AccessibilityTableColumn::headerObject):
(WebCore::AccessibilityTableColumn::addChildren):
* accessibility/AccessibilityTableColumn.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (195704 => 195705)


--- trunk/Source/WebCore/ChangeLog	2016-01-28 00:16:52 UTC (rev 195704)
+++ trunk/Source/WebCore/ChangeLog	2016-01-28 00:17:20 UTC (rev 195705)
@@ -1,3 +1,28 @@
+2016-01-27  Nan Wang  <n_w...@apple.com>
+
+        AX: Crash in AccessibilityTableColumn::headerObject
+        https://bugs.webkit.org/show_bug.cgi?id=153553
+        <rdar://problem/23196278>
+
+        Reviewed by Chris Fleizach.
+
+        Webkit was crashing sometimes when we asked for column headers of a table.
+        The columns vector of the table was reset during the iteration when we
+        were asking for the headerObject of each column. The column's addChildren()
+        function calls elementRect() for each child cell and that sometimes causes 
+        the parent table to reset its children.
+        Fixed it by caching the columns vector and moving out the elementRect() logic
+        from AccessibilityTalbeColumn::addChildren().  
+
+        * accessibility/AccessibilityTable.cpp:
+        (WebCore::AccessibilityTable::columnHeaders):
+        (WebCore::AccessibilityTable::rowHeaders):
+        * accessibility/AccessibilityTableColumn.cpp:
+        (WebCore::AccessibilityTableColumn::elementRect):
+        (WebCore::AccessibilityTableColumn::headerObject):
+        (WebCore::AccessibilityTableColumn::addChildren):
+        * accessibility/AccessibilityTableColumn.h:
+
 2016-01-27  Chris Dumez  <cdu...@apple.com>
 
         Settings a reflected DOMString attribute to null should set it to the "null" string rather than the empty string

Modified: trunk/Source/WebCore/accessibility/AccessibilityTable.cpp (195704 => 195705)


--- trunk/Source/WebCore/accessibility/AccessibilityTable.cpp	2016-01-28 00:16:52 UTC (rev 195704)
+++ trunk/Source/WebCore/accessibility/AccessibilityTable.cpp	2016-01-28 00:17:20 UTC (rev 195705)
@@ -502,7 +502,10 @@
     
     updateChildrenIfNecessary();
     
-    for (const auto& column : m_columns) {
+    // Sometimes m_columns can be reset during the iteration, we cache it here to be safe.
+    AccessibilityChildrenVector columnsCopy = m_columns;
+    
+    for (const auto& column : columnsCopy) {
         if (AccessibilityObject* header = downcast<AccessibilityTableColumn>(*column).headerObject())
             headers.append(header);
     }
@@ -515,7 +518,10 @@
     
     updateChildrenIfNecessary();
     
-    for (const auto& row : m_rows) {
+    // Sometimes m_rows can be reset during the iteration, we cache it here to be safe.
+    AccessibilityChildrenVector rowsCopy = m_rows;
+    
+    for (const auto& row : rowsCopy) {
         if (AccessibilityObject* header = downcast<AccessibilityTableRow>(*row).headerObject())
             headers.append(header);
     }

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp (195704 => 195705)


--- trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp	2016-01-28 00:16:52 UTC (rev 195704)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp	2016-01-28 00:17:20 UTC (rev 195705)
@@ -63,8 +63,14 @@
     
 LayoutRect AccessibilityTableColumn::elementRect() const
 {
-    // this will be filled in when addChildren is called
-    return m_columnRect;
+    // This used to be cached during the call to addChildren(), but calling elementRect()
+    // can invalidate elements, so its better to ask for this on demand.
+    LayoutRect columnRect;
+    AccessibilityChildrenVector childrenCopy = m_children;
+    for (const auto& cell : childrenCopy)
+        columnRect.unite(cell->elementRect());
+
+    return columnRect;
 }
 
 AccessibilityObject* AccessibilityTableColumn::headerObject()
@@ -191,7 +197,6 @@
             continue;
             
         m_children.append(cell);
-        m_columnRect.unite(cell->elementRect());
     }
 }
     

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableColumn.h (195704 => 195705)


--- trunk/Source/WebCore/accessibility/AccessibilityTableColumn.h	2016-01-28 00:16:52 UTC (rev 195704)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableColumn.h	2016-01-28 00:17:20 UTC (rev 195705)
@@ -62,7 +62,6 @@
     virtual bool isTableColumn() const override { return true; }
 
     unsigned m_columnIndex;
-    LayoutRect m_columnRect;
 };
 
 } // namespace WebCore 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to