Title: [126495] trunk/Source/WebCore
Revision
126495
Author
[email protected]
Date
2012-08-23 16:02:19 -0700 (Thu, 23 Aug 2012)

Log Message

Remove RenderTable::removeChild
https://bugs.webkit.org/show_bug.cgi?id=94842

Reviewed by Abhishek Arya.

This change removed removeChild, replaced by willBeRemovedFromTree calls. The upside is that
the invalidations are now guaranteed to run if we split a table (which is not guaranteed when
using removeChild). This change also shows that our code may be doing too much work in some
of the child's removal, in which case the code was marked as needed.

Covered by existing tests.

* rendering/RenderTable.cpp:
(WebCore::RenderTable::removeCaption):
Helper function used to remove the caption from our Vector. The invalidation are very likely
unneeded so added a comment about that.

* rendering/RenderTable.h:
* rendering/RenderTableCaption.cpp:
(WebCore::RenderTableCaption::willBeRemovedFromTree):
(WebCore::RenderTableCaption::table):
* rendering/RenderTableCaption.h:
* rendering/RenderTableCol.cpp:
(WebCore::RenderTableCol::willBeRemovedFromTree):
* rendering/RenderTableCol.h:
Added the following functions to do the invalidation.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (126494 => 126495)


--- trunk/Source/WebCore/ChangeLog	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/ChangeLog	2012-08-23 23:02:19 UTC (rev 126495)
@@ -1,3 +1,32 @@
+2012-08-23  Julien Chaffraix  <[email protected]>
+
+        Remove RenderTable::removeChild
+        https://bugs.webkit.org/show_bug.cgi?id=94842
+
+        Reviewed by Abhishek Arya.
+
+        This change removed removeChild, replaced by willBeRemovedFromTree calls. The upside is that
+        the invalidations are now guaranteed to run if we split a table (which is not guaranteed when
+        using removeChild). This change also shows that our code may be doing too much work in some
+        of the child's removal, in which case the code was marked as needed.
+
+        Covered by existing tests.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::removeCaption):
+        Helper function used to remove the caption from our Vector. The invalidation are very likely
+        unneeded so added a comment about that.
+
+        * rendering/RenderTable.h:
+        * rendering/RenderTableCaption.cpp:
+        (WebCore::RenderTableCaption::willBeRemovedFromTree):
+        (WebCore::RenderTableCaption::table):
+        * rendering/RenderTableCaption.h:
+        * rendering/RenderTableCol.cpp:
+        (WebCore::RenderTableCol::willBeRemovedFromTree):
+        * rendering/RenderTableCol.h:
+        Added the following functions to do the invalidation.
+
 2012-08-23  Mark Hahnenberg  <[email protected]>
 
         Change behavior of MasqueradesAsUndefined to better accommodate DFG changes

Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTable.cpp	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp	2012-08-23 23:02:19 UTC (rev 126495)
@@ -197,16 +197,17 @@
     section->addChild(child);
 }
 
-void RenderTable::removeChild(RenderObject* oldChild)
+void RenderTable::removeCaption(const RenderTableCaption* oldCaption)
 {
-    RenderBox::removeChild(oldChild);
- 
-    size_t index = m_captions.find(oldChild);
-    if (index != notFound) {
-        m_captions.remove(index);
-        if (node())
-            node()->setNeedsStyleRecalc();
-    }
+    size_t index = m_captions.find(oldCaption);
+    ASSERT(index != notFound);
+    m_captions.remove(index);
+
+    // FIXME: The rest of this function is probably not needed since we have 
+    // implemented proper multiple captions support (see bug 58249).
+    if (node())
+        node()->setNeedsStyleRecalc();
+
     setNeedsSectionRecalc();
 }
 

Modified: trunk/Source/WebCore/rendering/RenderTable.h (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTable.h	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTable.h	2012-08-23 23:02:19 UTC (rev 126495)
@@ -231,6 +231,8 @@
     const BorderValue& tableStartBorderAdjoiningCell(const RenderTableCell*) const;
     const BorderValue& tableEndBorderAdjoiningCell(const RenderTableCell*) const;
 
+    void removeCaption(const RenderTableCaption*);
+
 protected:
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
 
@@ -241,8 +243,6 @@
 
     virtual bool avoidsFloats() const { return true; }
 
-    virtual void removeChild(RenderObject* oldChild);
-
     virtual void paint(PaintInfo&, const LayoutPoint&);
     virtual void paintObject(PaintInfo&, const LayoutPoint&);
     virtual void paintBoxDecorations(PaintInfo&, const LayoutPoint&);

Modified: trunk/Source/WebCore/rendering/RenderTableCaption.cpp (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTableCaption.cpp	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTableCaption.cpp	2012-08-23 23:02:19 UTC (rev 126495)
@@ -20,6 +20,8 @@
 #include "config.h"
 #include "RenderTableCaption.h"
 
+#include "RenderTable.h"
+
 namespace WebCore {
 
 RenderTableCaption::RenderTableCaption(Node* node)
@@ -37,4 +39,16 @@
     return cb->logicalWidth();
 }
 
+void RenderTableCaption::willBeRemovedFromTree()
+{
+    RenderBlock::willBeRemovedFromTree();
+
+    table()->removeCaption(this);
 }
+
+RenderTable* RenderTableCaption::table() const
+{
+    return toRenderTable(parent());
+}
+
+}

Modified: trunk/Source/WebCore/rendering/RenderTableCaption.h (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTableCaption.h	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTableCaption.h	2012-08-23 23:02:19 UTC (rev 126495)
@@ -24,6 +24,8 @@
 
 namespace WebCore {
 
+class RenderTable;
+
 class RenderTableCaption : public RenderBlock {
 public:
     explicit RenderTableCaption(Node*);
@@ -32,6 +34,10 @@
     
 private:
     virtual bool isTableCaption() const OVERRIDE { return true; }
+
+    virtual void willBeRemovedFromTree() OVERRIDE;
+
+    RenderTable* table() const;
 };
 
 inline RenderTableCaption* toRenderTableCaption(RenderObject* object)

Modified: trunk/Source/WebCore/rendering/RenderTableCol.cpp (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTableCol.cpp	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTableCol.cpp	2012-08-23 23:02:19 UTC (rev 126495)
@@ -69,6 +69,16 @@
         setNeedsLayoutAndPrefWidthsRecalc();
 }
 
+void RenderTableCol::willBeRemovedFromTree()
+{
+    RenderBox::willBeRemovedFromTree();
+
+    // We don't really need to recompute our sections, but we need to update our
+    // column count and whether we have a column. Currently, we only have one
+    // size-fit-all flag but we may have to consider splitting it.
+    table()->setNeedsSectionRecalc();
+}
+
 bool RenderTableCol::isChildAllowed(RenderObject* child, RenderStyle* style) const
 {
     // We cannot use isTableColumn here as style() may return 0.

Modified: trunk/Source/WebCore/rendering/RenderTableCol.h (126494 => 126495)


--- trunk/Source/WebCore/rendering/RenderTableCol.h	2012-08-23 23:00:31 UTC (rev 126494)
+++ trunk/Source/WebCore/rendering/RenderTableCol.h	2012-08-23 23:02:19 UTC (rev 126495)
@@ -75,6 +75,8 @@
     virtual bool isRenderTableCol() const OVERRIDE { return true; }
     virtual void updateFromElement();
 
+    virtual void willBeRemovedFromTree() OVERRIDE;
+
     virtual bool isChildAllowed(RenderObject*, RenderStyle*) const;
     virtual bool canHaveChildren() const;
     virtual bool requiresLayer() const { return false; }
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to