Merge r222008 - Switch multicolumn's spanner map from raw over to wea…

…k pointers. https://bugs.webkit.org/show_bug.cgi?id=176367 <rdar://problem/34254896> Reviewed by Antti Koivisto. Source/WebCore: Test: fast/multicol/spanner-crash-when-adding-summary.html * rendering/RenderMultiColumnFlowThread.cpp: (WebCore::RenderMultiColumnFlowThread::evacuateAndDestroy): (WebCore::RenderMultiColumnFlowThread::flowThreadDescendantInserted): (WebCore::RenderMultiColumnFlowThread::handleSpannerRemoval): * rendering/RenderMultiColumnFlowThread.h: * rendering/RenderMultiColumnSet.cpp: (WebCore::RenderMultiColumnSet::firstRendererInFlowThread const): (WebCore::RenderMultiColumnSet::lastRendererInFlowThread const): * rendering/RenderMultiColumnSpannerPlaceholder.cpp: (WebCore::RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder): * rendering/RenderMultiColumnSpannerPlaceholder.h: LayoutTests: * fast/multicol/spanner-crash-when-adding-summary-expected.txt: Added. * fast/multicol/spanner-crash-when-adding-summary.html: Added.
WebKit · Nov 8, 2017 · fa51f6b · fa51f6b
1 parent 74b89ab
commit fa51f6b
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 12 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-09-13  Zalan Bujtas  <zalan@apple.com>
+
+        Switch multicolumn's spanner map from raw over to weak pointers.
+        https://bugs.webkit.org/show_bug.cgi?id=176367
+        <rdar://problem/34254896>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/multicol/spanner-crash-when-adding-summary-expected.txt: Added.
+        * fast/multicol/spanner-crash-when-adding-summary.html: Added.
+
 2017-09-13  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline http/tests/loading/state-object-security-exception.html after r221978.

diff --git a/LayoutTests/fast/multicol/spanner-crash-when-adding-summary-expected.txt b/LayoutTests/fast/multicol/spanner-crash-when-adding-summary-expected.txt
@@ -0,0 +1 @@
+PASS if no crash.
diff --git a/LayoutTests/fast/multicol/spanner-crash-when-adding-summary.html b/LayoutTests/fast/multicol/spanner-crash-when-adding-summary.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+div {
+    column-count: 2;
+}
+details {
+    border-image: url(#foo);
+}
+</style>
+</head>
+<body>
+<div><details><summary style="column-span: all"></summary></details></div>
+PASS if no crash.
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+r = document.caretRangeFromPoint(0, 0);
+r.insertNode(document.createElement("summary"));
+document.body.offsetHeight;
+document.getElementsByTagName("summary")[1].remove();
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-09-13  Zalan Bujtas  <zalan@apple.com>
+
+        Switch multicolumn's spanner map from raw over to weak pointers.
+        https://bugs.webkit.org/show_bug.cgi?id=176367
+        <rdar://problem/34254896>
+
+        Reviewed by Antti Koivisto.
+
+        Test: fast/multicol/spanner-crash-when-adding-summary.html
+
+        * rendering/RenderMultiColumnFlowThread.cpp:
+        (WebCore::RenderMultiColumnFlowThread::evacuateAndDestroy):
+        (WebCore::RenderMultiColumnFlowThread::flowThreadDescendantInserted):
+        (WebCore::RenderMultiColumnFlowThread::handleSpannerRemoval):
+        * rendering/RenderMultiColumnFlowThread.h:
+        * rendering/RenderMultiColumnSet.cpp:
+        (WebCore::RenderMultiColumnSet::firstRendererInFlowThread const):
+        (WebCore::RenderMultiColumnSet::lastRendererInFlowThread const):
+        * rendering/RenderMultiColumnSpannerPlaceholder.cpp:
+        (WebCore::RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder):
+        * rendering/RenderMultiColumnSpannerPlaceholder.h:
+
 2017-09-13  Daniel Bates  <dabates@apple.com>
 
         Make history.pushState()/replaceState() more closely aligned to the HTML standard

diff --git a/Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp b/Source/WebCore/rendering/RenderMultiColumnFlowThread.cpp
@@ -184,11 +184,13 @@ void RenderMultiColumnFlowThread::evacuateAndDestroy()
     SpannerMap::iterator it;
     while ((it = m_spannerMap.begin()) != m_spannerMap.end()) {
         RenderBox* spanner = it->key;
-        RenderMultiColumnSpannerPlaceholder* placeholder = it->value;
-        RenderBlockFlow& originalContainer = downcast<RenderBlockFlow>(*placeholder->parent());
         multicolContainer->removeChild(*spanner);
-        originalContainer.addChild(spanner, placeholder);
-        placeholder->destroy();
+        ASSERT(it->value.get());
+        if (RenderMultiColumnSpannerPlaceholder* placeholder = it->value.get()) {
+            RenderBlockFlow& originalContainer = downcast<RenderBlockFlow>(*placeholder->parent());
+            originalContainer.addChild(spanner, placeholder);
+            placeholder->destroy();
+        }
         m_spannerMap.remove(it);
     }
 
@@ -436,7 +438,7 @@ void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject& new
             }
 
             ASSERT(!m_spannerMap.get(placeholder.spanner()));
-            m_spannerMap.add(placeholder.spanner(), &placeholder);
+            m_spannerMap.add(placeholder.spanner(), placeholder.createWeakPtr());
             ASSERT(!placeholder.firstChild()); // There should be no children here, but if there are, we ought to skip them.
             continue;
         }
@@ -447,7 +449,7 @@ void RenderMultiColumnFlowThread::flowThreadDescendantInserted(RenderObject& new
 void RenderMultiColumnFlowThread::handleSpannerRemoval(RenderObject& spanner)
 {
      // The placeholder may already have been removed, but if it hasn't, do so now.
-    if (RenderMultiColumnSpannerPlaceholder* placeholder = m_spannerMap.get(&downcast<RenderBox>(spanner))) {
+    if (RenderMultiColumnSpannerPlaceholder* placeholder = m_spannerMap.get(&downcast<RenderBox>(spanner)).get()) {
         placeholder->parent()->removeChild(*placeholder);
         m_spannerMap.remove(&downcast<RenderBox>(spanner));
     }

diff --git a/Source/WebCore/rendering/RenderMultiColumnFlowThread.h b/Source/WebCore/rendering/RenderMultiColumnFlowThread.h
@@ -47,7 +47,7 @@ class RenderMultiColumnFlowThread final : public RenderFlowThread {
     static RenderBox* nextColumnSetOrSpannerSiblingOf(const RenderBox*);
     static RenderBox* previousColumnSetOrSpannerSiblingOf(const RenderBox*);
 
-    RenderMultiColumnSpannerPlaceholder* findColumnSpannerPlaceholder(RenderBox* spanner) const { return m_spannerMap.get(spanner); }
+    RenderMultiColumnSpannerPlaceholder* findColumnSpannerPlaceholder(RenderBox* spanner) const { return m_spannerMap.get(spanner).get(); }
 
     void layout() override;
 
@@ -127,7 +127,7 @@ class RenderMultiColumnFlowThread final : public RenderFlowThread {
     RenderObject* processPossibleSpannerDescendant(RenderObject*& subtreeRoot, RenderObject& descendant);
 
 private:
-    typedef HashMap<RenderBox*, RenderMultiColumnSpannerPlaceholder*> SpannerMap;
+    typedef HashMap<RenderBox*, WeakPtr<RenderMultiColumnSpannerPlaceholder>> SpannerMap;
     SpannerMap m_spannerMap;
 
     // The last set we worked on. It's not to be used as the "current set". The concept of a

diff --git a/Source/WebCore/rendering/RenderMultiColumnSet.cpp b/Source/WebCore/rendering/RenderMultiColumnSet.cpp
@@ -74,8 +74,9 @@ RenderObject* RenderMultiColumnSet::firstRendererInFlowThread() const
         // Adjacent sets should not occur. Currently we would have no way of figuring out what each
         // of them contains then.
         ASSERT(!sibling->isRenderMultiColumnSet());
-        RenderMultiColumnSpannerPlaceholder* placeholder = multiColumnFlowThread()->findColumnSpannerPlaceholder(sibling);
-        return placeholder->nextInPreOrderAfterChildren();
+        if (RenderMultiColumnSpannerPlaceholder* placeholder = multiColumnFlowThread()->findColumnSpannerPlaceholder(sibling))
+            return placeholder->nextInPreOrderAfterChildren();
+        ASSERT_NOT_REACHED();
     }
     return flowThread()->firstChild();
 }
@@ -86,8 +87,9 @@ RenderObject* RenderMultiColumnSet::lastRendererInFlowThread() const
         // Adjacent sets should not occur. Currently we would have no way of figuring out what each
         // of them contains then.
         ASSERT(!sibling->isRenderMultiColumnSet());
-        RenderMultiColumnSpannerPlaceholder* placeholder = multiColumnFlowThread()->findColumnSpannerPlaceholder(sibling);
-        return placeholder->previousInPreOrder();
+        if (RenderMultiColumnSpannerPlaceholder* placeholder = multiColumnFlowThread()->findColumnSpannerPlaceholder(sibling))
+            return placeholder->previousInPreOrder();
+        ASSERT_NOT_REACHED();
     }
     return flowThread()->lastLeafChild();
 }

diff --git a/Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.cpp b/Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.cpp
@@ -47,6 +47,7 @@ RenderMultiColumnSpannerPlaceholder::RenderMultiColumnSpannerPlaceholder(RenderM
     : RenderBox(flowThread->document(), WTFMove(style), RenderBoxModelObjectFlag)
     , m_spanner(&spanner)
     , m_flowThread(flowThread)
+    , m_weakPtrFactory(this)
 {
 }
 

diff --git a/Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.h b/Source/WebCore/rendering/RenderMultiColumnSpannerPlaceholder.h
@@ -31,6 +31,8 @@
 
 #include "RenderBox.h"
 
+#include <wtf/WeakPtr.h>
+
 namespace WebCore {
 
 class RenderMultiColumnFlowThread;
@@ -41,6 +43,7 @@ class RenderMultiColumnSpannerPlaceholder final : public RenderBox {
 
     RenderBox* spanner() const { return m_spanner; }
     RenderMultiColumnFlowThread* flowThread() const { return m_flowThread; }
+    WeakPtr<RenderMultiColumnSpannerPlaceholder> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
 
 private:
     RenderMultiColumnSpannerPlaceholder(RenderMultiColumnFlowThread*, RenderBox& spanner, RenderStyle&&);
@@ -52,6 +55,7 @@ class RenderMultiColumnSpannerPlaceholder final : public RenderBox {
 
     RenderBox* m_spanner;
     RenderMultiColumnFlowThread* m_flowThread;
+    WeakPtrFactory<RenderMultiColumnSpannerPlaceholder> m_weakPtrFactory;
 };
 
 } // namespace WebCore