FloatingObjects should manage cleaning it's line box tree pointers itself
authorbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 21:35:56 +0000 (21:35 +0000)
committerbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 21:35:56 +0000 (21:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=120692

Reviewed by David Hyatt.

This is another step in properly encapsulating FloatingObjects.
Instead of having RenderBlock walk and clear the line box tree
pointers, create a method for the behavior, and have RenderBlock call
that.

In addtion, add a proper destructor to FloatingObjects, so that
RenderBlock does not have to explicitly delete the set in
FloatingObjects.

And as a bonus, fix the ordering of an if to avoid the expensive
descendantChild check.

This is a port of a Blink patch by Eric Seidel.

No new tests, no behavior change.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::~RenderBlock):
(WebCore::RenderBlock::deleteLineBoxTree):
(WebCore::RenderBlock::repaintOverhangingFloats):
(WebCore::RenderBlock::FloatingObjects::~FloatingObjects):
(WebCore::RenderBlock::FloatingObjects::clearLineBoxTreePointers):
(WebCore::RenderBlock::FloatingObjects::clear):
* rendering/RenderBlock.h:

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

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

index 27e13d5..962ca48 100644 (file)
@@ -1,3 +1,35 @@
+2013-09-04  Bem Jones-Bey  <bjonesbe@adobe.com>
+
+        FloatingObjects should manage cleaning it's line box tree pointers itself
+        https://bugs.webkit.org/show_bug.cgi?id=120692
+
+        Reviewed by David Hyatt.
+
+        This is another step in properly encapsulating FloatingObjects.
+        Instead of having RenderBlock walk and clear the line box tree
+        pointers, create a method for the behavior, and have RenderBlock call
+        that.
+
+        In addtion, add a proper destructor to FloatingObjects, so that
+        RenderBlock does not have to explicitly delete the set in
+        FloatingObjects.
+
+        And as a bonus, fix the ordering of an if to avoid the expensive
+        descendantChild check.
+
+        This is a port of a Blink patch by Eric Seidel.
+
+        No new tests, no behavior change.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::~RenderBlock):
+        (WebCore::RenderBlock::deleteLineBoxTree):
+        (WebCore::RenderBlock::repaintOverhangingFloats):
+        (WebCore::RenderBlock::FloatingObjects::~FloatingObjects):
+        (WebCore::RenderBlock::FloatingObjects::clearLineBoxTreePointers):
+        (WebCore::RenderBlock::FloatingObjects::clear):
+        * rendering/RenderBlock.h:
+
 2013-09-04  Tim Horton  <timothy_horton@apple.com>
 
         Rename customCssText -> customCSSText to match WebKit style
index 4da68af..389b29f 100644 (file)
@@ -227,12 +227,8 @@ static void removeBlockFromDescendantAndContainerMaps(RenderBlock* block, Tracke
 
 RenderBlock::~RenderBlock()
 {
-    if (m_floatingObjects)
-        deleteAllValues(m_floatingObjects->set());
-    
     if (hasColumns())
         gColumnInfoMap->take(this);
-
     if (gPercentHeightDescendantsMap)
         removeBlockFromDescendantAndContainerMaps(this, gPercentHeightDescendantsMap, gPercentHeightContainerMap);
     if (gPositionedDescendantsMap)
@@ -988,15 +984,8 @@ static void getInlineRun(RenderObject* start, RenderObject* boundary,
 
 void RenderBlock::deleteLineBoxTree()
 {
-    if (containsFloats()) {
-        // Clear references to originating lines, since the lines are being deleted
-        const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
-        FloatingObjectSetIterator end = floatingObjectSet.end();
-        for (FloatingObjectSetIterator it = floatingObjectSet.begin(); it != end; ++it) {
-            ASSERT(!((*it)->originatingLine()) || &(*it)->originatingLine()->renderer() == this);
-            (*it)->setOriginatingLine(0);
-        }
-    }
+    if (containsFloats())
+        m_floatingObjects->clearLineBoxTreePointers();
     m_lineBoxes.deleteLineBoxTree(renderArena());
 
     if (AXObjectCache* cache = document().existingAXObjectCache())
@@ -3042,7 +3031,9 @@ void RenderBlock::repaintOverhangingFloats(bool paintAllDescendants)
         // Only repaint the object if it is overhanging, is not in its own layer, and
         // is our responsibility to paint (m_shouldPaint is set). When paintAllDescendants is true, the latter
         // condition is replaced with being a descendant of us.
-        if (r->logicalBottom(isHorizontalWritingMode()) > logicalHeight() && ((paintAllDescendants && r->renderer()->isDescendantOf(this)) || r->shouldPaint()) && !r->renderer()->hasSelfPaintingLayer()) {
+        if (r->logicalBottom(isHorizontalWritingMode()) > logicalHeight()
+            && !r->renderer()->hasSelfPaintingLayer()
+            && (r->shouldPaint() || (paintAllDescendants && r->renderer()->isDescendantOf(this)))) {
             r->renderer()->repaint();
             r->renderer()->repaintOverhangingFloats(false);
         }
@@ -8237,6 +8228,22 @@ inline RenderBlock::FloatingObjects::FloatingObjects(const RenderBlock* renderer
 {
 }
 
+RenderBlock::FloatingObjects::~FloatingObjects()
+{
+    // FIXME: m_set should use OwnPtr instead.
+    deleteAllValues(m_set);
+}
+
+void RenderBlock::FloatingObjects::clearLineBoxTreePointers()
+{
+    // Clear references to originating lines, since the lines are being deleted
+    FloatingObjectSetIterator end = m_set.end();
+    for (FloatingObjectSetIterator it = m_set.begin(); it != end; ++it) {
+        ASSERT(!((*it)->originatingLine()) || &((*it)->originatingLine()->renderer()) == m_renderer);
+        (*it)->setOriginatingLine(0);
+    }
+}
+
 void RenderBlock::createFloatingObjects()
 {
     m_floatingObjects = adoptPtr(new FloatingObjects(this, isHorizontalWritingMode()));
@@ -8244,6 +8251,9 @@ void RenderBlock::createFloatingObjects()
 
 inline void RenderBlock::FloatingObjects::clear()
 {
+    // FIXME: This should call deleteAllValues, except RenderBlock::clearFloats
+    // like to play fast and loose with ownership of these pointers.
+    // If we move to OwnPtr that will fix this ownership oddness.
     m_set.clear();
     m_placedFloatsTree.clear();
     m_leftObjectsCount = 0;
index f386523..fefd386 100644 (file)
@@ -1251,6 +1251,8 @@ public:
     class FloatingObjects {
         WTF_MAKE_NONCOPYABLE(FloatingObjects); WTF_MAKE_FAST_ALLOCATED;
     public:
+        ~FloatingObjects();
+
         void clear();
         void add(FloatingObject*);
         void remove(FloatingObject*);
@@ -1266,6 +1268,7 @@ public:
             computePlacedFloatsTreeIfNeeded();
             return m_placedFloatsTree; 
         }
+        void clearLineBoxTreePointers();
     private:
         FloatingObjects(const RenderBlock*, bool horizontalWritingMode);
         void computePlacedFloatsTree();