Use LocalStyleChange when changing TD presentation attributes.

Source/core/html/HTMLTableElement.cpp

Index: Source/core/html/HTMLTableElement.cpp
diff --git a/Source/core/html/HTMLTableElement.cpp b/Source/core/html/HTMLTableElement.cpp
index e1c901485d1c1a1b958ae9e4bae3e44f1f2a4438..80ff4554298534cb79a8a294c47f2c0e27c4aefe 100644
--- a/Source/core/html/HTMLTableElement.cpp
+++ b/Source/core/html/HTMLTableElement.cpp
@@ -34,6 +34,7 @@
 #include "core/css/CSSValuePool.h"
 #include "core/css/StylePropertySet.h"
 #include "core/dom/Attribute.h"
+#include "core/dom/ElementTraversal.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/html/HTMLTableCaptionElement.h"
 #include "core/html/HTMLTableRowElement.h"
@@ -251,29 +252,24 @@ void HTMLTableElement::deleteRow(int index, ExceptionState& exceptionState)
     row->remove(exceptionState);
 }
 
-static inline bool isTableCellAncestor(Node* n)
+static inline bool isTableCellAncestor(Element* e)
 {
-    return n->hasTagName(theadTag) || n->hasTagName(tbodyTag) ||
-           n->hasTagName(tfootTag) || n->hasTagName(trTag) ||
-           n->hasTagName(thTag);
+    return e->hasTagName(theadTag) || e->hasTagName(tbodyTag) || e->hasTagName(tfootTag)
+        || e->hasTagName(trTag) || e->hasTagName(thTag);

[mstensho (USE GERRIT), 2014/02/10 10:02:06]

This looks wrong (both before and after the change), regarding TH elements. A TH
element typically IS a table cell, not an ancestor. It's more important to do
this right now that no element will be marked for style recalc by accident
anymore.

[rune, 2014/02/18 10:53:06]

Yes, indeed. It was saved by enough SubtreeStyleChange before.

 }
 
-static bool setTableCellsChanged(Node* n)
+void HTMLTableElement::setCellsNeedStyleRecalc() const
 {
-    ASSERT(n);
-    bool cellChanged = false;
-
-    if (n->hasTagName(tdTag))
-        cellChanged = true;
-    else if (isTableCellAncestor(n)) {
-        for (Node* child = n->firstChild(); child; child = child->nextSibling())
-            cellChanged |= setTableCellsChanged(child);
+    Element* element = ElementTraversal::next(*this, this);
+    while (element) {
+        if (element->hasTagName(tdTag)) {

[mstensho (USE GERRIT), 2014/02/10 10:02:06]

Not that it was explicitly taken care of before your change, but, what about TH
elements? It's probably more important now that the table doesn't get its
subtree marked for style recalc as long as there is at least one TD element
somewhere.

[rune, 2014/02/18 10:53:06]

Done.

+            element->setNeedsStyleRecalc(LocalStyleChange);
+        } else if (isTableCellAncestor(element)) {
+            element = ElementTraversal::next(*element, this);
+            continue;
+        }
+        element = ElementTraversal::nextSkippingChildren(*element, this);
     }
-
-    if (cellChanged)
-        n->setNeedsStyleRecalc(SubtreeStyleChange);
-
-    return cellChanged;
 }
 
 static bool getBordersFromFrameAttributeValue(const AtomicString& value, bool& borderTop, bool& borderRight, bool& borderBottom, bool& borderLeft)
@@ -407,11 +403,7 @@ void HTMLTableElement::parseAttribute(const QualifiedName& name, const AtomicStr
 
     if (bordersBefore != cellBorders() || oldPadding != m_padding) {
         m_sharedCellStyle = 0;
-        bool cellChanged = false;
-        for (Node* child = firstChild(); child; child = child->nextSibling())
-            cellChanged |= setTableCellsChanged(child);
-        if (cellChanged)
-            setNeedsStyleRecalc(SubtreeStyleChange);
+        setCellsNeedStyleRecalc();

[mstensho (USE GERRIT), 2014/02/10 10:02:06]

It looks like table columns, column groups and row/header/footer groups also may
need their style recalculated, if table border changes, at least if border
collapsing is enabled. I suppose this could have worked by accident before, but
it won't anymore.

I wonder if you should just recalculate style on all TD, TH, TR, TBODY, THEAD,
TFOOT, COL and COLGROUP elements in the table. The current solution is to
recalculate style on all TD elements, anyway, which I guess typically comprise a
lot more than 50% of all elements in question.

[rune, 2014/02/18 10:53:06]

Done.

     }
 }