WebCore:
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Apr 2009 22:33:48 +0000 (22:33 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Apr 2009 22:33:48 +0000 (22:33 +0000)
2009-04-08  David Hyatt  <hyatt@apple.com>

        Reviewed by Adam Roben and Darin Adler

        Fix for https://bugs.webkit.org/show_bug.cgi?id=12440, repaints inconsistent for
        fixed positioned elements.

        Rewrite the updateScrollers method to be more correct in its results.

        Test: fast/block/positioning/fixed-positioning-scrollbar-bug.html

        * dom/Document.cpp:
        (WebCore::Document::recalcStyle):
        (WebCore::Document::implicitClose):
        * page/FrameView.cpp:
        (WebCore::FrameView::createScrollbar):
        (WebCore::FrameView::layout):
        (WebCore::FrameView::adjustPageHeight):
        * page/FrameView.h:
        * page/win/FrameWin.cpp:
        (WebCore::computePageRectsForFrame):
        * platform/ScrollView.cpp:
        (WebCore::ScrollView::ScrollView):
        (WebCore::ScrollView::updateScrollbars):
        * platform/ScrollView.h:
        * rendering/RenderView.cpp:
        (WebCore::RenderView::layout):
        (WebCore::RenderView::docHeight):
        (WebCore::RenderView::docWidth):
        * rendering/RenderView.h:

WebKit/mac:

2009-04-08  David Hyatt  <hyatt@apple.com>

        Reviewed by Adam Roben and Darin Adler

        Fix for https://bugs.webkit.org/show_bug.cgi?id=12440, fixed positioned elements end up in
        inconsistent positions.  Rewrite updateScrollers to improve the correctness.

        * WebView/WebDynamicScrollBarsView.h:
        * WebView/WebDynamicScrollBarsView.m:
        (-[WebDynamicScrollBarsView updateScrollers]):

LayoutTests:

2009-04-08  David Hyatt  <hyatt@apple.com>

        Reviewed by Adam Roben and Darin Adler

        Updated results and new tests for https://bugs.webkit.org/show_bug.cgi?id=12440.

        * fast/block/positioning/fixed-positioning-scrollbar-bug.html: Added.
        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.checksum: Added.
        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png: Added.
        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt: Added.
        * platform/mac/fast/flexbox/016-expected.checksum:
        * platform/mac/fast/flexbox/016-expected.png:
        * platform/mac/fast/flexbox/016-expected.txt:
        * platform/mac/fast/flexbox/flex-hang-expected.checksum:
        * platform/mac/fast/flexbox/flex-hang-expected.png:
        * platform/mac/fast/flexbox/flex-hang-expected.txt:
        * platform/mac/fast/lists/001-expected.checksum:
        * platform/mac/fast/lists/001-expected.png:
        * platform/mac/fast/lists/001-expected.txt:

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

26 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/block/positioning/fixed-positioning-scrollbar-bug.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/flexbox/016-expected.checksum
LayoutTests/platform/mac/fast/flexbox/016-expected.png
LayoutTests/platform/mac/fast/flexbox/016-expected.txt
LayoutTests/platform/mac/fast/flexbox/flex-hang-expected.checksum
LayoutTests/platform/mac/fast/flexbox/flex-hang-expected.png
LayoutTests/platform/mac/fast/flexbox/flex-hang-expected.txt
LayoutTests/platform/mac/fast/lists/001-expected.checksum
LayoutTests/platform/mac/fast/lists/001-expected.png
LayoutTests/platform/mac/fast/lists/001-expected.txt
WebCore/ChangeLog
WebCore/dom/Document.cpp
WebCore/page/FrameView.cpp
WebCore/page/FrameView.h
WebCore/page/win/FrameWin.cpp
WebCore/platform/ScrollView.cpp
WebCore/platform/ScrollView.h
WebCore/rendering/RenderView.cpp
WebCore/rendering/RenderView.h
WebKit/mac/ChangeLog
WebKit/mac/WebView/WebDynamicScrollBarsView.h
WebKit/mac/WebView/WebDynamicScrollBarsView.m

index 17da284a0bd97861c218465f39f64ef207f79753..d6417c8799fb2974d950531823167b98d60cac8c 100644 (file)
@@ -1,3 +1,23 @@
+2009-04-08  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Adam Roben and Darin Adler
+
+        Updated results and new tests for https://bugs.webkit.org/show_bug.cgi?id=12440.
+
+        * fast/block/positioning/fixed-positioning-scrollbar-bug.html: Added.
+        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.checksum: Added.
+        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png: Added.
+        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt: Added.
+        * platform/mac/fast/flexbox/016-expected.checksum:
+        * platform/mac/fast/flexbox/016-expected.png:
+        * platform/mac/fast/flexbox/016-expected.txt:
+        * platform/mac/fast/flexbox/flex-hang-expected.checksum:
+        * platform/mac/fast/flexbox/flex-hang-expected.png:
+        * platform/mac/fast/flexbox/flex-hang-expected.txt:
+        * platform/mac/fast/lists/001-expected.checksum:
+        * platform/mac/fast/lists/001-expected.png:
+        * platform/mac/fast/lists/001-expected.txt:
+
 2009-04-08  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
diff --git a/LayoutTests/fast/block/positioning/fixed-positioning-scrollbar-bug.html b/LayoutTests/fast/block/positioning/fixed-positioning-scrollbar-bug.html
new file mode 100644 (file)
index 0000000..609770b
--- /dev/null
@@ -0,0 +1,16 @@
+<html>
+<head>
+       <title></title>
+</head>
+<body>
+    The purple box should stick to the bottom of the window.
+    <script>
+        document.body.offsetTop;    // Force layout now.
+    </script>
+    <div style="height: 2000px; position: absolute;">
+    </div>
+    <div style="position: fixed; bottom: 0; height: 100px; width: 100px; background: purple;"></div>
+    <!-- Try to load a resource. -->
+    <span style="display: none; background-image: url(data:image/png,);"></span>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.checksum b/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.checksum
new file mode 100644 (file)
index 0000000..eb0c777
--- /dev/null
@@ -0,0 +1 @@
+4d24193615b65dea1fcbaaeb968dfd2f
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png b/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png
new file mode 100644 (file)
index 0000000..a6d58ee
Binary files /dev/null and b/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt b/LayoutTests/platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt
new file mode 100644 (file)
index 0000000..1bcffce
--- /dev/null
@@ -0,0 +1,16 @@
+layer at (0,0) size 785x2026
+  RenderView at (0,0) size 785x600
+layer at (0,0) size 785x600
+  RenderBlock {HTML} at (0,0) size 785x600
+    RenderBody {BODY} at (8,8) size 769x584
+      RenderText {#text} at (0,0) size 367x18
+        text run at (0,0) width 367: "The purple box should stick to the bottom of the window. "
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+layer at (8,500) size 100x100
+  RenderBlock (positioned) {DIV} at (8,500) size 100x100 [bgcolor=#800080]
index 3a9e846df728830df8ed4968a592716bb2f135ba..74b025ee01adc101aadb302835889f2d7b7b6e8e 100644 (file)
@@ -1 +1 @@
-e28a973da4ebd916c5493205e0cf7866
\ No newline at end of file
+770ce5977c24643477c634416f5c0b82
\ No newline at end of file
index 3a29ba830ef5863b234a096bdfaf3e05aad38814..23295e6e7a7738ddd8e4294409da0fae595f62ce 100644 (file)
Binary files a/LayoutTests/platform/mac/fast/flexbox/016-expected.png and b/LayoutTests/platform/mac/fast/flexbox/016-expected.png differ
index b1e734dc698525f8c6e2e1889030aa6e532ed956..2bc12e56c1e341f5b59113e8078d918602a1fdc6 100644 (file)
@@ -1,9 +1,9 @@
-layer at (0,0) size 812x600
-  RenderView at (0,0) size 800x600
-layer at (0,0) size 812x600
-  RenderBlock {HTML} at (0,0) size 800x600 [bgcolor=#FFFFFF]
-    RenderBody {BODY} at (0,0) size 800x600
-      RenderFlexibleBox {DIV} at (0,0) size 800x600
+layer at (0,0) size 812x585
+  RenderView at (0,0) size 800x585
+layer at (0,0) size 812x585
+  RenderBlock {HTML} at (0,0) size 800x585 [bgcolor=#FFFFFF]
+    RenderBody {BODY} at (0,0) size 800x585
+      RenderFlexibleBox {DIV} at (0,0) size 800x585
         RenderBlock {DIV} at (0,0) size 800x54
           RenderText {#text} at (0,0) size 800x54
             text run at (0,0) width 485: "This header should remain at the top of the browser window as you resize it. "
@@ -12,13 +12,13 @@ layer at (0,0) size 812x600
             text run at (0,18) width 254: "filled with an olive-bordered green box. "
             text run at (254,18) width 499: "It should start beneath the header, end above the footer, and fill the width of the"
             text run at (0,36) width 111: "browser window."
-        RenderPartObject {IFRAME} at (0,54) size 812x528 [bgcolor=#008000] [border: (10px solid #808000)]
-          layer at (0,0) size 792x508
-            RenderView at (0,0) size 792x508
-          layer at (0,0) size 792x508
-            RenderBlock {HTML} at (0,0) size 792x508
-              RenderBody {BODY} at (8,8) size 776x492
-        RenderBlock {DIV} at (0,582) size 800x18
+        RenderPartObject {IFRAME} at (0,54) size 812x513 [bgcolor=#008000] [border: (10px solid #808000)]
+          layer at (0,0) size 792x493
+            RenderView at (0,0) size 792x493
+          layer at (0,0) size 792x493
+            RenderBlock {HTML} at (0,0) size 792x493
+              RenderBody {BODY} at (8,8) size 776x477
+        RenderBlock {DIV} at (0,567) size 800x18
           RenderText {#text} at (0,0) size 664x18
             text run at (0,0) width 407: "This footer should remain at the bottom of the browser window. "
             text run at (407,0) width 257: "It can wrap to multiple lines if necessary."
index 258a540db352438a222234cb53dcb8f3da9c6d18..e9b4934d0603b0282682c0051ab840e031e84f7a 100644 (file)
@@ -1 +1 @@
-6dc980c12ad5492084a0afeb8c0ca53c
\ No newline at end of file
+1f38bab3a53ab10e50eedeb03b796747
\ No newline at end of file
index 760f5fc2756eaa6a6b9cbc1f29e80713597ab81d..4e62decb506c101e7d992637d12b6ff8efc7833c 100644 (file)
Binary files a/LayoutTests/platform/mac/fast/flexbox/flex-hang-expected.png and b/LayoutTests/platform/mac/fast/flexbox/flex-hang-expected.png differ
index 7f503639180dcc6a86ef7f2c3e6190b6c07bf41a..52010fe38e3bf8b066ddd5dd6b9b8bb957e8d431 100644 (file)
@@ -4,9 +4,9 @@ layer at (0,0) size 788x587
   RenderBlock {HTML} at (0,0) size 785x587
     RenderBody {BODY} at (0,0) size 785x587
       RenderFlexibleBox {DIV} at (0,0) size 787x587 [border: (1px solid #000000)]
-        RenderFlexibleBox {DIV} at (1,1) size 785x380 [border: (1px solid #000000)]
-          RenderFlexibleBox {DIV} at (1,1) size 510x378 [border: (1px solid #000000)]
-          RenderFlexibleBox {DIV} at (511,1) size 276x378 [border: (1px solid #000000)]
-        RenderFlexibleBox {DIV} at (1,381) size 785x205 [border: (1px solid #000000)]
-          RenderFlexibleBox {DIV} at (1,1) size 510x203 [border: (1px solid #000000)]
-          RenderFlexibleBox {DIV} at (511,1) size 276x203 [border: (1px solid #000000)]
+        RenderFlexibleBox {DIV} at (1,1) size 785x379 [border: (1px solid #000000)]
+          RenderFlexibleBox {DIV} at (1,1) size 510x377 [border: (1px solid #000000)]
+          RenderFlexibleBox {DIV} at (511,1) size 276x377 [border: (1px solid #000000)]
+        RenderFlexibleBox {DIV} at (1,380) size 785x206 [border: (1px solid #000000)]
+          RenderFlexibleBox {DIV} at (1,1) size 510x204 [border: (1px solid #000000)]
+          RenderFlexibleBox {DIV} at (511,1) size 276x204 [border: (1px solid #000000)]
index 0c346b6edd0aa70146bfa3324052d740f1f3159d..6aaa37a0f0d6cf65b6726f2bb6ccff2b92a68200 100644 (file)
@@ -1 +1 @@
-4db83380f83e249fa123502288c8037f
\ No newline at end of file
+fa6148a3e615abad86aea15059ac7bd5
\ No newline at end of file
index dcffbc0887cf000079c2d91c296dd71f97f4da2e..0c90bcda5f128b036ec2fac5af1b8228f090f02f 100644 (file)
Binary files a/LayoutTests/platform/mac/fast/lists/001-expected.png and b/LayoutTests/platform/mac/fast/lists/001-expected.png differ
index 43a1e241427a0e37151fcacb15de3b90df535950..bdd5cb8244ed094a8527f4a11738988083f71512 100644 (file)
@@ -1,8 +1,8 @@
-layer at (0,0) size 809x600
-  RenderView at (0,0) size 800x600
-layer at (0,0) size 809x600
-  RenderBlock {HTML} at (0,0) size 800x600
-    RenderBody {BODY} at (8,8) size 784x576
+layer at (0,0) size 809x585
+  RenderView at (0,0) size 800x585
+layer at (0,0) size 809x585
+  RenderBlock {HTML} at (0,0) size 800x585
+    RenderBody {BODY} at (8,8) size 784x561
       RenderBlock {DIV} at (0,0) size 784x106 [border: (3px solid #FFA500)]
         RenderBlock (floating) {DIV} at (11,11) size 122x122 [bgcolor=#FFA500] [border: (3px solid #FFA500)]
         RenderListItem {DIV} at (11,11) size 762x84 [border: (3px solid #FFA500)]
index 38af1fadd5653d3a35f47290b948e22e67135687..5dcc168b438254c26669316a2efe48e40366e46c 100644 (file)
@@ -1,3 +1,34 @@
+2009-04-08  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Adam Roben and Darin Adler
+
+        Fix for https://bugs.webkit.org/show_bug.cgi?id=12440, repaints inconsistent for
+        fixed positioned elements.
+
+        Rewrite the updateScrollers method to be more correct in its results.
+
+        Test: fast/block/positioning/fixed-positioning-scrollbar-bug.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle):
+        (WebCore::Document::implicitClose):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::createScrollbar):
+        (WebCore::FrameView::layout):
+        (WebCore::FrameView::adjustPageHeight):
+        * page/FrameView.h:
+        * page/win/FrameWin.cpp:
+        (WebCore::computePageRectsForFrame):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::ScrollView):
+        (WebCore::ScrollView::updateScrollbars):
+        * platform/ScrollView.h:
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::layout):
+        (WebCore::RenderView::docHeight):
+        (WebCore::RenderView::docWidth):
+        * rendering/RenderView.h:
+
 2009-04-08  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
index 759fc37a2852b8b4906e36640b039e057a32f0ac..a5f2fd2c0afc427f9f8896a6bdcad54b2e26a18b 100644 (file)
@@ -1155,18 +1155,14 @@ void Document::recalcStyle(StyleChange change)
         if (change >= Inherit || n->hasChangedChild() || n->changed())
             n->recalcStyle(change);
 
-    if (view()) {
-        if (changed())
-            view()->layout();
 #if USE(ACCELERATED_COMPOSITING)
-        else {
-            bool layoutPending = view()->layoutPending() || renderer()->needsLayout();
-            // If we didn't update compositing layers because of layout(), we need to do so here.
-            if (!layoutPending)
-                view()->updateCompositingLayers();
-        }
-#endif
+    if (view()) {
+        bool layoutPending = view()->layoutPending() || renderer()->needsLayout();
+        // If we didn't update compositing layers because of layout(), we need to do so here.
+        if (!layoutPending)
+            view()->updateCompositingLayers();
     }
+#endif
 
 bail_out:
     setChanged(NoStyleChange);
@@ -1613,25 +1609,16 @@ void Document::implicitClose()
 
     frame()->loader()->checkCallImplicitClose();
 
-    // Now do our painting/layout, but only if we aren't in a subframe or if we're in a subframe
-    // that has been sized already.  Otherwise, our view size would be incorrect, so doing any 
-    // layout/painting now would be pointless.
+    // We used to force a synchronous display and flush here.  This really isn't
+    // necessary and can in fact be actively harmful if pages are loading at a rate of > 60fps
+    // (if your platform is syncing flushes and limiting them to 60fps).
+    m_overMinimumLayoutThreshold = true;
     if (!ownerElement() || (ownerElement()->renderer() && !ownerElement()->renderer()->needsLayout())) {
         updateRendering();
         
         // Always do a layout after loading if needed.
         if (view() && renderer() && (!renderer()->firstChild() || renderer()->needsLayout()))
             view()->layout();
-            
-        // Paint immediately after the document is ready.  We do this to ensure that any timers set by the
-        // onload don't have a chance to fire before we would have painted.  To avoid over-flushing we only
-        // worry about this for the top-level document.  For platforms that use native widgets for ScrollViews, this
-        // call does nothing (Mac, wx).
-        // FIXME: This causes a timing issue with the dispatchDidFinishLoad delegate callback on Mac, so think
-        // before enabling it, even if Mac becomes viewless later.
-        // See <rdar://problem/5092361>
-        if (view() && !ownerElement())
-            view()->hostWindow()->paint();
     }
 
 #if PLATFORM(MAC)
index fe11532cf95cbd2ea15a0585e13d26b05bb6faa3..bdeb33a0c5b30708c67f9e9a7c3edfc69e4b384b 100644 (file)
@@ -315,12 +315,12 @@ PassRefPtr<Scrollbar> FrameView::createScrollbar(ScrollbarOrientation orientatio
     Document* doc = m_frame->document();
 
     // Try the <body> element first as a scrollbar source.
-    Element* body = doc->body();
+    Element* body = doc ? doc->body() : 0;
     if (body && body->renderer() && body->renderer()->style()->hasPseudoStyle(SCROLLBAR))
         return RenderScrollbar::createCustomScrollbar(this, orientation, body->renderBox());
     
     // If the <body> didn't have a custom style, then the root element might.
-    Element* docElement = doc->documentElement();
+    Element* docElement = doc ? doc->documentElement() : 0;
     if (docElement && docElement->renderer() && docElement->renderer()->style()->hasPseudoStyle(SCROLLBAR))
         return RenderScrollbar::createCustomScrollbar(this, orientation, docElement->renderBox());
         
@@ -1439,7 +1439,7 @@ void FrameView::adjustPageHeight(float *newBottom, float oldTop, float oldBottom
         // Use a context with painting disabled.
         GraphicsContext context((PlatformGraphicsContext*)0);
         root->setTruncatedAt((int)floorf(oldBottom));
-        IntRect dirtyRect(0, (int)floorf(oldTop), root->docWidth(), (int)ceilf(oldBottom - oldTop));
+        IntRect dirtyRect(0, (int)floorf(oldTop), root->overflowWidth(), (int)ceilf(oldBottom - oldTop));
         root->layer()->paint(&context, dirtyRect);
         *newBottom = root->bestTruncatedAt();
         if (*newBottom == 0)
@@ -1448,12 +1448,4 @@ void FrameView::adjustPageHeight(float *newBottom, float oldTop, float oldBottom
         *newBottom = oldBottom;
 }
 
-IntSize FrameView::minimumContentsSize() const
-{
-    RenderView* root = m_frame->contentRenderer();
-    if (!root)
-        return ScrollView::minimumContentsSize();
-    return IntSize(root->docWidth(), root->docHeight());
-}
-
 } // namespace WebCore
index d9896d0bfeecd91148995161ebb04372fbf66dbe..07295d337c937f38a2cacbaa8416491b9dae1b67 100644 (file)
@@ -73,8 +73,6 @@ public:
     void setMarginWidth(int);
     void setMarginHeight(int);
 
-    virtual IntSize minimumContentsSize() const;
-
     virtual void setCanHaveScrollbars(bool);
 
     virtual PassRefPtr<Scrollbar> createScrollbar(ScrollbarOrientation);
index db55d30bb9b6aa1fa3f589c86589d9a07e1bb322..0c1c5b11e46a7d0613f86b64b195ec85ea37d3b9 100644 (file)
@@ -61,7 +61,7 @@ void computePageRectsForFrame(Frame* frame, const IntRect& printRect, float head
     
     float ratio = static_cast<float>(printRect.height()) / static_cast<float>(printRect.width());
  
-    float pageWidth  = static_cast<float>(root->docWidth());
+    float pageWidth  = static_cast<float>(root->overflowWidth());
     float pageHeight = pageWidth * ratio;
     outPageHeight = static_cast<int>(pageHeight);   // this is the height of the page adjusted by margins
     pageHeight -= (headerHeight + footerHeight);
index f24b5e99ddccf9c8286386bb07e1194c2507e273..56859683dea51f870001d4eb3ac5a1437c6c1203 100644 (file)
@@ -45,7 +45,8 @@ ScrollView::ScrollView()
     , m_canBlitOnScroll(true)
     , m_scrollbarsAvoidingResizer(0)
     , m_scrollbarsSuppressed(false)
-    , m_inUpdateScrollbars(false)
+    , m_updatingScrollbarAttributes(false)
+    , m_updateScrollbarsPass(0)
     , m_drawPanScrollIcon(false)
     , m_useFixedLayout(false)
 {
@@ -313,59 +314,78 @@ bool ScrollView::scroll(ScrollDirection direction, ScrollGranularity granularity
     return false;
 }
 
+static const unsigned cMaxUpdateScrollbarsPass = 2;
+
 void ScrollView::updateScrollbars(const IntSize& desiredOffset)
 {
-    // Don't allow re-entrancy into this function.
-    if (m_inUpdateScrollbars || prohibitsScrolling() || platformWidget())
+    if (m_updatingScrollbarAttributes || prohibitsScrolling() || platformWidget())
         return;
 
-    m_inUpdateScrollbars = true;
-
-    bool hasVerticalScrollbar = m_verticalScrollbar;
     bool hasHorizontalScrollbar = m_horizontalScrollbar;
-    bool oldHasVertical = hasVerticalScrollbar;
-    bool oldHasHorizontal = hasHorizontalScrollbar;
+    bool hasVerticalScrollbar = m_verticalScrollbar;
+    
+    bool newHasHorizontalScrollbar = hasHorizontalScrollbar;
+    bool newHasVerticalScrollbar = hasVerticalScrollbar;
+   
     ScrollbarMode hScroll = m_horizontalScrollbarMode;
     ScrollbarMode vScroll = m_verticalScrollbarMode;
 
-    const int scrollbarThickness = ScrollbarTheme::nativeTheme()->scrollbarThickness();
-
-    for (int pass = 0; pass < 2; pass++) {
-        bool scrollsVertically;
-        bool scrollsHorizontally;
-
-        if (!m_scrollbarsSuppressed && (hScroll == ScrollbarAuto || vScroll == ScrollbarAuto)) {
-            // Do a layout if pending before checking if scrollbars are needed.
-            if (hasVerticalScrollbar != oldHasVertical || hasHorizontalScrollbar != oldHasHorizontal)
-                visibleContentsResized();
-
-            IntSize docSize = minimumContentsSize();
-
-            scrollsVertically = (vScroll == ScrollbarAlwaysOn) || (vScroll == ScrollbarAuto && docSize.height() > height());
-            if (scrollsVertically)
-                scrollsHorizontally = (hScroll == ScrollbarAlwaysOn) || (hScroll == ScrollbarAuto && docSize.width() + scrollbarThickness > width());
-            else {
-                scrollsHorizontally = (hScroll == ScrollbarAlwaysOn) || (hScroll == ScrollbarAuto && docSize.width() > width());
-                if (scrollsHorizontally)
-                    scrollsVertically = (vScroll == ScrollbarAlwaysOn) || (vScroll == ScrollbarAuto && docSize.height() + scrollbarThickness > height());
-            }
-        } else {
-            scrollsHorizontally = (hScroll == ScrollbarAuto) ? hasHorizontalScrollbar : (hScroll == ScrollbarAlwaysOn);
-            scrollsVertically = (vScroll == ScrollbarAuto) ? hasVerticalScrollbar : (vScroll == ScrollbarAlwaysOn);
-        }
+    if (m_scrollbarsSuppressed || (hScroll != ScrollbarAuto && vScroll != ScrollbarAuto)) {
+        if (hScroll != ScrollbarAuto)
+            newHasHorizontalScrollbar = (hScroll == ScrollbarAlwaysOn);
+        if (vScroll != ScrollbarAuto)
+            newHasVerticalScrollbar = (vScroll == ScrollbarAlwaysOn);
+        if (hasHorizontalScrollbar != newHasHorizontalScrollbar)
+            setHasHorizontalScrollbar(newHasHorizontalScrollbar);
+        if (hasVerticalScrollbar != newHasVerticalScrollbar)
+            setHasVerticalScrollbar(newHasVerticalScrollbar);
+    } else {
+        bool sendContentResizedNotification = false;
+        
+        IntSize docSize = contentsSize();
         
-        if (hasVerticalScrollbar != scrollsVertically) {
-            setHasVerticalScrollbar(scrollsVertically);
-            hasVerticalScrollbar = scrollsVertically;
+        if (hScroll == ScrollbarAuto)
+            newHasHorizontalScrollbar = docSize.width() > visibleWidth();
+        if (vScroll == ScrollbarAuto)
+            newHasVerticalScrollbar = docSize.height() > visibleHeight();
+        
+        // If we ever turn one scrollbar off, always turn the other one off too.  Never ever
+        // try to both gain/lose a scrollbar in the same pass.
+        if (!newHasHorizontalScrollbar && hasHorizontalScrollbar)
+            newHasVerticalScrollbar = false;
+        if (!newHasVerticalScrollbar && hasVerticalScrollbar)
+            newHasHorizontalScrollbar = false;
+
+        if (hasHorizontalScrollbar != newHasHorizontalScrollbar) {
+            setHasHorizontalScrollbar(newHasHorizontalScrollbar);
+            sendContentResizedNotification = true;
+        }
+        if (hasVerticalScrollbar != newHasVerticalScrollbar) {
+            setHasVerticalScrollbar(newHasVerticalScrollbar);
+            sendContentResizedNotification = true;
         }
 
-        if (hasHorizontalScrollbar != scrollsHorizontally) {
-            setHasHorizontalScrollbar(scrollsHorizontally);
-            hasHorizontalScrollbar = scrollsHorizontally;
+        if (sendContentResizedNotification && m_updateScrollbarsPass < cMaxUpdateScrollbarsPass) {
+            m_updateScrollbarsPass++;
+            visibleContentsResized();
+            IntSize newDocSize = contentsSize();
+            if (newDocSize != docSize) {
+                // The layout with the new scroll state had no impact on
+                // the document's overall size, so updateScrollbars didn't get called.
+                // Recur manually.
+                updateScrollbars(desiredOffset);
+            }
+            m_updateScrollbarsPass--;
         }
     }
     
-    // Set up the range (and page step/line step).
+    // Set up the range (and page step/line step), but only do this if we're not in a nested call (to avoid
+    // doing it multiple times).
+    if (m_updateScrollbarsPass)
+        return;
+
+    m_updatingScrollbarAttributes = true;
     IntSize maxScrollPosition(contentsWidth() - visibleWidth(), contentsHeight() - visibleHeight());
     IntSize scroll = desiredOffset.shrunkTo(maxScrollPosition);
     scroll.clampNegativeToZero();
@@ -418,7 +438,7 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
             m_verticalScrollbar->setSuppressInvalidation(false);
     }
 
-    if (oldHasVertical != (m_verticalScrollbar != 0) || oldHasHorizontal != (m_horizontalScrollbar != 0))
+    if (hasHorizontalScrollbar != (m_horizontalScrollbar != 0) || hasVerticalScrollbar != (m_verticalScrollbar != 0))
         frameRectsChanged();
 
     // See if our offset has changed in a situation where we might not have scrollbars.
@@ -431,7 +451,7 @@ void ScrollView::updateScrollbars(const IntSize& desiredOffset)
        scrollContents(scrollDelta);
     }
 
-    m_inUpdateScrollbars = false;
+    m_updatingScrollbarAttributes = false;
 }
 
 const int panIconSizeLength = 20;
index 9e4d3b17d0e91076b620ac2a2ea2e8e82277228d..17b3160228ce8e3f5bad1e6ece3a9772f6265c2a 100644 (file)
@@ -126,7 +126,6 @@ public:
     // Methods for getting/setting the size of the document contained inside the ScrollView (as an IntSize or as individual width and height
     // values).
     IntSize contentsSize() const; // Always at least as big as the visibleWidth()/visibleHeight().
-    virtual IntSize minimumContentsSize() const { return contentsSize(); } // Allows a subclass to indicate a smaller size than the viewport (used for scrollbar checking).
     int contentsWidth() const { return contentsSize().width(); }
     int contentsHeight() const { return contentsSize().height(); }
     virtual void setContentsSize(const IntSize&);
@@ -254,7 +253,8 @@ private:
     int m_scrollbarsAvoidingResizer;
     bool m_scrollbarsSuppressed;
 
-    bool m_inUpdateScrollbars;
+    bool m_updatingScrollbarAttributes;
+    unsigned m_updateScrollbarsPass;
 
     IntPoint m_panScrollIconPoint;
     bool m_drawPanScrollIcon;
index d11e74491edfaf918621fa6854d32056bf4e24c5..53dd8d845864521f3a563103411a63956de76eb6 100644 (file)
@@ -121,11 +121,12 @@ void RenderView::layout()
     if (needsLayout())
         RenderBlock::layout();
 
-    m_docWidth = calcDocWidth();
-    m_docHeight = calcDocHeight();
-
-    setOverflowWidth(max(width(), m_docWidth));
-    setOverflowHeight(max(height(), m_docHeight));
+    // Reset overflowWidth and overflowHeight, since they act as a lower bound for docWidth() and docHeight().
+    setOverflowWidth(width());
+    setOverflowHeight(height());
+    
+    setOverflowWidth(docWidth());
+    setOverflowHeight(docHeight());
 
     ASSERT(layoutDelta() == IntSize());
     ASSERT(m_layoutStateDisableCount == 0);
@@ -549,12 +550,9 @@ IntRect RenderView::viewRect() const
     return IntRect();
 }
 
-int RenderView::calcDocHeight() const
+int RenderView::docHeight() const
 {
-    // Exclude our own height, since it always matches the viewport height.  We want to know the height
-    // of the document if it is potentially smaller than the viewport (this value will be used when trying
-    // to figure out if scrollbars are needed).
-    int h = max(0, lowestPosition(true, false));
+    int h = lowestPosition();
 
     // FIXME: This doesn't do any margin collapsing.
     // Instead of this dh computation we should keep the result
@@ -569,12 +567,9 @@ int RenderView::calcDocHeight() const
     return h;
 }
 
-int RenderView::calcDocWidth() const
+int RenderView::docWidth() const
 {
-    // Exclude our own width, since it always matches the viewport width.  We want to know the width
-    // of the document if it is potentially smaller than the viewport (this value will be used when trying
-    // to figure out if scrollbars are needed).
-    int w = max(0, rightmostPosition(true, false));
+    int w = rightmostPosition();
 
     for (RenderBox* c = firstChildBox(); c; c = c->nextSiblingBox()) {
         int dw = c->width() + c->marginLeft() + c->marginRight();
index 65913575feeb8beff8e1dd8fee2e7b31d663b7b3..750b4de7c1e50c2fa47d94e77065b32b9afa3b8e 100644 (file)
@@ -54,9 +54,6 @@ public:
     // The same as the FrameView's layoutHeight/layoutWidth but with null check guards.
     int viewHeight() const;
     int viewWidth() const;
-    
-    int docWidth() const { return m_docWidth; }
-    int docHeight() const { return m_docHeight; }
 
     float zoomFactor() const;
 
@@ -172,8 +169,8 @@ protected:
 private:
     bool shouldRepaint(const IntRect& r) const;
         
-    int calcDocHeight() const;
-    int calcDocWidth() const;
+    int docHeight() const;
+    int docWidth() const;
 
 protected:
     FrameView* m_frameView;
@@ -195,9 +192,6 @@ protected:
     RenderWidgetSet m_widgets;
 
 private:
-    int m_docWidth;
-    int m_docHeight;
-
     int m_bestTruncatedAt;
     int m_truncatorWidth;
     bool m_forcedPageBreak;
index b07c2fba1b315d259dcdee680bf5b30b49d71f0c..643580d52c467d2b1e0a496fc335667f6ca33b92 100644 (file)
@@ -1,3 +1,14 @@
+2009-04-08  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Adam Roben and Darin Adler
+
+        Fix for https://bugs.webkit.org/show_bug.cgi?id=12440, fixed positioned elements end up in
+        inconsistent positions.  Rewrite updateScrollers to improve the correctness.
+
+        * WebView/WebDynamicScrollBarsView.h:
+        * WebView/WebDynamicScrollBarsView.m:
+        (-[WebDynamicScrollBarsView updateScrollers]):
+
 2009-04-07  Anders Carlsson  <andersca@apple.com>
 
         Fix Tiger build for real this time.
index a0f800616dfbc47f9b9aa3b48a9e7d666110009e..ce92b337e728717b241bbb99de9043410c72398a 100644 (file)
@@ -43,6 +43,7 @@ extern const int WebCoreScrollbarAlwaysOn;
     BOOL suppressLayout;
     BOOL suppressScrollers;
     BOOL inUpdateScrollers;
+    unsigned inUpdateScrollersLayoutPass;
 }
 - (void)setAllowsHorizontalScrolling:(BOOL)flag; // This method is used by Safari, so it cannot be removed.
 @end
index a988d846b2b69be0dd007bb728a0b6db0606c9fd..e4b65a8ee28d186d9c6929a12793d6bac15ab724 100644 (file)
@@ -85,88 +85,87 @@ const int WebCoreScrollbarAlwaysOn = ScrollbarAlwaysOn;
 #endif
 }
 
+static const unsigned cMaxUpdateScrollbarsPass = 2;
+
 - (void)updateScrollers
 {
-    // We need to do the work below twice in the case where a scroll bar disappears,
-    // making the second layout have a wider width than the first. Doing it more than
-    // twice would indicate some kind of infinite loop, so we do it at most twice.
-    // It's quite efficient to do this work twice in the normal case, so we don't bother
-    // trying to figure out of the second pass is needed or not.
-    if (inUpdateScrollers)
-        return;
-    
-    inUpdateScrollers = true;
-
-    int pass;
     BOOL hasVerticalScroller = [self hasVerticalScroller];
     BOOL hasHorizontalScroller = [self hasHorizontalScroller];
-    BOOL oldHasVertical = hasVerticalScroller;
-    BOOL oldHasHorizontal = hasHorizontalScroller;
-
-    for (pass = 0; pass < 2; pass++) {
-        BOOL scrollsVertically;
-        BOOL scrollsHorizontally;
-
-        if (!suppressLayout && !suppressScrollers && (hScroll == ScrollbarAuto || vScroll == ScrollbarAuto)) {
-            // Do a layout if pending, before checking if scrollbars are needed.
-            // This fixes 2969367, although may introduce a slowdown in live resize performance.
-            NSView *documentView = [self documentView];
-            if (!documentView) {
-                scrollsHorizontally = NO;
-                scrollsVertically = NO;
-            } else {
-                if ((hasVerticalScroller != oldHasVertical ||
-                    hasHorizontalScroller != oldHasHorizontal || [documentView inLiveResize]) && [documentView conformsToProtocol:@protocol(WebDocumentView)]) {
-                    [(id <WebDocumentView>)documentView setNeedsLayout: YES];
-                    [(id <WebDocumentView>)documentView layout];
-                }
-
-                NSSize documentSize = [documentView frame].size;
-                if ([documentView isKindOfClass:[WebHTMLView class]]) {
-                    WebHTMLView *htmlView = (WebHTMLView*)documentView;
-                    if (Frame* coreFrame = core([htmlView _frame])) {
-                        if (FrameView* coreView = coreFrame->view())
-                            documentSize = coreView->minimumContentsSize();
-                    }
-                }
-
-                NSSize frameSize = [self frame].size;
-
-                scrollsVertically = (vScroll == ScrollbarAlwaysOn) ||
-                    (vScroll == ScrollbarAuto && documentSize.height > frameSize.height);
-                if (scrollsVertically)
-                    scrollsHorizontally = (hScroll == ScrollbarAlwaysOn) ||
-                        (hScroll == ScrollbarAuto && documentSize.width + [NSScroller scrollerWidth] > frameSize.width);
-                else {
-                    scrollsHorizontally = (hScroll == ScrollbarAlwaysOn) ||
-                        (hScroll == ScrollbarAuto && documentSize.width > frameSize.width);
-                    if (scrollsHorizontally)
-                        scrollsVertically = (vScroll == ScrollbarAlwaysOn) ||
-                            (vScroll == ScrollbarAuto && documentSize.height + [NSScroller scrollerWidth] > frameSize.height);
-                }
-            }
-        } else {
-            scrollsHorizontally = (hScroll == ScrollbarAuto) ? hasHorizontalScroller : (hScroll == ScrollbarAlwaysOn);
-            scrollsVertically = (vScroll == ScrollbarAuto) ? hasVerticalScroller : (vScroll == ScrollbarAlwaysOn);
-        }
+    
+    BOOL newHasHorizontalScroller = hasHorizontalScroller;
+    BOOL newHasVerticalScroller = hasVerticalScroller;
+    
+    BOOL needsLayout = NO;
 
-        if (hasVerticalScroller != scrollsVertically) {
-            [self setHasVerticalScroller:scrollsVertically];
-            hasVerticalScroller = scrollsVertically;
+    NSView *documentView = [self documentView];
+    if (!documentView) {
+        newHasHorizontalScroller = NO;
+        newHasVerticalScroller = NO;
+    } 
+    
+    if (!documentView || suppressLayout || suppressScrollers || (hScroll != ScrollbarAuto && vScroll != ScrollbarAuto)) {
+        inUpdateScrollers = YES;
+        if (hScroll != ScrollbarAuto)
+            newHasHorizontalScroller = (hScroll == ScrollbarAlwaysOn);
+        if (vScroll != ScrollbarAuto)
+            newHasVerticalScroller = (vScroll == ScrollbarAlwaysOn);
+        if (hasHorizontalScroller != newHasHorizontalScroller)
+            [self setHasHorizontalScroller:newHasHorizontalScroller];
+        if (hasVerticalScroller != newHasVerticalScroller)
+            [self setHasVerticalScroller:newHasVerticalScroller];
+        if (suppressScrollers) {
+            [[self verticalScroller] setNeedsDisplay:NO];
+            [[self horizontalScroller] setNeedsDisplay:NO];
         }
+        inUpdateScrollers = NO;
+        return;
+    }
 
-        if (hasHorizontalScroller != scrollsHorizontally) {
-            [self setHasHorizontalScroller:scrollsHorizontally];
-            hasHorizontalScroller = scrollsHorizontally;
-        }
+    needsLayout = NO;
+
+    NSSize documentSize = [documentView frame].size;
+    NSSize visibleSize = [self documentVisibleRect].size;
+    
+    if (hScroll == ScrollbarAuto)
+        newHasHorizontalScroller = documentSize.width > visibleSize.width;
+    if (vScroll == ScrollbarAuto)
+        newHasVerticalScroller = documentSize.height > visibleSize.height;
+    
+    // If we ever turn one scrollbar off, always turn the other one off too.  Never ever
+    // try to both gain/lose a scrollbar in the same pass.
+    if (!newHasHorizontalScroller && hasHorizontalScroller)
+        newHasVerticalScroller = NO;
+    if (!newHasVerticalScroller && hasVerticalScroller)
+        newHasHorizontalScroller = NO;
+
+    if (hasHorizontalScroller != newHasHorizontalScroller) {
+        inUpdateScrollers = YES;
+        [self setHasHorizontalScroller:newHasHorizontalScroller];
+        inUpdateScrollers = NO;
+        needsLayout = YES;
     }
 
-    if (suppressScrollers) {
-        [[self verticalScroller] setNeedsDisplay: NO];
-        [[self horizontalScroller] setNeedsDisplay: NO];
+    if (hasVerticalScroller != newHasVerticalScroller) {
+        inUpdateScrollers = YES;
+        [self setHasVerticalScroller:newHasVerticalScroller];
+        inUpdateScrollers = NO;
+        needsLayout = YES;
     }
 
-    inUpdateScrollers = false;
+    if (needsLayout && inUpdateScrollersLayoutPass < cMaxUpdateScrollbarsPass && 
+        [documentView conformsToProtocol:@protocol(WebDocumentView)]) {
+        inUpdateScrollersLayoutPass++;
+        [(id <WebDocumentView>)documentView setNeedsLayout: YES];
+        [(id <WebDocumentView>)documentView layout];
+        NSSize newDocumentSize = [documentView frame].size;
+        if (NSEqualSizes(documentSize, newDocumentSize)) {
+            // The layout with the new scroll state had no impact on
+            // the document's overall size, so updateScrollers didn't get called.
+            // Recur manually.
+            [self updateScrollers];
+        }
+        inUpdateScrollersLayoutPass--;
+    }
 }
 
 // Make the horizontal and vertical scroll bars come and go as needed.