651021 - Make nsRenderingContext a stack class

Status: RESOLVED FIXED

(Core :: Graphics, enhancement)

Description

[Zack Weinberg (:zwol)]

nsRenderingContext is now a very thin shim around gfxContext; there's no good reason to allocate it on the heap anymore.  It might at this point also make more sense if it lived in layout/base rather than gfx/src.

Comment 1

[Joe Drew (not getting mail)]

Honestly, I'd almost rather it go away entirely.

Comment 2

[Zack Weinberg (:zwol)]

I wouldn't mind seeing it go away entirely, but I am not going to have time to do that patch, whereas I might have time to do this one.

Comment 3

[Jonathan Watt [:jwatt]]

Fixing this makes it easier to fix bug 1088760, since we can then convert the code piecemeal and wrap gfxContexts up in in-the-stack nsRenderingContexts at arbitrary points without worrying about allocation overhead.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 5

[Jonathan Watt [:jwatt]]

Added MOZ_STACK_CLASS locally.

Comment 6

[Jonathan Watt [:jwatt]]

This passes Try BTW.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1a8a85fb333b

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 8513898 [details] [diff] [review]
patch

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

I feel that I'm the wrong reviewer, because I've never dealt with nsRenderingContext at all, so I could be missing basic facts about it. Try :jrmuizel or :mattwoodrow.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8513898 [details] [diff] [review]
patch

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

::: gfx/src/nsRenderingContext.h
@@ +16,5 @@
>  class DrawTarget;
>  }
>  }
>  
>  class nsRenderingContext MOZ_FINAL

Would MOZ_STACK_CLASS be appropriate here?

::: layout/printing/nsPrintEngine.cpp
@@ +2264,5 @@
>          fprintf(fd, "Title: %s\n", docStr.get());
>          fprintf(fd, "URL:   %s\n", urlStr.get());
>          fprintf(fd, "--------------- Frames ----------------\n");
> +        //nsRefPtr<gfxContext> renderingContext =
> +        //  mPrt->mPrintDocDC->CreateRenderingContext();

Should these lines go away?

@@ +3809,5 @@
>      fprintf(fd, "URL:   %s\n", aURLStr?aURLStr:"");
>      fprintf(fd, "--------------- Frames ----------------\n");
>      fprintf(fd, "--------------- Frames ----------------\n");
> +    //nsRefPtr<gfxContext> renderingContext =
> +    //  aDC->CreateRenderingContext();

Same here

Comment 9

[Jonathan Watt [:jwatt]]

Thanks!

(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> Would MOZ_STACK_CLASS be appropriate here?

Indeed, see comment 5.

As for the commented out lines, probably, but I don't know and I don't care to investigate much (lots of better things to do).

Comment 10

[Jonathan Watt [:jwatt]]

Comment on attachment 8513898 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/097629b2eb2a

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/097629b2eb2a