[webkit-reviews] review granted: [Bug 201284] Avoid running the outline painting phase if no renderers have outlines : [Attachment 377636] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 201284: Avoid running the outline painting phase if no renderers have
outlines
https://bugs.webkit.org/show_bug.cgi?id=201284

Attachment 377636: Patch

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




--- Comment #6 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 377636
  --> https://bugs.webkit.org/attachment.cgi?id=377636
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?