Background doesn't fully repaint when body has margins.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 4 Aug 2013 21:29:00 +0000 (21:29 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 4 Aug 2013 21:29:00 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=119033

Reviewed by Simon Fraser.

Ensure that background-color changes do not leave unpainted areas when
body has margins.

Both <body> and <html> background-color get propagated up to the viewport.
If <body> has background-color attribute set, while <html> doesn't, the color is
applied not only on the <body> but on both the <html> and the viewport. However,
it's not enough to mark the RenderView dirty because with tiles backing on,
there could be areas outside of the viewport that need repaint. By marking
the RenderView's graphics layer dirty instead, we ensure that all the related
tiles get marked dirty too and the new background color covers all areas.

Manual test added. When forcing top-level composition on (even with embedded iframe to
make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
that the repaint rects don't reflect the functionality difference anymore.

.:

Reviewed by Simon Fraser.

* ManualTests/compositing/background-color-change-on-body-with-margin.html: Added.

Source/WebCore:

* page/FrameView.cpp:
(WebCore::FrameView::reset):
(WebCore::FrameView::layout):
* page/FrameView.h:
(WebCore::FrameView::needsFullRepaint):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::styleWillChange):
* rendering/RenderObjectChildList.cpp:
(WebCore::RenderObjectChildList::removeChildNode):
* rendering/RenderView.cpp:
(WebCore::RenderView::repaintRootContents):
(WebCore::RenderView::repaintViewAndCompositedLayers):
* rendering/RenderView.h:

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

ChangeLog
ManualTests/compositing/background-color-change-on-body-with-margin.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderObjectChildList.cpp
Source/WebCore/rendering/RenderView.cpp
Source/WebCore/rendering/RenderView.h

index 243cff8..7cda9fe 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,29 @@
+2013-08-04  Zalan Bujtas  <zalan@apple.com>
+
+        Background doesn't fully repaint when body has margins.
+        https://bugs.webkit.org/show_bug.cgi?id=119033
+
+        Reviewed by Simon Fraser.
+
+        Ensure that background-color changes do not leave unpainted areas when
+        body has margins.
+
+        Both <body> and <html> background-color get propagated up to the viewport.
+        If <body> has background-color attribute set, while <html> doesn't, the color is
+        applied not only on the <body> but on both the <html> and the viewport. However,
+        it's not enough to mark the RenderView dirty because with tiles backing on,
+        there could be areas outside of the viewport that need repaint. By marking
+        the RenderView's graphics layer dirty instead, we ensure that all the related
+        tiles get marked dirty too and the new background color covers all areas.
+
+        Manual test added. When forcing top-level composition on (even with embedded iframe to
+        make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
+        that the repaint rects don't reflect the functionality difference anymore.
+
+        Reviewed by Simon Fraser.
+
+        * ManualTests/compositing/background-color-change-on-body-with-margin.html: Added.
+
 2013-07-30  Ádám Kallai  <kadam@inf.u-szeged.hu>
 
         [Qt] Workaround to make syncqt run and generate forwarding headers in SVN repositories too.
diff --git a/ManualTests/compositing/background-color-change-on-body-with-margin.html b/ManualTests/compositing/background-color-change-on-body-with-margin.html
new file mode 100644 (file)
index 0000000..03cbcb6
--- /dev/null
@@ -0,0 +1,116 @@
+<html>
+<head>
+
+<style>
+body {
+  margin: 100px 100px 100px 100px;
+  background-color: red;
+}
+</style>
+
+<script>
+  function start() {
+    window.setTimeout("document.getElementsByTagName('body')[0].style.backgroundColor='white';", 500); 
+  }
+</script>
+</head>
+<body onload='start();'>
+  <p>This test verifies that we don't do partial repaint when background-color changes.<br>
+     Scroll to check if the background is white all the way down.</p>
+  <div id='content'>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+  </div>
+</body>
+</html>
index 09fc113..a278e01 100644 (file)
@@ -1,3 +1,39 @@
+2013-08-04  Zalan Bujtas  <zalan@apple.com>
+
+        Background doesn't fully repaint when body has margins.
+        https://bugs.webkit.org/show_bug.cgi?id=119033
+
+        Reviewed by Simon Fraser.
+
+        Ensure that background-color changes do not leave unpainted areas when
+        body has margins.
+
+        Both <body> and <html> background-color get propagated up to the viewport.
+        If <body> has background-color attribute set, while <html> doesn't, the color is
+        applied not only on the <body> but on both the <html> and the viewport. However,
+        it's not enough to mark the RenderView dirty because with tiles backing on,
+        there could be areas outside of the viewport that need repaint. By marking
+        the RenderView's graphics layer dirty instead, we ensure that all the related
+        tiles get marked dirty too and the new background color covers all areas.
+
+        Manual test added. When forcing top-level composition on (even with embedded iframe to
+        make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
+        that the repaint rects don't reflect the functionality difference anymore.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::reset):
+        (WebCore::FrameView::layout):
+        * page/FrameView.h:
+        (WebCore::FrameView::needsFullRepaint):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::styleWillChange):
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::RenderObjectChildList::removeChildNode):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::repaintRootContents):
+        (WebCore::RenderView::repaintViewAndCompositedLayers):
+        * rendering/RenderView.h:
+
 2013-08-04  Andreas Kling  <akling@apple.com>
 
         Document needn't expose its active element.
index 149eeec..370d766 100644 (file)
@@ -274,7 +274,7 @@ void FrameView::reset()
     m_layoutTimer.stop();
     m_layoutRoot = 0;
     m_delayedLayout = false;
-    m_doFullRepaint = true;
+    m_needsFullRepaint = true;
     m_layoutSchedulingEnabled = true;
     m_inLayout = false;
     m_doingPreLayoutStyleUpdate = false;
@@ -1259,7 +1259,7 @@ void FrameView::layout(bool allowSubtree)
         ScrollbarMode vMode;    
         calculateScrollbarModesForLayout(hMode, vMode);
 
-        m_doFullRepaint = !subtree && (m_firstLayout || toRenderView(root)->printing());
+        m_needsFullRepaint = !subtree && (m_firstLayout || toRenderView(root)->printing());
 
         if (!subtree) {
             // Now set our scrollbar state for the layout.
@@ -1296,7 +1296,7 @@ void FrameView::layout(bool allowSubtree)
             m_size = LayoutSize(layoutWidth(), layoutHeight());
 
             if (oldSize != m_size) {
-                m_doFullRepaint = true;
+                m_needsFullRepaint = true;
                 if (!m_firstLayout) {
                     RenderBox* rootRenderer = document->documentElement() ? document->documentElement()->renderBox() : 0;
                     RenderBox* bodyRenderer = rootRenderer && document->body() ? document->body()->renderBox() : 0;
@@ -1339,20 +1339,19 @@ void FrameView::layout(bool allowSubtree)
         m_layoutRoot = 0;
     } // Reset m_layoutSchedulingEnabled to its previous value.
 
-    bool neededFullRepaint = m_doFullRepaint;
+    bool neededFullRepaint = m_needsFullRepaint;
 
     if (!subtree && !toRenderView(root)->printing())
         adjustViewSize();
 
-    m_doFullRepaint = neededFullRepaint;
+    m_needsFullRepaint = neededFullRepaint;
 
     // Now update the positions of all layers.
     beginDeferredRepaints();
-    if (m_doFullRepaint)
-        root->view()->repaint(); // FIXME: This isn't really right, since the RenderView doesn't fully encompass the visibleContentRect(). It just happens
-                                 // to work out most of the time, since first layouts and printing don't have you scrolled anywhere.
+    if (m_needsFullRepaint)
+        root->view()->repaintRootContents();
 
-    layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_doFullRepaint));
+    layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_needsFullRepaint));
 
     endDeferredRepaints();
 
index 6040c9b..0acc389 100644 (file)
@@ -115,7 +115,7 @@ public:
     void setNeedsLayout();
     void setViewportConstrainedObjectsNeedLayout();
 
-    bool needsFullRepaint() const { return m_doFullRepaint; }
+    bool needsFullRepaint() const { return m_needsFullRepaint; }
 
 #if ENABLE(REQUEST_ANIMATION_FRAME)
     void serviceScriptedAnimations(double monotonicAnimationStartTime);
@@ -555,7 +555,7 @@ private:
 
     OwnPtr<RenderObjectSet> m_slowRepaintObjects;
 
-    bool m_doFullRepaint;
+    bool m_needsFullRepaint;
     
     bool m_canHaveScrollbars;
     bool m_cannotBlitToWindow;
index 325e39c..3430c72 100644 (file)
@@ -206,11 +206,9 @@ void RenderBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyl
     RenderStyle* oldStyle = style();
     if (oldStyle) {
         // The background of the root element or the body element could propagate up to
-        // the canvas.  Just dirty the entire canvas when our style changes substantially.
-        if (diff >= StyleDifferenceRepaint && node() &&
-            (node()->hasTagName(htmlTag) || node()->hasTagName(bodyTag))) {
-            view()->repaint();
-            
+        // the canvas. Issue full repaint, when our style changes substantially.
+        if (diff >= StyleDifferenceRepaint && (isRoot() || isBody())) {
+            view()->repaintRootContents();
 #if USE(ACCELERATED_COMPOSITING)
             if (oldStyle->hasEntirelyFixedBackground() != newStyle->hasEntirelyFixedBackground())
                 view()->compositor()->rootFixedBackgroundsChanged();
@@ -229,7 +227,7 @@ void RenderBox::styleWillChange(StyleDifference diff, const RenderStyle* newStyl
                 removeFloatingOrPositionedChildFromBlockLists();
         }
     } else if (newStyle && isBody())
-        view()->repaint();
+        view()->repaintRootContents();
 
     RenderBoxModelObject::styleWillChange(diff, newStyle);
 }
index 9445df8..2068ed1 100644 (file)
@@ -67,7 +67,7 @@ RenderObject* RenderObjectChildList::removeChildNode(RenderObject* owner, Render
         oldChild->setNeedsLayoutAndPrefWidthsRecalc();
         // We only repaint |oldChild| if we have a RenderLayer as its visual overflow may not be tracked by its parent.
         if (oldChild->isBody())
-            owner->view()->repaint();
+            owner->view()->repaintRootContents();
         else
             oldChild->repaint();
     }
index 251659f..b291227 100644 (file)
@@ -526,6 +526,17 @@ bool RenderView::shouldRepaint(const LayoutRect& r) const
     return true;
 }
 
+void RenderView::repaintRootContents()
+{
+#if USE(ACCELERATED_COMPOSITING)
+    if (layer()->isComposited()) {
+        layer()->setBackingNeedsRepaint();
+        return;
+    }
+#endif
+    repaint();
+}
+
 void RenderView::repaintViewRectangle(const LayoutRect& ur, bool immediate) const
 {
     if (!shouldRepaint(ur))
@@ -567,8 +578,7 @@ void RenderView::repaintRectangleInViewAndCompositedLayers(const LayoutRect& ur,
 
 void RenderView::repaintViewAndCompositedLayers()
 {
-    repaint();
-    
+    repaintRootContents();
 #if USE(ACCELERATED_COMPOSITING)
     if (compositor()->inCompositingMode())
         compositor()->repaintCompositedLayers();
index 920498e..bcbae7c 100644 (file)
@@ -76,6 +76,7 @@ public:
 
     virtual LayoutRect visualOverflowRect() const OVERRIDE;
     virtual void computeRectForRepaint(const RenderLayerModelObject* repaintContainer, LayoutRect&, bool fixed = false) const OVERRIDE;
+    void repaintRootContents();
     void repaintViewRectangle(const LayoutRect&, bool immediate = false) const;
     // Repaint the view, and all composited layers that intersect the given absolute rectangle.
     // FIXME: ideally we'd never have to do this, if all repaints are container-relative.