I have a patch which consolidates the 3 types of PresContexts (Galley, Print, PrintPreview) onto a single nsIPresContext implementation. nsIPresContext no longer has any virtual methods when called from inside layout. When called from outside layout, I implemented passthrough virtual functions for the small amount of functionality that's needed (that isn't inlined).
Brian, I probably won't be able to get to this till a week from now....
+ NS_HIDDEN_(void) SetImageAnimationModeInternal(PRUint16 aMode); This can be moved out of the #ifdef. Personally I'd have just made the external callers all change to call External, and just had SetImageAnimationMode be local, but I can deal with this. Ditto for the other methods you made external... + NS_HIDDEN_(void) SetPageDim(nsRect* aRect); Really this should be a const nsRect&. + /* Convenience method for converting one pixel to twips */ Should say "for converting one pixel *value* to twips" more coming...
+ nsCAutoString mCharset; I think you don't want an auto string as a class member? +nsIPresContext::GetPageDim(nsRect* aActualRect, nsRect* aAdjRect) I think this should be broken up into two methods, GetPageDimActual and GetPageDimAdjusted, which each return an nsRect. Anyway, as written, + *aAdjRect = mPageDim; this should be inside the if (aAdjRect && aActualRect) + nscoord IntPixelsToTwips(nscoord aPixels) const I think this should be called IntScaledPixelsToTwips. The rest looks good. Let me know how you plan to address these and I'll apply the stamp :-)
> I think this should be broken up into two methods, GetPageDimActual and > GetPageDimAdjusted, which each return an nsRect. Anyway, as written, It's a toss-up... 2 out of 4 callers want both rects; one call is more efficient than two. I'm going to leave and just tweak the null check. I applied all the rest of your suggestions.
Very well. I still think GetPageDim would be better as two functions, but I'll yield on that point especially because this is existing code not new code. r+sr=roc, check this thing in!
checked in, along with follow-up to rename nsIPresContext to nsPresContext.
In the checkin with comment: "Change nsIPresContext to nsPresContext globally, follow-up to bug 253470. rs=roc", it looks like a number of interfaces were modified, but the UUIDs of those interfaces were not bumped. Should they have been bumped? nsPresContext -> nsIPresContext doesn't sound like it would throw off any vtables, so maybe not an issue.
Yeah, I think there's no issue there.