[webkit-reviews] review granted: [Bug 230296] setNeedsLayout() should not be called when changing the SVG properties : [Attachment 455394] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted Rob Buis
<rbuis at igalia.com>'s request for review:
Bug 230296: setNeedsLayout() should not be called when changing the SVG
properties
https://bugs.webkit.org/show_bug.cgi?id=230296

Attachment 455394: Patch

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




--- Comment #27 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 455394
  --> https://bugs.webkit.org/attachment.cgi?id=455394
Patch

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

> Source/WebCore/svg/SVGSVGElement.cpp:222
> +#if ENABLE(LAYER_BASED_SVG_ENGINE)
> +		   if (is<RenderSVGRoot>(renderer) &&
downcast<RenderSVGRoot>(renderer)->isEmbeddedThroughFrameContainingSVGDocument(
))
> +		       embeddedThroughFrame = true;
> +#endif
> +		   if (is<LegacyRenderSVGRoot>(renderer) &&
downcast<LegacyRenderSVGRoot>(renderer)->isEmbeddedThroughFrameContainingSVGDoc
ument())
> +		       embeddedThroughFrame = true;

Why we do have to check is<LegacyRenderSVGRoot>(renderer) in the case #if
ENABLE(LAYER_BASED_SVG_ENGINE)? Should not the check
is<LegacyRenderSVGRoot>(renderer) be inside the #else ... #endif block? I think
also we can get rid of the local "embeddedThroughFrame" and call
renderer->view().setNeedsLayout(MarkOnlyThis); directly.