Bug 201284 – Avoid running the outline painting phase if no renderers have outlines

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Avoid running the outline painting phase if no renderers have outlines

[Simon Fraser (smfr)]

Created attachment 377544 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 377584 [details]
Patch

[EWS Watchlist]

Comment on attachment 377584 [details]
Patch

Attachment 377584 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12980576

New failing tests:
fast/borders/hidpi-outline-on-subpixel-position.html
fast/replaced/outline-replaced-elements-offset.html
svg/custom/rgba-color-outline.svg
svg/text/text-outline-rgba.html
transforms/2d/preserve3d-not-fixed-container.html
svg/text/tspan-multiple-outline.svg
transforms/2d/perspective-not-fixed-container.html
fast/block/block-only/block-replaced-with-vertical-margins.html
fast/repaint/layer-child-outline.html
fast/css/sticky/sticky-side-margins.html

[EWS Watchlist]

Created attachment 377594 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Simon Fraser (smfr)]

Created attachment 377636 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 377636 [details]
Patch

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

> Source/WebCore/rendering/RenderView.h:153
> +    void addRenderWithOutline() { m_renderersWithOutlineCount++; }

This name is a little bit confusing. This function does not add a renderer with outline to the RenderView. How about didAddOutlineToRenderer() or something similar? Also can we use the pre increment ++ operator?

> Source/WebCore/rendering/RenderView.h:154
> +    void removeRenderWithOutline() { ASSERT(m_renderersWithOutlineCount > 0); m_renderersWithOutlineCount--; }

This ASSERT will not prevent the overflow in the release build. Can we add an if statement to ensure m_renderersWithOutlineCount will not overflow?

[Simon Fraser (smfr)]

(In reply to Said Abou-Hallawa from comment #6)
> Comment on attachment 377636 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377636&action=review
> 
> > Source/WebCore/rendering/RenderView.h:153
> > +    void addRenderWithOutline() { m_renderersWithOutlineCount++; }
> 
> This name is a little bit confusing. This function does not add a renderer
> with outline to the RenderView. How about didAddOutlineToRenderer() or
> something similar? Also can we use the pre increment ++ operator?

I'll call them incrementRendersWithOutline/decrementRendersWithOutline

> > Source/WebCore/rendering/RenderView.h:154
> > +    void removeRenderWithOutline() { ASSERT(m_renderersWithOutlineCount > 0); m_renderersWithOutlineCount--; }
> 
> This ASSERT will not prevent the overflow in the release build. Can we add
> an if statement to ensure m_renderersWithOutlineCount will not overflow?

Underflow is benign in this case; we'll just do a bit more painting.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/249309/webkit

[Radar WebKit Bug Importer]

<rdar://problem/54866678>