LayoutTests:
authorantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Aug 2007 23:29:50 +0000 (23:29 +0000)
committerantti <antti@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Aug 2007 23:29:50 +0000 (23:29 +0000)
        Reviewed by Hyatt.

        Test for <rdar://problem/5403773>
        CrashTracer: [USER] 88 crashes in Safari at com.apple.WebCore: WebCore::RenderTableSection::paint + 846

        Changed results for fast/dynamic/containing-block-change.html are progression
        (even though new results don't match Firefox and old ones did!)

        * fast/dynamic/ancestor-to-absolute-expected.txt: Added.
        * fast/dynamic/ancestor-to-absolute.html: Added.
        * fast/dynamic/containing-block-change-expected.checksum:
        * fast/dynamic/containing-block-change-expected.png:
        * fast/dynamic/containing-block-change-expected.txt:

WebCore:

        Reviewed by Hyatt.

        Fix <rdar://problem/5403773>
        CrashTracer: [USER] 88 crashes in Safari at com.apple.WebCore: WebCore::RenderTableSection::paint + 846

        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::removePositionedObjects):

        Fix crash in http://www.infobae.com/interior/home.html
        Positioned objects removed from m_positionedObjects would in some cases not get added back to any
        positioned objects list. Adding objects happens in block layout but since layout was not invalidated
        correctly in removePositionedObjects() it would not get invoked. As a result some positioned objects
        would stay in layout dirty state leading to crashes and other bad things.

        * rendering/RenderTableSection.cpp:
        (WebCore::RenderTableSection::paint):

        Add needLayout() guard to eliminate this class of crashes from release builds.
        Assert commented out for now since one existing layout test can't handle it.

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

LayoutTests/ChangeLog
LayoutTests/fast/dynamic/ancestor-to-absolute-expected.txt [new file with mode: 0644]
LayoutTests/fast/dynamic/ancestor-to-absolute.html [new file with mode: 0644]
LayoutTests/fast/dynamic/containing-block-change-expected.checksum
LayoutTests/fast/dynamic/containing-block-change-expected.png
LayoutTests/fast/dynamic/containing-block-change-expected.txt
WebCore/ChangeLog
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RenderTableSection.cpp

index 161d701dc9cc33856669df60c52ef35ad1712455..6b846483ecf178ba67f22060d56c6e8e0e24a354 100644 (file)
@@ -1,3 +1,19 @@
+2007-08-17  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Hyatt.
+
+        Test for <rdar://problem/5403773>
+        CrashTracer: [USER] 88 crashes in Safari at com.apple.WebCore: WebCore::RenderTableSection::paint + 846
+        
+        Changed results for fast/dynamic/containing-block-change.html are progression
+        (even though new results don't match Firefox and old ones did!)
+
+        * fast/dynamic/ancestor-to-absolute-expected.txt: Added.
+        * fast/dynamic/ancestor-to-absolute.html: Added.
+        * fast/dynamic/containing-block-change-expected.checksum:
+        * fast/dynamic/containing-block-change-expected.png:
+        * fast/dynamic/containing-block-change-expected.txt:
+
 2007-08-17  Kevin Decker  <kdecker@apple.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/fast/dynamic/ancestor-to-absolute-expected.txt b/LayoutTests/fast/dynamic/ancestor-to-absolute-expected.txt
new file mode 100644 (file)
index 0000000..ca64333
--- /dev/null
@@ -0,0 +1,2 @@
+Test that dynamically making element absolute positioned does not corrupt it's childrens layout dirty state. This passes if it does not crash. 
+
diff --git a/LayoutTests/fast/dynamic/ancestor-to-absolute.html b/LayoutTests/fast/dynamic/ancestor-to-absolute.html
new file mode 100644 (file)
index 0000000..5858bec
--- /dev/null
@@ -0,0 +1,20 @@
+<body>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+Test that dynamically making element absolute positioned does not corrupt it's childrens layout dirty state. This passes if it does not crash.
+<div id=outer style="float:left" >
+  <div>
+    <div id=inner style="position:absolute; left:0px; top:0px; width:15px; height:15px;"></div>
+  </div>
+</div>
+<script>
+// force layout
+document.getElementById("outer").offsetHeight;
+// this would corrupt layout dirty bits
+document.getElementById("outer").style.position = "absolute";
+// this would turn corruption into crash
+document.getElementById("inner").innerHTML =  "<table><tr><td></td></tr></table>";
+</script>
+
index a91be28921089698ddd8d90f74ea4201b56e550a..4352836633edba912342d4dc02af644e164a40a1 100644 (file)
@@ -1 +1 @@
-26d62cfb10f4b7f1d0b36712332cba30
\ No newline at end of file
+01ba6b7ed71481077c4d7bfbc336864c
\ No newline at end of file
index 5167c6baf7df7d3e7cb00c11c53045caa3a65abc..66fbc210410420493517611d2c1f489c302def17 100644 (file)
Binary files a/LayoutTests/fast/dynamic/containing-block-change-expected.png and b/LayoutTests/fast/dynamic/containing-block-change-expected.png differ
index 7fe730772d102fe5a45a6f003ada020a564b77ad..abacea43181b62b566a8662c819f534582b1a1f8 100644 (file)
@@ -21,7 +21,8 @@ layer at (8,60) size 6x6
     RenderTableSection {TBODY} at (0,0) size 6x6
       RenderTableRow {TR} at (0,2) size 6x2
         RenderTableCell {TD} at (2,2) size 2x2 [r=0 c=0 rs=1 cs=1]
-layer at (19,123) size 84x18
-  RenderBlock (positioned) {DIV} at (11,63) size 84x18
-    RenderText {#text} at (0,0) size 84x18
-      text run at (0,0) width 84: "Lorem ipsum"
+layer at (11,63) size 42x36
+  RenderBlock (positioned) {DIV} at (3,3) size 42x36
+    RenderText {#text} at (0,0) size 42x36
+      text run at (0,0) width 42: "Lorem"
+      text run at (0,18) width 38: "ipsum"
index 75ffd961aa54e69211f4b0871a9bfe4e0f82bc33..a6277396a97541cc8f36bca8ddcdaa1864f824ae 100644 (file)
@@ -1,3 +1,25 @@
+2007-08-17  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Hyatt.
+        
+        Fix <rdar://problem/5403773>
+        CrashTracer: [USER] 88 crashes in Safari at com.apple.WebCore: WebCore::RenderTableSection::paint + 846
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::removePositionedObjects):
+        
+        Fix crash in http://www.infobae.com/interior/home.html
+        Positioned objects removed from m_positionedObjects would in some cases not get added back to any 
+        positioned objects list. Adding objects happens in block layout but since layout was not invalidated 
+        correctly in removePositionedObjects() it would not get invoked. As a result some positioned objects 
+        would stay in layout dirty state leading to crashes and other bad things.
+        
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::paint):
+        
+        Add needLayout() guard to eliminate this class of crashes from release builds. 
+        Assert commented out for now since one existing layout test can't handle it.
+
 2007-08-17  Kevin Decker <kdecker@apple.com>
 
         Code change by Darin, landed and reviewed by me.
index 78dfb3dfa21d2c63e52b5160ebc355faa533ccd4..8d35673b987d44e607257dae2d912e565e2fb599 100644 (file)
@@ -2084,6 +2084,15 @@ void RenderBlock::removePositionedObjects(RenderBlock* o)
         if (!o || it.current()->isDescendantOf(o)) {
             if (o)
                 it.current()->setChildNeedsLayout(true, false);
+            
+            // It is parent blocks job to add positioned child to positioned objects list of its containing block
+            // Parent layout needs to be invalidated to ensure this happens.
+            RenderObject* p = it.current()->parent();
+            while (p && !p->isRenderBlock())
+                p = p->parent();
+            if (p)
+                p->setChildNeedsLayout(true);
+            
             m_positionedObjects->removeRef(it.current());
         } else
             ++it;
index b00b184d764d6b5fed8f718aa5084c067f673b50..46c415b8faff5d56e81b0a4953becda7850b5dec 100644 (file)
@@ -856,6 +856,12 @@ void RenderTableSection::recalcOuterBorder()
 
 void RenderTableSection::paint(PaintInfo& paintInfo, int tx, int ty)
 {
+    // put this back in when all layout tests can handle it
+    // ASSERT(!needsLayout());
+    // avoid crashing on bugs that cause us to paint with dirty layout
+    if (needsLayout())
+        return;
+    
     unsigned totalRows = m_gridRows;
     unsigned totalCols = table()->columns().size();