Changeset 155065 in webkit

Timestamp:

Sep 4, 2013 2:35:56 PM (11 years ago)

Author:

Bem Jones-Bey

Message:

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderBlock.cpp (modified) (5 diffs)
• rendering/RenderBlock.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -228 +228 @@
 RenderBlock::~RenderBlock()
 {
-    if (m_floatingObjects)
-        deleteAllValues(m_floatingObjects->set());
-    
     if (hasColumns())
         gColumnInfoMap->take(this);
-
     if (gPercentHeightDescendantsMap)
         removeBlockFromDescendantAndContainerMaps(this, gPercentHeightDescendantsMap, gPercentHeightContainerMap);
@@ -989 +985 @@
 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());
 
@@ -3043 +3032 @@
         // 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);
@@ -8238 +8229 @@
 }
 
+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()
 {
@@ -8245 +8252 @@
 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();

trunk/Source/WebCore/rendering/RenderBlock.h

@@ -1252 +1252 @@
         WTF_MAKE_NONCOPYABLE(FloatingObjects); WTF_MAKE_FAST_ALLOCATED;
     public:
+        ~FloatingObjects();
+
         void clear();
         void add(FloatingObject*);
@@ -1267 +1269 @@
             return m_placedFloatsTree; 
         }
+        void clearLineBoxTreePointers();
     private:
         FloatingObjects(const RenderBlock*, bool horizontalWritingMode);