[webkit-reviews] review denied: [Bug 216295] [Repaint] styleWillChange may call repaint on the same renderer multiple times. : [Attachment 408347] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied zalan
<zalan at apple.com>'s request for review:
Bug 216295: [Repaint] styleWillChange may call repaint on the same renderer
multiple times.
https://bugs.webkit.org/show_bug.cgi?id=216295

Attachment 408347: Patch

https://bugs.webkit.org/attachment.cgi?id=408347&action=review




--- Comment #10 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 408347
  --> https://bugs.webkit.org/attachment.cgi?id=408347
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408347&action=review

> Source/WebCore/rendering/RenderElement.cpp:383
> +void RenderElement::issueRepaintBeforeStyleChange(StyleDifference diff,
const RenderStyle& oldStyle, const RenderStyle& newStyle)

s/issue//

> Source/WebCore/rendering/RenderElement.cpp:387
> +	       // Can't resolve absolute coordinates.

Or more accurately, we're disconnected so don't need to repaint anyway.

> Source/WebCore/rendering/RenderElement.cpp:392
> +	       if (diff == StyleDifference::Repaint)
> +		   return true;

No need to change in this patch, but shouldRepaintForStyleDifference() will
handle this below.

> Source/WebCore/rendering/RenderElement.cpp:400
> +		   auto& layer = *enclosingLayer();

I think you've introduced a behavior change here. The old code lived on
RenderElement and enclosingLayer() could return the layer for an ancestor. So
this renders hasLayer could be false, but we would have found an enclosing
layer. That no longer happens.