I've now completed bug 1087958, so nsRenderingContext is now just an empty wrapper around a gfxContext/DrawTarget that provides no functionality. Converting the close to 1000 nsRenderingContext instances and arguments to gfxContext at this stage would be a waste of coding/review time though. I've been finding that as I've been converting code to DrawTarget the number of instances of the string "nsRenderingContext" in the code has been dropping rapidly anyway. Rather than doing a huge conversion from nsRenderingContext to gfxContext and then later from gfxContext to DrawTarget, I think we should just wait for the number of nsRenderingContext instances to drop via the Moz2D conversion process and then do a direct nsRenderingContext -> DrawTarget conversion once we get to an appropriate point. That way we save ourselves some work.

Hey jwatt! I was working on refactoring a bunch of code to use DrawTarget and stumbled across the results of your work -- that nsRenderingContext just wraps gfxContext. It looked like the replacement work was trivial, so I wrote up a quick patch with some find-and-replace with manual cleanup. While finishing up the cleanup I came across this bug -- do you still think going forward with this would be a waste of time?

If the patches are fairly straightforward to review I can review what you've done. If they do things like change pointers to references (and require lots of related review work to sanity check) then it may not be the best use of review time. If you can break up the patches into manageable, standalone, compilable chunks (sub 50 KB chunks ideally, but definitely sub 100 KB chucks) I'm happy to take a look. If you've converted everything then attach the patches here, but if there is still outstanding work then probably file one or more blocker bugs (like the existing, fixed blockers) and attach the patches there.

Patch pushed: it's almost entirely completely boring mechanical work, as most uses of nsRenderingContext are just feeding through to other APIs that require nsRenderingContext. The only real work happens at the boundaries between APIs that take nsRenderingContext and gfxContext. For those it's mostly just eliminating a temporary variable for the conversion. Sometimes this leads to the used variable changing from a reference to a pointer, but the fallout of this is pretty boring (and trivially caught by the compiler if an error is made).

Comment on attachment 8876822 [details] Bug 1088760 - Remove nsRenderingContext, replacing all of its uses with gfxContext. https://reviewboard.mozilla.org/r/148140/#review152506 Looks good. We can pick up any fixes in a post commit review. Let's get this in ASAP to avoid having it rot due to churn.

Comment on attachment 8876822 [details] Bug 1088760 - Remove nsRenderingContext, replacing all of its uses with gfxContext. https://reviewboard.mozilla.org/r/148140/#review152550 Lovely. ::: gfx/src/nsFontMetrics.h:203 (Diff revision 1) > DrawTarget* aDrawTarget); > > // Draw a string using this font handle on the surface passed in. > void DrawString(const char *aString, uint32_t aLength, > nscoord aX, nscoord aY, > - nsRenderingContext *aContext); > + gfxContext *aContext); Don't fix this now (get this landed, as Jeff says), but for future reference while you're touching lines like this you might as well type-hug the "*" as per our style guidelines.

Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1ce85e6348 Remove nsRenderingContext, replacing all of its uses with gfxContext. r=jwatt,jrmuizel

yay!

Backed out for bustage, at least on Android at layout/generic/nsPluginFrame.cpp:1612: https://hg.mozilla.org/integration/mozilla-inbound/rev/b1940873102d01722956b79991166286e121072a Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=3d1ce85e6348307a1e98284e6d13da828729bf91&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Bustage log: https://treeherder.mozilla.org/logviewer.html#?job_id=106479413&repo=mozilla-inbound [task 2017-06-12T22:24:35.079183Z] 22:24:35 INFO - /home/worker/workspace/build/src/layout/generic/nsPluginFrame.cpp: In member function 'void nsPluginFrame::PaintPlugin(nsDisplayListBuilder*, gfxContext&, const nsRect&, const nsRect&)': [task 2017-06-12T22:24:35.079294Z] 22:24:35 INFO - /home/worker/workspace/build/src/layout/generic/nsPluginFrame.cpp:1612:72: error: no matching function for call to 'nsPluginInstanceOwner::Paint(gfxContext&, gfxRect&, gfxRect&)' [task 2017-06-12T22:24:35.079338Z] 22:24:35 INFO - mInstanceOwner->Paint(aRenderingContext, frameGfxRect, dirtyGfxRect); [task 2017-06-12T22:24:35.079375Z] 22:24:35 INFO - ^ [task 2017-06-12T22:24:35.080718Z] 22:24:35 INFO - /home/worker/workspace/build/src/layout/generic/nsPluginFrame.cpp:1612:72: note: candidate is: [task 2017-06-12T22:24:35.081411Z] 22:24:35 INFO - In file included from /home/worker/workspace/build/src/layout/generic/nsPluginFrame.cpp:50:0: [task 2017-06-12T22:24:35.082103Z] 22:24:35 INFO - /home/worker/workspace/build/src/dom/plugins/base/nsPluginInstanceOwner.h:112:8: note: void nsPluginInstanceOwner::Paint(gfxContext*, const gfxRect&, const gfxRect&) [task 2017-06-12T22:24:35.082969Z] 22:24:35 INFO - void Paint(gfxContext* aContext, [task 2017-06-12T22:24:35.083776Z] 22:24:35 INFO - ^ [task 2017-06-12T22:24:35.084641Z] 22:24:35 INFO - /home/worker/workspace/build/src/dom/plugins/base/nsPluginInstanceOwner.h:112:8: note: no known conversion for argument 1 from 'gfxContext' to 'gfxContext*' [task 2017-06-12T22:24:35.085469Z] 22:24:35 INFO - /home/worker/workspace/build/src/config/rules.mk:1064: recipe for target 'nsPluginFrame.o' failed [task 2017-06-12T22:24:35.086294Z] 22:24:35 INFO - gmake[5]: *** [nsPluginFrame.o] Error 1 (In reply to Nicholas Nethercote [:njn] from comment #8) > yay! http://i.imgur.com/jLUAQuo.gif

Ah, I only checked compilation on linux/mac. I'll kick off a try on all platforms to check all the ifdefs.

Oh yes, an all-platforms try build is definitely a good idea before pushing any large refactoring change :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bb830220893e021ca9b04570e8f7e2f2e18e727&selectedJob=106520865 Seems good. Only change was fixing that one line that Sebastien posted. Pushed to mozreview.

Pushed by jmuizelaar@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e5dda793c3e Remove nsRenderingContext, replacing all of its uses with gfxContext. r=jwatt,jrmuizel