WebCore:
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2007 05:23:20 +0000 (05:23 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2007 05:23:20 +0000 (05:23 +0000)
        Reviewed by Eric Seidel.

        - fix <rdar://problem/5607037> REGRESSION (r27351): Departure date does not repaint when changed on Google Maps public transit planner (16034)

        Test: fast/repaint/subtree-root-skipped.html

        * page/FrameView.cpp:
        (WebCore::FrameViewPrivate::FrameViewPrivate): Initialize the layout
        root to 0.
        (WebCore::FrameView::layoutRoot): Changed to return a RenderObject
        instead of a Node.
        (WebCore::FrameView::layout): Changed for layout root being a renderer
        rather than a DOM node. Also replaced clearing the repaint rects
        set with asserting that it is empty if this is the top-level call to
        layout(). If it is not, the set may contain rects from enclosing
        layout() and those should not be removed.
        (WebCore::FrameView::scheduleRelayout): Changed for layout root being
        a renderer rather than a DOM node.
        (WebCore::isObjectAncestorContainerOf): Added this helper function that
        tests whether one object will be marked by calling
        markContainingBlocksForLayout() on the other.
        (WebCore::FrameView::scheduleRelayoutOfSubtree): Changed for layout
        root being a renderer rather than a DOM node. Changed the check if new
        and current layout roots are on the same path from the root to use
        the subgraph of the render tree defined by container()hood instead of
        the DOM tree and parenthood.
        * page/FrameView.h:
        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::calcWidth): Changed for layout root being a
        renderer rather than a DOM node.
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::~RenderObject): Added an assertion that the
        object being deleted is not currently the layout root.
        (WebCore::RenderObject::scheduleRelayout): Changed for layout root being
        a renderer rather than a DOM node.

LayoutTests:

        Reviewed by Eric Seidel.

        - repaint test for <rdar://problem/5607037> REGRESSION (r27351): Departure date does not repaint when changed on Google Maps public transit planner (16034)

        * fast/repaint/subtree-root-skipped.html: Added.
        * platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.checksum: Added.
        * platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png: Added.
        * platform/mac/fast/repaint/subtree-root-skipped-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/repaint/subtree-root-skipped.html [new file with mode: 0644]
LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/repaint/subtree-root-skipped-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/page/FrameView.cpp
WebCore/page/FrameView.h
WebCore/rendering/RenderBox.cpp
WebCore/rendering/RenderObject.cpp

index 292da1b39971c456eef9884f91d153bb63bc1ea9..b882ce3358285ebb6ecaa79139b69010be1ff566 100644 (file)
@@ -1,3 +1,14 @@
+2007-11-21  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Eric Seidel.
+
+        - repaint test for <rdar://problem/5607037> REGRESSION (r27351): Departure date does not repaint when changed on Google Maps public transit planner (16034)
+
+        * fast/repaint/subtree-root-skipped.html: Added.
+        * platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.checksum: Added.
+        * platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png: Added.
+        * platform/mac/fast/repaint/subtree-root-skipped-expected.txt: Added.
+
 2007-11-21  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Maciej.
diff --git a/LayoutTests/fast/repaint/subtree-root-skipped.html b/LayoutTests/fast/repaint/subtree-root-skipped.html
new file mode 100644 (file)
index 0000000..1bd6493
--- /dev/null
@@ -0,0 +1,26 @@
+<head>
+    <title>Repaint test for bug 16034</title>
+    <script src="repaint.js" type="text/javascript"></script>
+    <script>
+        function repaintTest()
+        {
+            var input = document.getElementById("input");
+            input.value = "PASS";
+            var div = document.getElementById("div");
+            div.style.height = "20px";
+        }
+    </script>
+</head>
+<body onload="runRepaintTest()">
+    <div style="overflow: hidden; height: 300px; width: 300px; background-color: aliceblue; position: relative;">
+        <div style="overflow: hidden; height: 200px; width: 200px; background-color: silver;">
+            <input id="input" value="FAIL">
+            <div id="selection">Selection is here</div>
+            <div id="div" style="position: absolute; bottom: 0; height: 10px; width: 10px; background-color: lightblue;"></div>
+        </div>
+    </div>
+    <script>
+        var selection = document.getElementById("selection");
+        getSelection().setBaseAndExtent(selection, 0, selection, 0);
+    </script>
+</body>
diff --git a/LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.checksum b/LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.checksum
new file mode 100644 (file)
index 0000000..393b71b
--- /dev/null
@@ -0,0 +1 @@
+abf3bc5ecbc70d4423e4e9fe714242b9
\ No newline at end of file
diff --git a/LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png b/LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png
new file mode 100644 (file)
index 0000000..62be81f
Binary files /dev/null and b/LayoutTests/platform/mac-leopard/fast/repaint/subtree-root-skipped-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/repaint/subtree-root-skipped-expected.txt b/LayoutTests/platform/mac/fast/repaint/subtree-root-skipped-expected.txt
new file mode 100644 (file)
index 0000000..2d6ab47
--- /dev/null
@@ -0,0 +1,22 @@
+layer at (0,0) size 800x600
+  RenderView 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
+layer at (8,8) size 300x300
+  RenderBlock (relative positioned) {DIV} at (0,0) size 300x300 [bgcolor=#F0F8FF]
+layer at (8,8) size 200x200
+  RenderBlock {DIV} at (0,0) size 200x200 [bgcolor=#C0C0C0]
+    RenderBlock (anonymous) at (0,0) size 200x23
+      RenderTextControl {INPUT} at (2,2) size 148x19 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
+      RenderText {#text} at (0,0) size 0x0
+    RenderBlock {DIV} at (0,23) size 200x18
+      RenderText {#text} at (0,0) size 103x18
+        text run at (0,0) width 103: "Selection is here"
+layer at (13,13) size 142x13
+  RenderBlock {DIV} at (3,3) size 142x13
+    RenderText {#text} at (1,0) size 26x13
+      text run at (1,0) width 26: "PASS"
+layer at (8,288) size 10x20
+  RenderBlock (positioned) {DIV} at (0,280) size 10x20 [bgcolor=#ADD8E6]
+caret: position 0 of child 0 {#text} of child 3 {DIV} of child 1 {DIV} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 01200939f7cef7df9d09fbb65e4c45e1bfb3ca7a..544fa364f5a2453f20f73837c72a02a517e5c28d 100644 (file)
@@ -1,3 +1,41 @@
+2007-11-21  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Eric Seidel.
+
+        - fix <rdar://problem/5607037> REGRESSION (r27351): Departure date does not repaint when changed on Google Maps public transit planner (16034)
+
+        Test: fast/repaint/subtree-root-skipped.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameViewPrivate::FrameViewPrivate): Initialize the layout
+        root to 0.
+        (WebCore::FrameView::layoutRoot): Changed to return a RenderObject
+        instead of a Node.
+        (WebCore::FrameView::layout): Changed for layout root being a renderer
+        rather than a DOM node. Also replaced clearing the repaint rects
+        set with asserting that it is empty if this is the top-level call to
+        layout(). If it is not, the set may contain rects from enclosing
+        layout() and those should not be removed.
+        (WebCore::FrameView::scheduleRelayout): Changed for layout root being
+        a renderer rather than a DOM node.
+        (WebCore::isObjectAncestorContainerOf): Added this helper function that
+        tests whether one object will be marked by calling
+        markContainingBlocksForLayout() on the other.
+        (WebCore::FrameView::scheduleRelayoutOfSubtree): Changed for layout
+        root being a renderer rather than a DOM node. Changed the check if new
+        and current layout roots are on the same path from the root to use
+        the subgraph of the render tree defined by container()hood instead of
+        the DOM tree and parenthood.
+        * page/FrameView.h:
+        * rendering/RenderBox.cpp: 
+        (WebCore::RenderBox::calcWidth): Changed for layout root being a
+        renderer rather than a DOM node.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::~RenderObject): Added an assertion that the
+        object being deleted is not currently the layout root.
+        (WebCore::RenderObject::scheduleRelayout): Changed for layout root being
+        a renderer rather than a DOM node.
+
 2007-11-21  Mark Rowe  <mrowe@apple.com>
 
         Reviewed by Eric.
index 5cb86c24c09351096a6d8904d8c5dd3e631038f1..6ab01366bc60dfd0205f4377349fb37b207d9083 100644 (file)
@@ -57,6 +57,7 @@ public:
     FrameViewPrivate(FrameView* view)
         : m_slowRepaintObjectCount(0)
         , layoutTimer(view, &FrameView::layoutTimerFired)
+        , layoutRoot(0)
         , m_mediaType("screen")
         , m_enqueueEvents(0)
         , m_overflowStatusDirty(true)
@@ -99,7 +100,7 @@ public:
 
     Timer<FrameView> layoutTimer;
     bool delayedLayout;
-    RefPtr<Node> layoutRoot;
+    RenderObject* layoutRoot;
     
     bool layoutSchedulingEnabled;
     bool midLayout;
@@ -283,9 +284,9 @@ void FrameView::addRepaintInfo(RenderObject* o, const IntRect& r)
     d->repaintRects.append(RenderObject::RepaintInfo(o, r));
 }
 
-Node* FrameView::layoutRoot() const
+RenderObject* FrameView::layoutRoot() const
 {
-    return layoutPending() ? 0 : d->layoutRoot.get();
+    return layoutPending() ? 0 : d->layoutRoot;
 }
 
 void FrameView::layout(bool allowSubtree)
@@ -312,8 +313,7 @@ void FrameView::layout(bool allowSubtree)
         return;
 
     if (!allowSubtree && d->layoutRoot) {
-        if (d->layoutRoot->renderer())
-            d->layoutRoot->renderer()->markContainingBlocksForLayout(false);
+        d->layoutRoot->markContainingBlocksForLayout(false);
         d->layoutRoot = 0;
     }
 
@@ -335,14 +335,13 @@ void FrameView::layout(bool allowSubtree)
         document->recalcStyle();
     
     bool subtree = d->layoutRoot;
-    Node* rootNode = subtree ? d->layoutRoot.get() : document;
 
     // If there is only one ref to this view left, then its going to be destroyed as soon as we exit, 
     // so there's no point to continuiing to layout
     if (protector->hasOneRef())
         return;
 
-    RenderObject* root = rootNode->renderer();
+    RenderObject* root = subtree ? d->layoutRoot : document->renderer();
     if (!root) {
         // FIXME: Do we need to set m_size here?
         d->layoutSchedulingEnabled = true;
@@ -355,7 +354,6 @@ void FrameView::layout(bool allowSubtree)
     ScrollbarMode vMode = d->vmode;
     
     if (!subtree) {
-        Document* document = static_cast<Document*>(rootNode);
         RenderObject* rootRenderer = document->documentElement() ? document->documentElement()->renderer() : 0;
         if (document->isHTMLDocument()) {
             Node* body = static_cast<HTMLDocument*>(document)->body();
@@ -384,7 +382,7 @@ void FrameView::layout(bool allowSubtree)
     }
 
     d->doFullRepaint = !subtree && (d->firstLayout || static_cast<RenderView*>(root)->printing());
-    d->repaintRects.clear();
+    ASSERT(d->nestedLayoutCount > 1 || d->repaintRects.isEmpty());
 
     bool didFirstLayout = false;
     if (!subtree) {
@@ -689,8 +687,7 @@ void FrameView::scheduleRelayout()
     ASSERT(m_frame->view() == this);
 
     if (d->layoutRoot) {
-        if (d->layoutRoot->renderer())
-            d->layoutRoot->renderer()->markContainingBlocksForLayout(false);
+        d->layoutRoot->markContainingBlocksForLayout(false);
         d->layoutRoot = 0;
     }
     if (!d->layoutSchedulingEnabled)
@@ -715,41 +712,47 @@ void FrameView::scheduleRelayout()
     d->layoutTimer.startOneShot(delay * 0.001);
 }
 
-void FrameView::scheduleRelayoutOfSubtree(Node* n)
+static bool isObjectAncestorContainerOf(RenderObject* ancestor, RenderObject* descendant)
+{
+    for (RenderObject* r = descendant; r; r = r->container()) {
+        if (r == ancestor)
+            return true;
+    }
+    return false;
+}
+
+void FrameView::scheduleRelayoutOfSubtree(RenderObject* o)
 {
     ASSERT(m_frame->view() == this);
 
     if (!d->layoutSchedulingEnabled || (m_frame->document()
             && m_frame->document()->renderer()
             && m_frame->document()->renderer()->needsLayout())) {
-        if (n->renderer())
-            n->renderer()->markContainingBlocksForLayout(false);
+        if (o)
+            o->markContainingBlocksForLayout(false);
         return;
     }
 
     if (layoutPending()) {
-        if (d->layoutRoot != n) {
-            if (n->isDescendantOf(d->layoutRoot.get())) {
+        if (d->layoutRoot != o) {
+            if (isObjectAncestorContainerOf(d->layoutRoot, o)) {
                 // Keep the current root
-                if (n->renderer())
-                    n->renderer()->markContainingBlocksForLayout(false);
-            } else if (d->layoutRoot && d->layoutRoot->isDescendantOf(n)) {
-                // Re-root at n
-                if (d->layoutRoot->renderer())
-                    d->layoutRoot->renderer()->markContainingBlocksForLayout(false);
-                d->layoutRoot = n;
+                o->markContainingBlocksForLayout(false);
+            } else if (d->layoutRoot && isObjectAncestorContainerOf(o, d->layoutRoot)) {
+                // Re-root at o
+                d->layoutRoot->markContainingBlocksForLayout(false);
+                d->layoutRoot = o;
             } else {
                 // Just do a full relayout
-                if (d->layoutRoot && d->layoutRoot->renderer())
-                    d->layoutRoot->renderer()->markContainingBlocksForLayout(false);
+                if (d->layoutRoot)
+                    d->layoutRoot->markContainingBlocksForLayout(false);
                 d->layoutRoot = 0;
-                if (n->renderer())
-                    n->renderer()->markContainingBlocksForLayout(false);
+                o->markContainingBlocksForLayout(false);
             }
         }
     } else {
         int delay = m_frame->document()->minimumLayoutDelay();
-        d->layoutRoot = n;
+        d->layoutRoot = o;
         d->delayedLayout = delay != 0;
         d->layoutTimer.startOneShot(delay * 0.001);
     }
index 039a59b7533c41555b9d647eb3f746caf517a6a2..133884205af8baa1e9130f36de3258cb02cdf0a9 100644 (file)
@@ -72,11 +72,11 @@ public:
     bool didFirstLayout() const;
     void layoutTimerFired(Timer<FrameView>*);
     void scheduleRelayout();
-    void scheduleRelayoutOfSubtree(Node*);
+    void scheduleRelayoutOfSubtree(RenderObject*);
     void unscheduleRelayout();
     bool layoutPending() const;
 
-    Node* layoutRoot() const;
+    RenderObject* layoutRoot() const;
     int layoutCount() const;
 
     // These two helper functions just pass through to the RenderView.
index d8f79a30a803876102a25a501879821df93f024d..c04ee1d3812226418ab3e630394ed9cc5785a660 100644 (file)
@@ -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() == node())
+    if (node() && view()->frameView() && view()->frameView()->layoutRoot() == this)
         return;
 
     // The parent box is flexing us, so it has increased or decreased our
index fde65cf6e8d7b9cc01170a885af4e048b062d417..d8366dfb104a2c35ac9dbbfa043e8c9ee1982d95 100644 (file)
@@ -201,6 +201,7 @@ RenderObject::RenderObject(Node* node)
 
 RenderObject::~RenderObject()
 {
+    ASSERT(!node() || !document()->frame()->view() || document()->frame()->view()->layoutRoot() != this);
 #ifndef NDEBUG
     --RenderObjectCounter::count;
 #endif
@@ -2733,7 +2734,7 @@ void RenderObject::scheduleRelayout()
     } else if (parent()) {
         FrameView* v = view() ? view()->frameView() : 0;
         if (v)
-            v->scheduleRelayoutOfSubtree(node());
+            v->scheduleRelayoutOfSubtree(this);
     }
 }