Fix partial painting with render pipeline throttling

This patch fixes a problem with throttled FrameViews sometimes appearing partially painted.
This was because we generally paint some distance beyond the viewport, i.e., covering the
entire interest rect. FrameViews which are inside the interest rect but outside the viewport
are skipped during painting, so the recorded display list won't include their contents. If
the FrameView is then scrolled on-screen without causing any other paint invalidations, the
display list won't get updated and the FrameView contents will not be shown.

This patch fixes the problem by forcing a repaint of FrameViews when they become
unthrottled, discarding any previous display lists and tile textures for the area they
cover.

BUG=562343

Review URL: https://codereview.chromium.org/1603983002

Cr-Commit-Position: refs/heads/master@{#371517}

4 files changed, 78 insertions, 15 deletions

diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index f94c98e..a6304a7 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -3727,16 +3727,11 @@ void FrameView::paint(GraphicsContext& context, const CullRect& cullRect) const
 
 void FrameView::paint(GraphicsContext& context, const GlobalPaintFlags globalPaintFlags, const CullRect& cullRect) const
 {
-    // TODO(skyostil): Remove this early-out in favor of painting cached scrollbars.
-    if (shouldThrottleRendering())
-        return;
     FramePainter(*this).paint(context, globalPaintFlags, cullRect);
 }
 
 void FrameView::paintContents(GraphicsContext& context, const GlobalPaintFlags globalPaintFlags, const IntRect& damageRect) const
 {
-    if (shouldThrottleRendering())
-        return;
     FramePainter(*this).paintContents(context, globalPaintFlags, damageRect);
 }
 
@@ -4032,8 +4027,15 @@ void FrameView::notifyRenderThrottlingObservers()
     }
 
     bool becameUnthrottled = wasThrottled && !canThrottleRendering();
-    if (becameUnthrottled)
+    if (becameUnthrottled) {
+        // Start ticking animation frames again if necessary.
         page()->animator().scheduleVisualUpdate(m_frame.get());
+        // Force a full repaint of this frame to ensure we are not left with a
+        // partially painted version of this frame's contents if we skipped
+        // painting them while the frame was throttled.
+        if (LayoutView* layoutView = this->layoutView())
+            layoutView->setShouldDoFullPaintInvalidation(PaintInvalidationBecameVisible);
+    }
 }
 
 bool FrameView::shouldThrottleRendering() const
diff --git a/third_party/WebKit/Source/core/paint/FramePainter.cpp b/third_party/WebKit/Source/core/paint/FramePainter.cpp
index a6eb08c..ba89543 100644
--- a/third_party/WebKit/Source/core/paint/FramePainter.cpp
+++ b/third_party/WebKit/Source/core/paint/FramePainter.cpp
@@ -111,8 +111,10 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag
         return;
     }
 
-    RELEASE_ASSERT(!frameView().needsLayout());
-    ASSERT(document->lifecycle().state() >= DocumentLifecycle::CompositingClean);
+    if (!frameView().shouldThrottleRendering()) {
+        RELEASE_ASSERT(!frameView().needsLayout());
+        ASSERT(document->lifecycle().state() >= DocumentLifecycle::CompositingClean);
+    }
 
     TRACE_EVENT1("devtools.timeline", "Paint", "data", InspectorPaintEvent::data(layoutView, LayoutRect(rect), 0));
 
@@ -133,7 +135,8 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag
     PaintLayer* rootLayer = layoutView->layer();
 
 #if ENABLE(ASSERT)
-    layoutView->assertSubtreeIsLaidOut();
+    if (!frameView().shouldThrottleRendering())
+        layoutView->assertSubtreeIsLaidOut();
     LayoutObject::SetLayoutNeededForbiddenScope forbidSetNeedsLayout(*rootLayer->layoutObject());
 #endif
 
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp b/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
index 31bbc5c..4eea6f7 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
@@ -4,6 +4,7 @@
 
 #include "core/paint/PaintLayerPainter.h"
 
+#include "core/frame/FrameView.h"
 #include "core/frame/Settings.h"
 #include "core/layout/ClipPathOperation.h"
 #include "core/layout/LayoutBlock.h"
@@ -80,8 +81,7 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayer(GraphicsContext& co
     if (shouldSuppressPaintingLayer(&m_paintLayer))
         return FullyPainted;
 
-    // TODO(skyostil): Unify this early-out logic with subsequence caching.
-    if (m_paintLayer.layoutObject()->isLayoutPart() && toLayoutPart(m_paintLayer.layoutObject())->isThrottledFrameView())
+    if (m_paintLayer.layoutObject()->isLayoutView() && toLayoutView(m_paintLayer.layoutObject())->frameView()->shouldThrottleRendering())
         return FullyPainted;
 
     // If this layer is totally invisible then there is nothing to paint.
@@ -270,9 +270,8 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayerContents(GraphicsCon
     if (paintFlags & PaintLayerPaintingRootBackgroundOnly && !m_paintLayer.layoutObject()->isLayoutView() && !m_paintLayer.layoutObject()->isDocumentElement())
         return result;
 
-    // TODO(skyostil): Unify this early-out logic with subsequence caching.
-    if (m_paintLayer.layoutObject()->isLayoutPart() && toLayoutPart(m_paintLayer.layoutObject())->isThrottledFrameView())
-        return FullyPainted;
+    if (m_paintLayer.layoutObject()->isLayoutView() && toLayoutView(m_paintLayer.layoutObject())->frameView()->shouldThrottleRendering())
+        return result;
 
     // Ensure our lists are up-to-date.
     m_paintLayer.stackingNode()->updateLayerListsIfNeeded();
diff --git a/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp b/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
index beca4dd..3abba53 100644
--- a/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
+++ b/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
@@ -211,7 +211,7 @@ TEST_F(FrameThrottlingTest, MutatingThrottledFrameDoesNotCauseAnimation)
     frameElement->contentDocument()->documentElement()->setAttribute(styleAttr, "background: green");
     EXPECT_FALSE(compositor().needsAnimate());
 
-    // Moving the frame back on screen to unthrottle it.
+    // Move the frame back on screen to unthrottle it.
     frameElement->setAttribute(styleAttr, "");
     EXPECT_TRUE(compositor().needsAnimate());
 
@@ -251,6 +251,65 @@ TEST_F(FrameThrottlingTest, SynchronousLayoutInThrottledFrame)
     EXPECT_EQ(50, divElement->clientWidth());
 }
 
+TEST_F(FrameThrottlingTest, UnthrottlingTriggersRepaint)
+{
+    // Create a hidden frame which is throttled.
+    SimRequest mainResource("https://example.com/", "text/html");
+    SimRequest frameResource("https://example.com/iframe.html", "text/html");
+
+    loadURL("https://example.com/");
+    mainResource.complete("<iframe id=frame sandbox src=iframe.html></iframe>");
+    frameResource.complete("<style> html { background: green; } </style>");
+
+    // Move the frame offscreen to throttle it.
+    auto* frameElement = toHTMLIFrameElement(document().getElementById("frame"));
+    frameElement->setAttribute(styleAttr, "transform: translateY(480px)");
+    EXPECT_FALSE(frameElement->contentDocument()->view()->shouldThrottleRendering());
+    compositeFrame();
+    EXPECT_TRUE(frameElement->contentDocument()->view()->shouldThrottleRendering());
+
+    // Scroll down to unthrottle the frame. The first frame we composite after
+    // scrolling won't contain the frame yet, but will schedule another repaint.
+    webView().mainFrameImpl()->frameView()->setScrollPosition(DoublePoint(0, 480), ProgrammaticScroll);
+    auto displayItems = compositeFrame();
+    EXPECT_FALSE(displayItems.contains(SimCanvas::Rect, "green"));
+
+    // Now the frame contents should be visible again.
+    auto displayItems2 = compositeFrame();
+    EXPECT_TRUE(displayItems2.contains(SimCanvas::Rect, "green"));
+}
+
+TEST_F(FrameThrottlingTest, ChangeStyleInThrottledFrame)
+{
+    // Create a hidden frame which is throttled.
+    SimRequest mainResource("https://example.com/", "text/html");
+    SimRequest frameResource("https://example.com/iframe.html", "text/html");
+
+    loadURL("https://example.com/");
+    mainResource.complete("<iframe id=frame sandbox src=iframe.html></iframe>");
+    frameResource.complete("<style> html { background: red; } </style>");
+
+    // Move the frame offscreen to throttle it.
+    auto* frameElement = toHTMLIFrameElement(document().getElementById("frame"));
+    frameElement->setAttribute(styleAttr, "transform: translateY(480px)");
+    EXPECT_FALSE(frameElement->contentDocument()->view()->shouldThrottleRendering());
+    compositeFrame();
+    EXPECT_TRUE(frameElement->contentDocument()->view()->shouldThrottleRendering());
+
+    // Change the background color of the frame's contents from red to green.
+    frameElement->contentDocument()->body()->setAttribute(styleAttr, "background: green");
+
+    // Scroll down to unthrottle the frame.
+    webView().mainFrameImpl()->frameView()->setScrollPosition(DoublePoint(0, 480), ProgrammaticScroll);
+    auto displayItems = compositeFrame();
+    EXPECT_FALSE(displayItems.contains(SimCanvas::Rect, "red"));
+    EXPECT_FALSE(displayItems.contains(SimCanvas::Rect, "green"));
+
+    // Make sure the new style shows up instead of the old one.
+    auto displayItems2 = compositeFrame();
+    EXPECT_TRUE(displayItems2.contains(SimCanvas::Rect, "green"));
+}
+
 TEST(RemoteFrameThrottlingTest, ThrottledLocalRoot)
 {
     FrameTestHelpers::TestWebViewClient viewClient;