RenderTable should not hold a collection of raw pointers to RenderTableCol
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Oct 2017 23:15:31 +0000 (23:15 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Oct 2017 23:15:31 +0000 (23:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178030
<rdar://problem/34865236>

Reviewed by Simon Fraser.

In addition to the m_columnRenderersValid flag, this patch ensures that
we don't dereference stale column renderers even when the flag is out of sync.

Covered by existing tests.

* rendering/RenderTable.cpp:
(WebCore::RenderTable::updateColumnCache const):
(WebCore::RenderTable::slowColElement const):
* rendering/RenderTable.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTable.h

index 552f6bf..56bb093 100644 (file)
@@ -1,5 +1,23 @@
 2017-10-06  Zalan Bujtas  <zalan@apple.com>
 
+        RenderTable should not hold a collection of raw pointers to RenderTableCol
+        https://bugs.webkit.org/show_bug.cgi?id=178030
+        <rdar://problem/34865236>
+
+        Reviewed by Simon Fraser.
+
+        In addition to the m_columnRenderersValid flag, this patch ensures that
+        we don't dereference stale column renderers even when the flag is out of sync.
+
+        Covered by existing tests.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::updateColumnCache const):
+        (WebCore::RenderTable::slowColElement const):
+        * rendering/RenderTable.h:
+
+2017-10-06  Zalan Bujtas  <zalan@apple.com>
+
         RootInlineBox should not hold a collection of raw pointers to RenderBox
         https://bugs.webkit.org/show_bug.cgi?id=178025
         <rdar://problem/34862488>
index 7190def..6b63c4b 100644 (file)
@@ -931,7 +931,7 @@ void RenderTable::updateColumnCache() const
     for (RenderTableCol* columnRenderer = firstColumn(); columnRenderer; columnRenderer = columnRenderer->nextColumn()) {
         if (columnRenderer->isTableColumnGroupWithColumnChildren())
             continue;
-        m_columnRenderers.append(columnRenderer);
+        m_columnRenderers.append(makeWeakPtr(columnRenderer));
         // FIXME: We should look to compute the effective column index successively from previous values instead of
         // calling colToEffCol(), which is in O(numEffCols()). Although it's unlikely that this is a hot function.
         m_effectiveColumnIndexMap.add(columnRenderer, colToEffCol(columnIndex));
@@ -1027,8 +1027,9 @@ RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge, bool*
         updateColumnCache();
 
     unsigned columnCount = 0;
-    for (unsigned i = 0; i < m_columnRenderers.size(); i++) {
-        RenderTableCol* columnRenderer = m_columnRenderers[i];
+    for (auto& columnRenderer : m_columnRenderers) {
+        if (!columnRenderer)
+            continue;
         unsigned span = columnRenderer->span();
         unsigned startCol = columnCount;
         ASSERT(span >= 1);
@@ -1039,10 +1040,10 @@ RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge, bool*
                 *startEdge = startCol == col;
             if (endEdge)
                 *endEdge = endCol == col;
-            return columnRenderer;
+            return columnRenderer.get();
         }
     }
-    return 0;
+    return nullptr;
 }
 
 void RenderTable::recalcSections() const
index 603b81e..48c982d 100644 (file)
@@ -326,7 +326,7 @@ private:
     mutable Vector<LayoutUnit> m_columnPos;
     mutable Vector<ColumnStruct> m_columns;
     mutable Vector<RenderTableCaption*> m_captions;
-    mutable Vector<RenderTableCol*> m_columnRenderers;
+    mutable Vector<WeakPtr<RenderTableCol>> m_columnRenderers;
 
     unsigned effectiveIndexOfColumn(const RenderTableCol&) const;
     typedef HashMap<const RenderTableCol*, unsigned> EffectiveColumnIndexMap;