FloatingObject should not hold a raw pointer to RootInlineBox.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2017 22:03:06 +0000 (22:03 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Sep 2017 22:03:06 +0000 (22:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177266

Reviewed by Simon Fraser.

FloatingObject and RootInlineBox objects' lifetimes are very much independent from each other.

Not testable.

* rendering/FloatingObjects.cpp:
(WebCore::FloatingObjects::clearLineBoxTreePointers):
* rendering/FloatingObjects.h:
(WebCore::FloatingObject::originatingLine const):
(WebCore::FloatingObject::clearOriginatingLine):
(WebCore::FloatingObject::setOriginatingLine):
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::removeFloatingObject):
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlockFlow::appendFloatingObjectToLastLine):
(WebCore::RenderBlockFlow::reattachCleanLineFloats):
(WebCore::RenderBlockFlow::determineStartPosition):
* rendering/RootInlineBox.cpp:
(WebCore::RootInlineBox::RootInlineBox):
* rendering/RootInlineBox.h:
(WebCore::RootInlineBox::createWeakPtr):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/FloatingObjects.cpp
Source/WebCore/rendering/FloatingObjects.h
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderBlockLineLayout.cpp
Source/WebCore/rendering/RootInlineBox.cpp
Source/WebCore/rendering/RootInlineBox.h

index 7b4d878..8eeada1 100644 (file)
@@ -1,3 +1,31 @@
+2017-09-20  Zalan Bujtas  <zalan@apple.com>
+
+        FloatingObject should not hold a raw pointer to RootInlineBox.
+        https://bugs.webkit.org/show_bug.cgi?id=177266
+
+        Reviewed by Simon Fraser.
+
+        FloatingObject and RootInlineBox objects' lifetimes are very much independent from each other.
+
+        Not testable.
+
+        * rendering/FloatingObjects.cpp:
+        (WebCore::FloatingObjects::clearLineBoxTreePointers):
+        * rendering/FloatingObjects.h:
+        (WebCore::FloatingObject::originatingLine const):
+        (WebCore::FloatingObject::clearOriginatingLine):
+        (WebCore::FloatingObject::setOriginatingLine):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::removeFloatingObject):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlockFlow::appendFloatingObjectToLastLine):
+        (WebCore::RenderBlockFlow::reattachCleanLineFloats):
+        (WebCore::RenderBlockFlow::determineStartPosition):
+        * rendering/RootInlineBox.cpp:
+        (WebCore::RootInlineBox::RootInlineBox):
+        * rendering/RootInlineBox.h:
+        (WebCore::RootInlineBox::createWeakPtr):
+
 2017-09-20  Chris Dumez  <cdumez@apple.com>
 
         Drop legacy DOMError type
index e4a9277..0b7469b 100644 (file)
@@ -263,7 +263,7 @@ void FloatingObjects::clearLineBoxTreePointers()
     // Clear references to originating lines, since the lines are being deleted
     for (auto it = m_set.begin(), end = m_set.end(); it != end; ++it) {
         ASSERT(!((*it)->originatingLine()) || &((*it)->originatingLine()->renderer()) == &m_renderer);
-        (*it)->setOriginatingLine(0);
+        (*it)->clearOriginatingLine();
     }
 }
 
index 62fc873..66a6fe1 100644 (file)
@@ -26,6 +26,7 @@
 #include "PODIntervalTree.h"
 #include "RootInlineBox.h"
 #include <wtf/ListHashSet.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -82,8 +83,9 @@ public:
     void setIsDescendant(bool isDescendant) { m_isDescendant = isDescendant; }
 
     // FIXME: Callers of these methods are dangerous and should be whitelisted explicitly or removed.
-    RootInlineBox* originatingLine() const { return m_originatingLine; }
-    void setOriginatingLine(RootInlineBox* line) { m_originatingLine = line; }
+    RootInlineBox* originatingLine() const { return m_originatingLine.get(); }
+    void clearOriginatingLine() { m_originatingLine = nullptr; }
+    void setOriginatingLine(RootInlineBox& line) { m_originatingLine = line.createWeakPtr(); }
 
     LayoutSize locationOffsetOfBorderBox() const
     {
@@ -95,7 +97,7 @@ public:
 
 private:
     RenderBox& m_renderer;
-    RootInlineBox* m_originatingLine { nullptr };
+    WeakPtr<RootInlineBox> m_originatingLine;
     LayoutRect m_frameRect;
     LayoutUnit m_paginationStrut;
     LayoutSize m_marginOffset;
index 054c77f..18eee3b 100644 (file)
@@ -2326,7 +2326,7 @@ void RenderBlockFlow::removeFloatingObject(RenderBox& floatBox)
                         floatingObject.originatingLine()->markDirty();
                     }
 #if !ASSERT_DISABLED
-                    floatingObject.setOriginatingLine(0);
+                    floatingObject.clearOriginatingLine();
 #endif
                 }
                 markLinesDirtyInBlockRange(0, logicalBottom);
index e61dba2..0ce5eca 100644 (file)
@@ -1096,7 +1096,8 @@ inline BidiRun* RenderBlockFlow::handleTrailingSpaces(BidiRunList<BidiRun>& bidi
 void RenderBlockFlow::appendFloatingObjectToLastLine(FloatingObject& floatingObject)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject.originatingLine());
-    floatingObject.setOriginatingLine(lastRootBox());
+    ASSERT(lastRootBox());
+    floatingObject.setOriginatingLine(*lastRootBox());
     lastRootBox()->appendFloat(floatingObject.renderer());
 }
 
@@ -1557,7 +1558,7 @@ void RenderBlockFlow::reattachCleanLineFloats(RootInlineBox& cleanLine, LayoutUn
             continue;
         }
         ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject->originatingLine());
-        floatingObject->setOriginatingLine(&cleanLine);
+        floatingObject->setOriginatingLine(cleanLine);
         setLogicalHeight(logicalTopForChild(*floatingBox) - marginBeforeForChild(*floatingBox) + delta);
         positionNewFloats();
     }
@@ -1870,7 +1871,7 @@ RootInlineBox* RenderBlockFlow::determineStartPosition(LineLayoutState& layoutSt
                     auto* floatingBox = *it;
                     auto* floatingObject = insertFloatingObject(*floatingBox);
                     ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject->originatingLine());
-                    floatingObject->setOriginatingLine(line);
+                    floatingObject->setOriginatingLine(*line);
                     setLogicalHeight(logicalTopForChild(*floatingBox) - marginBeforeForChild(*floatingBox));
                     positionNewFloats();
                     floats.setLastCleanFloat(*floatingBox);
index 1f14928..14f331f 100644 (file)
@@ -43,7 +43,7 @@ namespace WebCore {
 
 struct SameSizeAsRootInlineBox : public InlineFlowBox {
     unsigned variables[7];
-    void* pointers[3];
+    void* pointers[4];
 };
 
 COMPILE_ASSERT(sizeof(RootInlineBox) == sizeof(SameSizeAsRootInlineBox), RootInlineBox_should_stay_small);
@@ -61,6 +61,8 @@ RootInlineBox::RootInlineBox(RenderBlockFlow& block)
     : InlineFlowBox(block)
     , m_lineBreakPos(0)
     , m_lineBreakObj(nullptr)
+    , m_weakPtrFactory(this)
+
 {
     setIsHorizontal(block.isHorizontalWritingMode());
 }
index ce4cd47..c1958f3 100644 (file)
@@ -22,6 +22,7 @@
 
 #include "BidiContext.h"
 #include "InlineFlowBox.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -38,6 +39,7 @@ class RootInlineBox : public InlineFlowBox {
 public:
     explicit RootInlineBox(RenderBlockFlow&);
     virtual ~RootInlineBox();
+    WeakPtr<RootInlineBox> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
 
     RenderBlockFlow& blockFlow() const;
 
@@ -227,6 +229,7 @@ private:
     // Floats hanging off the line are pushed into this vector during layout. It is only
     // good for as long as the line has not been marked dirty.
     std::unique_ptr<Vector<RenderBox*>> m_floats;
+    WeakPtrFactory<RootInlineBox> m_weakPtrFactory;
 };
 
 inline RootInlineBox* RootInlineBox::nextRootBox() const