Bug 58807 - Create RIAA class for GraphicsContext::save() and restore()
Summary: Create RIAA class for GraphicsContext::save() and restore()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-18 12:12 PDT by Simon Fraser (smfr)
Modified: 2011-04-21 10:45 PDT (History)
4 users (show)

See Also:

Attachments
Patch (27.90 KB, text/plain)
2011-04-20 09:59 PDT, Simon Fraser (smfr) 		no flags Details
Patch (81.76 KB, patch)
2011-04-20 21:47 PDT, Simon Fraser (smfr) 		eric: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-04-18 12:12:15 PDT 
Create RIAA class for GraphicsContext::save() and restore()
Comment 1 Simon Fraser (smfr) 2011-04-20 09:59:34 PDT 
Created attachment 90354 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-20 10:31:50 PDT 
Comment on attachment 90354 [details]
Patch

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.
Comment 3 Simon Fraser (smfr) 2011-04-20 20:42:19 PDT 
Comment on attachment 90354 [details]
Patch

Landed in http://trac.webkit.org/changeset/84463

Will keep bug open for more usage.
Comment 4 Simon Fraser (smfr) 2011-04-20 21:47:12 PDT 
Created attachment 90490 [details]
Patch
Comment 5 Eric Seidel (no email) 2011-04-20 22:06:14 PDT 
Comment on attachment 90490 [details]
Patch

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

LGTM.

> Source/WebCore/rendering/RenderFileUploadControl.cpp:212
> +    GraphicsContextStateSaver stateSaver(*paintInfo.context, false);

I think we should turn this bool into an Enum at some point.
Comment 6 Simon Fraser (smfr) 2011-04-21 08:56:18 PDT 
http://trac.webkit.org/changeset/84504
Comment 7 WebKit Review Bot 2011-04-21 10:45:57 PDT 
http://trac.webkit.org/changeset/84504 might have broken GTK Linux 32-bit Release