WebCore:

        Reviewed by Darin Adler.

        - fix <rdar://problem/5619240> REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout)

        Test: fast/dynamic/subtree-common-root.html

        * page/FrameView.cpp:
        (WebCore::FrameView::layoutRoot): Added a parameter to let this method
        return the layout root for a pending layout as well.
        (WebCore::FrameView::scheduleRelayoutOfSubtree): Pass the new root
        to markContainingBlocksForLayout(). Otherwise,
        markContainingBlocksForLayout() could mark past the new root, if it had
        previously been marked as having a normal child needing layout and then
        was reached via a positioned child.
        * page/FrameView.h:
        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::calcWidth):
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::~RenderObject): Fixed the ASSERT so that
        it would really catch deletion of the layout root.
        (WebCore::RenderObject::markContainingBlocksForLayout): Added the
        newRoot parameter, which tells this method where to stop marking.
        * rendering/RenderObject.h:

LayoutTests:

        Reviewed by Darin Adler.

        - test for <rdar://problem/5619240> REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout)

        * fast/dynamic/subtree-common-root-expected.txt: Added.
        * fast/dynamic/subtree-common-root.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index a18f7989b7d00c44ba4d5d55b0d6806c2c14dcf3..88afeb7b270f6a5164185d7426d37642261129c1 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2007-12-01  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Darin Adler.
+
+        - test for <rdar://problem/5619240> REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout)
+
+        * fast/dynamic/subtree-common-root-expected.txt: Added.
+        * fast/dynamic/subtree-common-root.html: Added.
+
 2007-11-30  Eric Seidel  <eric@webkit.org>
 
         Reviewed by darin.

diff --git a/LayoutTests/fast/dynamic/subtree-common-root-expected.txt b/LayoutTests/fast/dynamic/subtree-common-root-expected.txt
new file mode 100644 (file)
index 0000000..c69ed4c
--- /dev/null
+++ b/LayoutTests/fast/dynamic/subtree-common-root-expected.txt
@@ -0,0 +1 @@
+Test for rdar://problem/5619240 REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout).

diff --git a/LayoutTests/fast/dynamic/subtree-common-root.html b/LayoutTests/fast/dynamic/subtree-common-root.html
new file mode 100644 (file)
index 0000000..2d065ca
--- /dev/null
+++ b/LayoutTests/fast/dynamic/subtree-common-root.html
@@ -0,0 +1,16 @@
+<p>
+    Test for <i><a href="rdar://problem/5619240">rdar://problem/5619240</a> REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout)</i>.
+</p>
+<div id="target">
+    <div style="overflow: hidden; width: 400px; height: 400px; position: relative;">
+        <div></div>
+        <div style="position: absolute; overflow: hidden; top: 0; width: 50px; height: 50px;"><div></div></div>
+    </div>
+</div>
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    document.body.offsetTop;
+    document.getElementById("target").style.display = "none";
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index f8e7c8e234b80b1ad36f2a8aa5cf72b05c5e4f4b..08bf23e4470432e9c50fd22cc91e25d1eede9632 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2007-12-01  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Darin Adler.
+
+        - fix <rdar://problem/5619240> REGRESSION (Leopard-r28069): Reproducible crash with a Mootools-based calendar picker (jump to null in FrameView::layout)
+
+        Test: fast/dynamic/subtree-common-root.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layoutRoot): Added a parameter to let this method
+        return the layout root for a pending layout as well.
+        (WebCore::FrameView::scheduleRelayoutOfSubtree): Pass the new root
+        to markContainingBlocksForLayout(). Otherwise,
+        markContainingBlocksForLayout() could mark past the new root, if it had
+        previously been marked as having a normal child needing layout and then
+        was reached via a positioned child.
+        * page/FrameView.h:
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::calcWidth):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::~RenderObject): Fixed the ASSERT so that
+        it would really catch deletion of the layout root.
+        (WebCore::RenderObject::markContainingBlocksForLayout): Added the
+        newRoot parameter, which tells this method where to stop marking.
+        * rendering/RenderObject.h:
+
 2007-12-01  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Darin Adler.

diff --git a/WebCore/page/FrameView.cpp b/WebCore/page/FrameView.cpp
index 6ab01366bc60dfd0205f4377349fb37b207d9083..596226f5ebfcf403c229d2d8e6228ea8ce6ad9b6 100644 (file)
--- a/WebCore/page/FrameView.cpp
+++ b/WebCore/page/FrameView.cpp
@@ -284,9 +284,9 @@ void FrameView::addRepaintInfo(RenderObject* o, const IntRect& r)
     d->repaintRects.append(RenderObject::RepaintInfo(o, r));
 }
 
-RenderObject* FrameView::layoutRoot() const
+RenderObject* FrameView::layoutRoot(bool onlyDuringLayout) const
 {
-    return layoutPending() ? 0 : d->layoutRoot;
+    return onlyDuringLayout && layoutPending() ? 0 : d->layoutRoot;
 }
 
 void FrameView::layout(bool allowSubtree)
@@ -737,10 +737,10 @@ void FrameView::scheduleRelayoutOfSubtree(RenderObject* o)
         if (d->layoutRoot != o) {
             if (isObjectAncestorContainerOf(d->layoutRoot, o)) {
                 // Keep the current root
-                o->markContainingBlocksForLayout(false);
+                o->markContainingBlocksForLayout(false, d->layoutRoot);
             } else if (d->layoutRoot && isObjectAncestorContainerOf(o, d->layoutRoot)) {
                 // Re-root at o
-                d->layoutRoot->markContainingBlocksForLayout(false);
+                d->layoutRoot->markContainingBlocksForLayout(false, o);
                 d->layoutRoot = o;
             } else {
                 // Just do a full relayout

diff --git a/WebCore/page/FrameView.h b/WebCore/page/FrameView.h
index 133884205af8baa1e9130f36de3258cb02cdf0a9..624dbc525a2ad55b8076ba99f56aa62037fb4c70 100644 (file)
--- a/WebCore/page/FrameView.h
+++ b/WebCore/page/FrameView.h
@@ -76,7 +76,7 @@ public:
     void unscheduleRelayout();
     bool layoutPending() const;
 
-    RenderObject* layoutRoot() const;
+    RenderObject* layoutRoot(bool onlyDuringLayout = false) const;
     int layoutCount() const;
 
     // These two helper functions just pass through to the RenderView.

diff --git a/WebCore/rendering/RenderBox.cpp b/WebCore/rendering/RenderBox.cpp
index 8070624ddc74bc4a2872ce7e0b5398447a5c8972..6d2e7a760ecf893b8b3bdcd3cc1db9bc53b0ccf9 100644 (file)
--- a/WebCore/rendering/RenderBox.cpp
+++ b/WebCore/rendering/RenderBox.cpp
@@ -1081,7 +1081,7 @@ void RenderBox::calcWidth()
     }
 
     // If layout is limited to a subtree, the subtree root's width does not change.
-    if (node() && view()->frameView() && view()->frameView()->layoutRoot() == this)
+    if (node() && view()->frameView() && view()->frameView()->layoutRoot(true) == this)
         return;
 
     // The parent box is flexing us, so it has increased or decreased our

diff --git a/WebCore/rendering/RenderObject.cpp b/WebCore/rendering/RenderObject.cpp
index 8e1eb1b90f709d99b0e095b4d6e9eea5d81fcea7..d190e99d2c941b09cc54dd4e655fa32fc9f0c7d2 100644 (file)
--- a/WebCore/rendering/RenderObject.cpp
+++ b/WebCore/rendering/RenderObject.cpp
@@ -201,7 +201,7 @@ RenderObject::RenderObject(Node* node)
 
 RenderObject::~RenderObject()
 {
-    ASSERT(!node() || !document()->frame()->view() || document()->frame()->view()->layoutRoot() != this);
+    ASSERT(!node() || documentBeingDestroyed() || !document()->frame()->view() || document()->frame()->view()->layoutRoot() != this);
 #ifndef NDEBUG
     --RenderObjectCounter::count;
 #endif
@@ -718,8 +718,10 @@ static inline bool objectIsRelayoutBoundary(const RenderObject *obj)
            ;
 }
     
-void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout)
+void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout, RenderObject* newRoot)
 {
+    ASSERT(!scheduleRelayout || !newRoot);
+
     RenderObject* o = container();
     RenderObject* last = this;
 
@@ -736,6 +738,9 @@ void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout)
             o->m_normalChildNeedsLayout = true;
         }
 
+        if (o == newRoot)
+            return;
+
         last = o;
         if (scheduleRelayout && objectIsRelayoutBoundary(last))
             break;

diff --git a/WebCore/rendering/RenderObject.h b/WebCore/rendering/RenderObject.h
index ef61f54087935b14c4a800b5f81f32a5432aca53..02ca4b069e37387afc113001d2258e7dec1fdf05 100644 (file)
--- a/WebCore/rendering/RenderObject.h
+++ b/WebCore/rendering/RenderObject.h
@@ -377,7 +377,7 @@ public:
     RenderObject* hoverAncestor() const;
 
     virtual void markAllDescendantsWithFloatsForLayout(RenderObject* floatToRemove = 0);
-    void markContainingBlocksForLayout(bool scheduleRelayout = true);
+    void markContainingBlocksForLayout(bool scheduleRelayout = true, RenderObject* newRoot = 0);
     void setNeedsLayout(bool b, bool markParents = true);
     void setChildNeedsLayout(bool b, bool markParents = true);