1349865 - Use RAII to set and clear PAINTSERVER state bit

Status: RESOLVED FIXED

(Core :: SVG, enhancement)

Comment 1

[Robert Longson [:longsonr]]

Attachment: 1349865.txt

Comment 2

[Jonathan Watt [:jwatt]]

Comment on attachment 8850418 [details] [diff] [review]
1349865.txt

Review of attachment 8850418 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like we have actual bugs in both places where we set NS_FRAME_DRAWING_AS_PAINTSERVER. Nice catch!

In the case of nsSVGPatternFrame I think we could handle this by adding another loop flag to that class and using AutoReferenceChainGuard. (So we'd have one flag for detecting loops along the pattern "inheritance" chain, and one for detecting loops in patterns that contain content that reference back to the same pattern.) Unfortunately it doesn't seem so easy to replace NS_FRAME_DRAWING_AS_PAINTSERVER with AutoReferenceChainGuard in the nsSVGIntegrationUtils code since we don't know the type of the nsIFrame in order to get a loop flag off it. :/

At any rate, this looks good.

::: layout/svg/nsSVGPaintServerFrame.h
@@ +26,5 @@
>  
>  struct gfxRect;
>  
> +/**
> + * RAII class used to temporarily set and remove the 

There's trailing white space at the end of this line.

@@ +34,5 @@
> +class MOZ_RAII AutoSetRestorePaintServerState
> +{
> +public:
> +  explicit AutoSetRestorePaintServerState(
> +             nsIFrame* aFrame MOZ_GUARD_OBJECT_NOTIFIER_PARAM) :

Maybe put MOZ_GUARD_OBJECT_NOTIFIER_PARAM on a new line since it introduces another parameter rather than modifying aFrame?

Comment 3

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ba56d4376513f8363a3145b7b37f2dc05487ac9

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d5787a383e4
use RAII to set/unset NS_FRAME_DRAWING_AS_PAINTSERVER state bit r=jwatt

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6d5787a383e4