[webkit-reviews] review granted: [Bug 58807] Create RIAA class for GraphicsContext::save() and restore() : [Attachment 90354] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 58807: Create RIAA class for GraphicsContext::save() and restore()
https://bugs.webkit.org/show_bug.cgi?id=58807

Attachment 90354: Patch
https://bugs.webkit.org/attachment.cgi?id=90354&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=90354&action=review

Looks great.

> Source/WebCore/platform/graphics/GraphicsContext.h:556
> +    class GraphicsContextStateSaver {

Kinda a mouth-full, but I don't have better.

> Source/WebCore/platform/graphics/GraphicsContext.h:560
> +	   : m_context(context)
> +	   , m_saveAndRestore(saveAndRestore)

I think we technically indent these on level (differnet from how xcode does
things).

> Source/WebCore/platform/graphics/GraphicsContext.h:576
> +	       ASSERT(!m_saveAndRestore);
> +	       m_context.save();
> +	       m_saveAndRestore = true;

We could make this a counter instead.  Not sure how useful that would be.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:638
> +    GraphicsContextStateSaver clipToBorderStateSaver(*context,
clipToBorderRadius);
> +    if (clipToBorderRadius) {

Oh, that's so sexy.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1525
> +		   ++outerBorderTopWidth;
> +		   ++outerBorderBottomWidth;
> +		   ++outerBorderLeftWidth;
> +		   ++outerBorderRightWidth;

Bleh.  (For another patch. :)

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1553
> +	   GraphicsContextStateSaver graphicsStateSaver(*graphicsContext);

You call it "stateSaver" other places.