Convert GetPreferredSize from:...

Convert GetPreferredSize from:

void GetPreferredSize(CSize* out);

to:

gfx::Size GetPreferredSize();

.. and update some other places to use gfx::Size as well.

http://crbug.com/2186



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=3400

[Ben Goodger (Google), 2008-10-15 07:58:41]

This is probably the worst of them.

[Elliot Glaysher, 2008-10-15 17:21:46]

http://codereview.chromium.org/7344/diff/1/4
File chrome/browser/drag_utils.cc (right):

http://codereview.chromium.org/7344/diff/1/4#newcode156
Line 156: ChromeCanvas canvas(prefsize.width(), prefsize.height(), false);
Can ChromeCanvas take a gfx::Size?

http://codereview.chromium.org/7344/diff/1/48
File chrome/browser/views/download_shelf_view.cc (right):

http://codereview.chromium.org/7344/diff/1/48#newcode68
Line 68: size->Enlarge(view_preferred.width(), 0);
One general comment: I haven't seen a case yet where one of the parameters to
Enlarge( ) isn't 0. Have you thought about separating Enlarge() into
EnlargeWidth() and EnlargeHeight()?

[Ben Goodger (Google), 2008-10-15 17:37:32]

http://codereview.chromium.org/7344/diff/1/4
File chrome/browser/drag_utils.cc (right):

http://codereview.chromium.org/7344/diff/1/4#newcode156
Line 156: ChromeCanvas canvas(prefsize.width(), prefsize.height(), false);
On 2008/10/15 17:21:46, Elliot Glaysher wrote:
> Can ChromeCanvas take a gfx::Size?
I looked at the users of Canvas. Most of them aren't using gfx::Size at the
moment. Upgrading Canvas would be a pretty large task. I could add the extra
ctor, but for one caller it doesn't seem worth it at the moment.

http://codereview.chromium.org/7344/diff/1/48
File chrome/browser/views/download_shelf_view.cc (right):

http://codereview.chromium.org/7344/diff/1/48#newcode68
Line 68: size->Enlarge(view_preferred.width(), 0);
On 2008/10/15 17:21:46, Elliot Glaysher wrote:
> One general comment: I haven't seen a case yet where one of the parameters to
> Enlarge( ) isn't 0. Have you thought about separating Enlarge() into
> EnlargeWidth() and EnlargeHeight()?

OK I tried EnlargeWidth and EnlargeHeight. But I don't like them. The purpose of
having these functions is to make some operations possible with less typing.
Otherwise we'd always use the setter/getters for the properties being adjusted.
While it might seem like it's simpler because it's only operating on one value,
EnlargeWidth/EnlargeHeight actually force wraps in some places where it's
awkward, because they're frequently longer than Enlarge(value, 0); or Enlarge(0,
value);

So I decided not to add EnlargeWidth and EnlargeHeight :-P

[Elliot Glaysher, 2008-10-15 17:56:24]

LGTM then.