Commit b4cb32a7 authored by skyostil's avatar skyostil Committed by Commit bot

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}
parent 057026e9
...@@ -3727,16 +3727,11 @@ void FrameView::paint(GraphicsContext& context, const CullRect& cullRect) const ...@@ -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 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); FramePainter(*this).paint(context, globalPaintFlags, cullRect);
} }
void FrameView::paintContents(GraphicsContext& context, const GlobalPaintFlags globalPaintFlags, const IntRect& damageRect) const void FrameView::paintContents(GraphicsContext& context, const GlobalPaintFlags globalPaintFlags, const IntRect& damageRect) const
{ {
if (shouldThrottleRendering())
return;
FramePainter(*this).paintContents(context, globalPaintFlags, damageRect); FramePainter(*this).paintContents(context, globalPaintFlags, damageRect);
} }
...@@ -4032,8 +4027,15 @@ void FrameView::notifyRenderThrottlingObservers() ...@@ -4032,8 +4027,15 @@ void FrameView::notifyRenderThrottlingObservers()
} }
bool becameUnthrottled = wasThrottled && !canThrottleRendering(); bool becameUnthrottled = wasThrottled && !canThrottleRendering();
if (becameUnthrottled) if (becameUnthrottled) {
// Start ticking animation frames again if necessary.
page()->animator().scheduleVisualUpdate(m_frame.get()); 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 bool FrameView::shouldThrottleRendering() const
......
...@@ -111,8 +111,10 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag ...@@ -111,8 +111,10 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag
return; return;
} }
RELEASE_ASSERT(!frameView().needsLayout()); if (!frameView().shouldThrottleRendering()) {
ASSERT(document->lifecycle().state() >= DocumentLifecycle::CompositingClean); RELEASE_ASSERT(!frameView().needsLayout());
ASSERT(document->lifecycle().state() >= DocumentLifecycle::CompositingClean);
}
TRACE_EVENT1("devtools.timeline", "Paint", "data", InspectorPaintEvent::data(layoutView, LayoutRect(rect), 0)); TRACE_EVENT1("devtools.timeline", "Paint", "data", InspectorPaintEvent::data(layoutView, LayoutRect(rect), 0));
...@@ -133,7 +135,8 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag ...@@ -133,7 +135,8 @@ void FramePainter::paintContents(GraphicsContext& context, const GlobalPaintFlag
PaintLayer* rootLayer = layoutView->layer(); PaintLayer* rootLayer = layoutView->layer();
#if ENABLE(ASSERT) #if ENABLE(ASSERT)
layoutView->assertSubtreeIsLaidOut(); if (!frameView().shouldThrottleRendering())
layoutView->assertSubtreeIsLaidOut();
LayoutObject::SetLayoutNeededForbiddenScope forbidSetNeedsLayout(*rootLayer->layoutObject()); LayoutObject::SetLayoutNeededForbiddenScope forbidSetNeedsLayout(*rootLayer->layoutObject());
#endif #endif
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "core/paint/PaintLayerPainter.h" #include "core/paint/PaintLayerPainter.h"
#include "core/frame/FrameView.h"
#include "core/frame/Settings.h" #include "core/frame/Settings.h"
#include "core/layout/ClipPathOperation.h" #include "core/layout/ClipPathOperation.h"
#include "core/layout/LayoutBlock.h" #include "core/layout/LayoutBlock.h"
...@@ -80,8 +81,7 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayer(GraphicsContext& co ...@@ -80,8 +81,7 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayer(GraphicsContext& co
if (shouldSuppressPaintingLayer(&m_paintLayer)) if (shouldSuppressPaintingLayer(&m_paintLayer))
return FullyPainted; return FullyPainted;
// TODO(skyostil): Unify this early-out logic with subsequence caching. if (m_paintLayer.layoutObject()->isLayoutView() && toLayoutView(m_paintLayer.layoutObject())->frameView()->shouldThrottleRendering())
if (m_paintLayer.layoutObject()->isLayoutPart() && toLayoutPart(m_paintLayer.layoutObject())->isThrottledFrameView())
return FullyPainted; return FullyPainted;
// If this layer is totally invisible then there is nothing to paint. // If this layer is totally invisible then there is nothing to paint.
...@@ -270,9 +270,8 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayerContents(GraphicsCon ...@@ -270,9 +270,8 @@ PaintLayerPainter::PaintResult PaintLayerPainter::paintLayerContents(GraphicsCon
if (paintFlags & PaintLayerPaintingRootBackgroundOnly && !m_paintLayer.layoutObject()->isLayoutView() && !m_paintLayer.layoutObject()->isDocumentElement()) if (paintFlags & PaintLayerPaintingRootBackgroundOnly && !m_paintLayer.layoutObject()->isLayoutView() && !m_paintLayer.layoutObject()->isDocumentElement())
return result; return result;
// TODO(skyostil): Unify this early-out logic with subsequence caching. if (m_paintLayer.layoutObject()->isLayoutView() && toLayoutView(m_paintLayer.layoutObject())->frameView()->shouldThrottleRendering())
if (m_paintLayer.layoutObject()->isLayoutPart() && toLayoutPart(m_paintLayer.layoutObject())->isThrottledFrameView()) return result;
return FullyPainted;
// Ensure our lists are up-to-date. // Ensure our lists are up-to-date.
m_paintLayer.stackingNode()->updateLayerListsIfNeeded(); m_paintLayer.stackingNode()->updateLayerListsIfNeeded();
......
...@@ -211,7 +211,7 @@ TEST_F(FrameThrottlingTest, MutatingThrottledFrameDoesNotCauseAnimation) ...@@ -211,7 +211,7 @@ TEST_F(FrameThrottlingTest, MutatingThrottledFrameDoesNotCauseAnimation)
frameElement->contentDocument()->documentElement()->setAttribute(styleAttr, "background: green"); frameElement->contentDocument()->documentElement()->setAttribute(styleAttr, "background: green");
EXPECT_FALSE(compositor().needsAnimate()); 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, ""); frameElement->setAttribute(styleAttr, "");
EXPECT_TRUE(compositor().needsAnimate()); EXPECT_TRUE(compositor().needsAnimate());
...@@ -251,6 +251,65 @@ TEST_F(FrameThrottlingTest, SynchronousLayoutInThrottledFrame) ...@@ -251,6 +251,65 @@ TEST_F(FrameThrottlingTest, SynchronousLayoutInThrottledFrame)
EXPECT_EQ(50, divElement->clientWidth()); 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) TEST(RemoteFrameThrottlingTest, ThrottledLocalRoot)
{ {
FrameTestHelpers::TestWebViewClient viewClient; FrameTestHelpers::TestWebViewClient viewClient;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment