Make nsRenderingContext a stack class

RESOLVED FIXED in mozilla36

Status

()

enhancement
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: zwol, Assigned: jwatt)

Tracking

(Blocks 1 bug)

Trunk
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Honestly, I'd almost rather it go away entirely.
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.
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.
Assignee: zackw → jwatt
Blocks: 1088760
Posted patch patchSplinter Review
Attachment #8513898 - Flags: review?(bjacob)
Added MOZ_STACK_CLASS locally.
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.
Attachment #8513898 - Flags: review?(bjacob)
Attachment #8513898 - Flags: review?(jmuizelaar)
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
Attachment #8513898 - Flags: review?(jmuizelaar) → review+
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).
https://hg.mozilla.org/mozilla-central/rev/097629b2eb2a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.