fast/table/neighboring-cells-when-collapsed-border-changes.html is flaky.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 May 2016 20:02:31 +0000 (20:02 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 May 2016 20:02:31 +0000 (20:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158009

Reviewed by David Hyatt.

RenderTable::invalidateCollapsedBorders() early returns on m_collapsedEmptyBorderIsPresent
which prevents us from marking neighboring cells dirty when style changes.
Decouple the "invalidate the collapsed borders" and the "mark the neighboring cells dirty" logic.

Source/WebCore:

Covered by existing tests. (and now we agree with FF on cached-change-cell-border-width.html)

* rendering/RenderTable.cpp:
(WebCore::RenderTable::invalidateCollapsedBorders):
(WebCore::markCellDirtyWhenCollapsedBorderChanges): Deleted.
* rendering/RenderTableCell.cpp:
(WebCore::markCellDirtyWhenCollapsedBorderChanges):
(WebCore::RenderTableCell::styleDidChange):

LayoutTests:

* fast/table/border-collapsing/cached-change-cell-border-width-expected.txt: Progression. FF agrees.
* fast/table/neighboring-cells-when-collapsed-border-changes.html:
* platform/ios-simulator-wk1/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt:
* platform/mac-wk1/TestExpectations:
* platform/mac/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt
LayoutTests/fast/table/neighboring-cells-when-collapsed-border-changes.html
LayoutTests/platform/ios-simulator-wk1/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/platform/mac/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTableCell.cpp

index c09c6c3..124e80d 100644 (file)
@@ -1,3 +1,20 @@
+2016-05-24  Zalan Bujtas  <zalan@apple.com>
+
+        fast/table/neighboring-cells-when-collapsed-border-changes.html is flaky.
+        https://bugs.webkit.org/show_bug.cgi?id=158009
+
+        Reviewed by David Hyatt.
+
+        RenderTable::invalidateCollapsedBorders() early returns on m_collapsedEmptyBorderIsPresent
+        which prevents us from marking neighboring cells dirty when style changes.
+        Decouple the "invalidate the collapsed borders" and the "mark the neighboring cells dirty" logic.
+
+        * fast/table/border-collapsing/cached-change-cell-border-width-expected.txt: Progression. FF agrees.
+        * fast/table/neighboring-cells-when-collapsed-border-changes.html:
+        * platform/ios-simulator-wk1/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt:
+        * platform/mac-wk1/TestExpectations:
+        * platform/mac/fast/table/border-collapsing/cached-change-cell-border-width-expected.txt:
+
 2016-05-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking mathml/wbr-in-mroot-crash.html as flaky on ios-simulator-wk2
index 7bcd704..30d2e33 100644 (file)
@@ -3,8 +3,8 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
-      RenderTable {TABLE} at (0,0) size 113x54 [border: (2px solid #0000FF)]
-        RenderTableSection {TBODY} at (2,2) size 110x50
-          RenderTableRow {TR} at (0,0) size 110x50
-            RenderTableCell {TD} at (0,22) size 55x6 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
-            RenderTableCell {TD} at (55,23) size 55x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
+      RenderTable {TABLE} at (0,0) size 114x54 [border: (2px solid #0000FF)]
+        RenderTableSection {TBODY} at (2,2) size 111x50
+          RenderTableRow {TR} at (0,0) size 111x50
+            RenderTableCell {TD} at (0,22) size 56x6 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
+            RenderTableCell {TD} at (56,23) size 55x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
index 23978e6..a7c2b12 100644 (file)
@@ -22,7 +22,7 @@ setTimeout(function() {
     document.body.offsetWidth;
     if (window.testRunner)
         testRunner.notifyDone();
-}, 10);
+}, 0);
 </script>
 </body>
 </html>
index ac42dcf..30d2e33 100644 (file)
@@ -7,4 +7,4 @@ layer at (0,0) size 800x600
         RenderTableSection {TBODY} at (2,2) size 111x50
           RenderTableRow {TR} at (0,0) size 111x50
             RenderTableCell {TD} at (0,22) size 56x6 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
-            RenderTableCell {TD} at (55,23) size 56x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
+            RenderTableCell {TD} at (56,23) size 55x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
index 062e005..76fb464 100644 (file)
@@ -220,4 +220,3 @@ fast/loader/iframe-src-invalid-url.html [ Skip ]
 
 webkit.org/b/157589 fast/text-autosizing/ios/text-autosizing-after-back.html [ Pass Timeout ]
 
-webkit.org/b/158009 fast/table/neighboring-cells-when-collapsed-border-changes.html [ Pass ImageOnlyFailure ]
index 2a360c7..30d2e33 100644 (file)
@@ -3,8 +3,8 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
-      RenderTable {TABLE} at (0,0) size 113x54 [border: (2px solid #0000FF)]
-        RenderTableSection {TBODY} at (2,2) size 110x50
-          RenderTableRow {TR} at (0,0) size 110x50
+      RenderTable {TABLE} at (0,0) size 114x54 [border: (2px solid #0000FF)]
+        RenderTableSection {TBODY} at (2,2) size 111x50
+          RenderTableRow {TR} at (0,0) size 111x50
             RenderTableCell {TD} at (0,22) size 56x6 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
-            RenderTableCell {TD} at (55,23) size 56x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
+            RenderTableCell {TD} at (56,23) size 55x4 [border: (1px solid #000000)] [r=0 c=1 rs=1 cs=1]
index f7a0740..8842a5d 100644 (file)
@@ -1,3 +1,23 @@
+2016-05-24  Zalan Bujtas  <zalan@apple.com>
+
+        fast/table/neighboring-cells-when-collapsed-border-changes.html is flaky.
+        https://bugs.webkit.org/show_bug.cgi?id=158009
+
+        Reviewed by David Hyatt.
+
+        RenderTable::invalidateCollapsedBorders() early returns on m_collapsedEmptyBorderIsPresent
+        which prevents us from marking neighboring cells dirty when style changes.
+        Decouple the "invalidate the collapsed borders" and the "mark the neighboring cells dirty" logic.
+
+        Covered by existing tests. (and now we agree with FF on cached-change-cell-border-width.html)
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::invalidateCollapsedBorders):
+        (WebCore::markCellDirtyWhenCollapsedBorderChanges): Deleted.
+        * rendering/RenderTableCell.cpp:
+        (WebCore::markCellDirtyWhenCollapsedBorderChanges):
+        (WebCore::RenderTableCell::styleDidChange):
+
 2016-05-24  Alex Christensen  <achristensen@webkit.org>
 
         Fix use-after-free after r201318
index f6289b6..d8a8d6e 100644 (file)
@@ -594,14 +594,6 @@ void RenderTable::layout()
     clearNeedsLayout();
 }
 
-static inline void markCellDirtyWhenCollapsedBorderChanges(RenderTableCell* cell)
-{
-    if (!cell)
-        return;
-    cell->invalidateHasEmptyCollapsedBorders();
-    cell->setNeedsLayoutAndPrefWidthsRecalc();
-}
-
 void RenderTable::invalidateCollapsedBorders(RenderTableCell* cellWithStyleChange)
 {
     m_collapsedBordersValid = false;
@@ -616,10 +608,14 @@ void RenderTable::invalidateCollapsedBorders(RenderTableCell* cellWithStyleChang
     if (cellWithStyleChange) {
         // It is enough to invalidate just the surrounding cells when cell border style changes.
         cellWithStyleChange->invalidateHasEmptyCollapsedBorders();
-        markCellDirtyWhenCollapsedBorderChanges(cellBelow(cellWithStyleChange));
-        markCellDirtyWhenCollapsedBorderChanges(cellAbove(cellWithStyleChange));
-        markCellDirtyWhenCollapsedBorderChanges(cellBefore(cellWithStyleChange));
-        markCellDirtyWhenCollapsedBorderChanges(cellAfter(cellWithStyleChange));
+        if (auto* below = cellBelow(cellWithStyleChange))
+            below->invalidateHasEmptyCollapsedBorders();
+        if (auto* above = cellAbove(cellWithStyleChange))
+            above->invalidateHasEmptyCollapsedBorders();
+        if (auto* before = cellBefore(cellWithStyleChange))
+            before->invalidateHasEmptyCollapsedBorders();
+        if (auto* after = cellAfter(cellWithStyleChange))
+            after->invalidateHasEmptyCollapsedBorders();
         return;
     }
 
index 9b1f9da..1d7906d 100644 (file)
@@ -404,6 +404,13 @@ LayoutUnit RenderTableCell::cellBaselinePosition() const
     return firstLineBaseline().valueOr(borderAndPaddingBefore() + contentLogicalHeight());
 }
 
+static inline void markCellDirtyWhenCollapsedBorderChanges(RenderTableCell* cell)
+{
+    if (!cell)
+        return;
+    cell->setNeedsLayoutAndPrefWidthsRecalc();
+}
+
 void RenderTableCell::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     ASSERT(style().display() == TABLE_CELL);
@@ -422,8 +429,15 @@ void RenderTableCell::styleDidChange(StyleDifference diff, const RenderStyle* ol
 
     // If border was changed, notify table.
     RenderTable* table = this->table();
-    if (table && oldStyle && oldStyle->border() != style().border())
+    if (table && oldStyle && oldStyle->border() != style().border()) {
         table->invalidateCollapsedBorders(this);
+        if (table->collapseBorders() && diff == StyleDifferenceLayout) {
+            markCellDirtyWhenCollapsedBorderChanges(table->cellBelow(this));
+            markCellDirtyWhenCollapsedBorderChanges(table->cellAbove(this));
+            markCellDirtyWhenCollapsedBorderChanges(table->cellBefore(this));
+            markCellDirtyWhenCollapsedBorderChanges(table->cellAfter(this));
+        }
+    }
 }
 
 // The following rules apply for resolving conflicts and figuring out which border