Skip to content
Permalink
Browse files
REGRESSION (r294902): Content with continuation leaves decoration bit…
…s behind when removed

https://bugs.webkit.org/show_bug.cgi?id=241734
<rdar://95308322>

Reviewed by Simon Fraser.

This patch ensures that when a renderer is removed we always issue a repaint regardless of what the associated layer's repaint bit says.

1. after r294902, repaint is not issued anymore if either the associated or an ancestor layer have the "full repaint" bit set.
2. such layer-driven repaints happen after layout.

In some dynamic content cases, the layer may be removed before layout happens. This patch ensures that we preemptively issue such repaints.

* LayoutTests/fast/repaint/force-repaint-when-layer-is-destroyed-expected.txt: Added.
* LayoutTests/fast/repaint/force-repaint-when-layer-is-destroyed.html: Added.

* Source/WebCore/rendering/RenderLayerModelObject.cpp: Force (full) repaint when the renderer is being destroyed (detached -> non-internal move).
(WebCore::RenderLayerModelObject::willBeRemovedFromTree):
* Source/WebCore/rendering/RenderLayerModelObject.h:
* Source/WebCore/rendering/RenderObject.cpp: move duplicated code from repaint() and repaintRectangle() to issueRepaint().

Canonical link: https://commits.webkit.org/251670@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295665 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
alanbaradlay committed Jun 20, 2022
1 parent 815da56 commit a440dc2
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 26 deletions.
@@ -0,0 +1 @@
PASS
@@ -0,0 +1,36 @@
<style>
.content {
transform: translateX(0px);
border: 10px solid red;
width: 120px;
}

.continuation {
width: 100px;
height: 100px;
background-color: green;
border: solid blue;
}
</style><div id=hide_this class=content><span><div class=continuation></div></span></div>
<pre id=result></pre>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

setTimeout(function() {
if (window.internals)
internals.startTrackingRepaints();

hide_this.style.display = "none";

if (window.internals) {
result.innerText = internals.repaintRectsAsText().indexOf("8 8 140 126") != -1 ? "PASS" : "FAIL";
internals.stopTrackingRepaints();
}

if (window.testRunner)
testRunner.notifyDone();
}, 10);
</script>
@@ -77,6 +77,14 @@ void RenderLayerModelObject::willBeDestroyed()
RenderElement::willBeDestroyed();
}

void RenderLayerModelObject::willBeRemovedFromTree(IsInternalMove isInternalMove)
{
if (auto* layer = this->layer(); layer && layer->needsFullRepaint() && isInternalMove == IsInternalMove::No)
issueRepaint(std::nullopt, ClipRepaintToLayer::No, ForceRepaint::Yes);

RenderElement::willBeRemovedFromTree(isInternalMove);
}

void RenderLayerModelObject::destroyLayer()
{
ASSERT(!hasLayer());
@@ -95,6 +95,7 @@ class RenderLayerModelObject : public RenderElement {

void createLayer();
void willBeDestroyed() override;
void willBeRemovedFromTree(IsInternalMove) override;

private:
std::unique_ptr<RenderLayer> m_layer;
@@ -977,45 +977,37 @@ static inline bool fullRepaintIsScheduled(const RenderObject& renderer)
return false;
}

void RenderObject::repaint() const
void RenderObject::issueRepaint(std::optional<LayoutRect> partialRepaintRect, ClipRepaintToLayer clipRepaintToLayer, ForceRepaint forceRepaint) const
{
// Don't repaint if we're unrooted (note that view() still returns the view when unrooted)
if (!isRooted())
return;

const RenderView& view = this->view();
if (view.printing())
return;

auto repaintContainer = containerForRepaint();
if (!repaintContainer.renderer)
repaintContainer = { fullRepaintIsScheduled(*this), &view };
repaintContainer = { fullRepaintIsScheduled(*this), &view() };

if (!repaintContainer.fullRepaintIsScheduled)
repaintUsingContainer(repaintContainer.renderer, clippedOverflowRectForRepaint(repaintContainer.renderer));
if (repaintContainer.fullRepaintIsScheduled && forceRepaint == ForceRepaint::No)
return;

auto repaintRect = partialRepaintRect ? computeRectForRepaint(*partialRepaintRect, repaintContainer.renderer) : clippedOverflowRectForRepaint(repaintContainer.renderer);
repaintUsingContainer(repaintContainer.renderer, repaintRect, clipRepaintToLayer == ClipRepaintToLayer::Yes);
}

void RenderObject::repaintRectangle(const LayoutRect& r, bool shouldClipToLayer) const
void RenderObject::repaint() const
{
// Don't repaint if we're unrooted (note that view() still returns the view when unrooted)
if (!isRooted())
if (!isRooted() || view().printing())
return;
issueRepaint();
}

const RenderView& view = this->view();
if (view.printing())
void RenderObject::repaintRectangle(const LayoutRect& repaintRect, bool shouldClipToLayer) const
{
// Don't repaint if we're unrooted (note that view() still returns the view when unrooted)
if (!isRooted() || view().printing())
return;

LayoutRect dirtyRect(r);
// FIXME: layoutDelta needs to be applied in parts before/after transforms and
// repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308
dirtyRect.move(view.frameView().layoutContext().layoutDelta());

auto repaintContainer = containerForRepaint();
if (!repaintContainer.renderer)
repaintContainer = { fullRepaintIsScheduled(*this), &view };

if (!repaintContainer.fullRepaintIsScheduled)
repaintUsingContainer(repaintContainer.renderer, computeRectForRepaint(dirtyRect, repaintContainer.renderer), shouldClipToLayer);
auto dirtyRect = repaintRect;
dirtyRect.move(view().frameView().layoutContext().layoutDelta());
issueRepaint(dirtyRect, shouldClipToLayer ? ClipRepaintToLayer::Yes : ClipRepaintToLayer::No);
}

void RenderObject::repaintSlowRepaintObject() const
@@ -803,6 +803,10 @@ class RenderObject : public CachedImageClient {

bool isSetNeedsLayoutForbidden() const;

enum class ClipRepaintToLayer : uint8_t { No, Yes };
enum class ForceRepaint : uint8_t { No, Yes };
void issueRepaint(std::optional<LayoutRect> partialRepaintRect = std::nullopt, ClipRepaintToLayer = ClipRepaintToLayer::No, ForceRepaint = ForceRepaint::No) const;

private:
void addAbsoluteRectForLayer(LayoutRect& result);
void setLayerNeedsFullRepaint();

0 comments on commit a440dc2

Please sign in to comment.