There are a few canvas functions that really ought to be shared with any other contexts. This introduces CanvasUtils for those common functions. Though from looking at this, ThebesSurfaceFromElement isn't really canvas specific; it could be used elsewhere. roc, what do you think about putting that on nsLayoutUtils?

That makes sense. Make sure it's 2-space-indent there though. It has a lot of code in common with nsBaseDragService::DrawDragForImage, although it's probably hard to share. Also I think instead of taking 4 out parameters it would make more sense for ThebesSurfaceFromElement to return a struct or at least take a struct as a single out parameter.

Also if you're going to call it CanvasUtils instead of nsCanvasUtils, put it in the mozilla namespace.

moved some stuff into CanvasUtils, and some stuff into nsLayoutUtils. Used struct, etc.

+ // Check that the rectangle [x,y,w,h] is a subrectangle of [0,0,realWidth,realHeight] + + static PRBool CheckSaneSubrectSize(PRInt32 x, PRInt32 y, PRInt32 w, PRInt32 h, + PRInt32 realWidth, PRInt32 realHeight) Why not just nsIntRect(0, 0, realWidth, realHeight).Contains(nsIntRect(x, y, w, h))? + * Obtain a gfxASurface from the given DOM element, if possible. + * This obtains the most natural surface from the element; that + * is, the one that can be obtained with the fewest conversions. It's probably worth saying exactly what it does for each element type. + struct SurfaceFromElementResult { + nsRefPtr<gfxASurface> surface; + gfxIntSize size; + nsCOMPtr<nsIPrincipal> principal; + PRBool isWriteOnly; + }; mBlahBlah convention + surf = new gfxImageSurface(gfxIntSize(w, h), gfxASurface::ImageFormatARGB32); + else + surf = gfxPlatform::GetPlatform()->CreateOffscreenSurface(gfxIntSize(w, h), gfxASurface::ImageFormatARGB32); We generally have {} around single statements in layout, apart from break/return/continue. See what I mean about suckiness of per-module style. + static nsresult SurfaceFromElement(nsIDOMElement *aElement, + PRUint32 aSurfaceFlags, + SurfaceFromElementResult *aResult); Why not just return SurfaceFromElementResult directly, and set mSurface to null on error?

Updated with comments addressed.