Created attachment 625590 [details] [diff] [review]
Unhandled enum value in switch statement is esp very noisy there. I've added defaults there, but probably in some cases default could be marked as unreachable. Other warnings were about static function was defained but not used (that's because they were in a headers and were not used by some header users; the fix requires marking them inline or moving to .cpp files). I've fixed also sign-compare warnings that seem safe in this case. The most useful seem to be unused variable warnings.
Comment on attachment 625590 [details] [diff] [review]
Review of attachment 625590 [details] [diff] [review]:
Looks fine mostly.
@@ +87,5 @@
> ID2D1Factory *DrawTargetD2D::mFactory;
> IDWriteFactory *DrawTargetD2D::mDWriteFactory;
> +static bool IsPatternSupportedByD2D(const Pattern &aPattern)
Let's make these inline and -not- move them into cpp files. I don't want to move them again when other files need this in the future.
I've moved them back and marked inline:
Thanks for the review.
Backed out for compilation failures:
The failure was because MOZ_NOT_REACHED_MARKER is not present at all for GCC older than 4.5 in Assertion.h. I believe it's a bug (it should be at least defined as empty), but that's a separated issue. I've removed that part from my patch it builds on try:
should just be
That would use MOZ_Assert instead of its macro version MOZ_ASSERT, which AFAIR is not desired in this code.