It's non-obvious what BaseRect::IsZero means, when reading code that uses it. If I see... if (r.IsZero()) { doStuff(); } ...it's not obvious which members are being checked for zeroness (and whether all have to be zero or if only one is enough). I think we should rename it to IsZeroArea(). That makes its meaning (width==0||height==0) clearer. (We *might* want to rename the subtly-different IsEmpty as well to e.g. IsEmptyArea, but I'd say we should probably leave that one alone -- "empty" is clearer on its own. It's obviously size-related and it's obvious that either dimension is sufficent to trigger "emptiness".)

Bas also suggested we do an audit of the places that currently call IsZero and see if those should be replaced by IsEmpty.

IMO we should still do this rename first, since it's trivial & a clear readability win with no behavior impact, and then we can audit usages (& make potentially-behavior-changing edits) afterwards/elsewhere. (feel free to hold on this until after bug 1424382 lands, to avoid causing bitrot in the massive-patch there. Though I guess it'd be a pretty trivial search-and-replace so either order is probably fine.)

Comment on attachment 8941653 [details] Bug 1429602: Rename BaseRect::IsZero to BaseRect::IsZeroArea. Also slip in some corrections to using BaseRect methods instead of direct member access. .schouten https://reviewboard.mozilla.org/r/211900/#review217692

Pushed by msreckovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0d48950798f4 Rename BaseRect::IsZero to BaseRect::IsZeroArea. Also slip in some corrections to using BaseRect methods instead of direct member access. r=bas.schouten