views::ImageButton: Added SetMinimumImageSize; removed SetPreferredSize.

views::ImageButton: Added SetMinimumImageSize; removed SetPreferredSize.

ImageButton::SetMinimumImageSize allows the client to explicitly set a
size larger than the image contained within the button. The image will
be aligned inside the button according to its alignment setting.

ImageButton::SetPreferredSize had a misleading name (since in fact it
only sets the size when there is no image) and is no longer needed. Its
original usage (r50040) is now gone. It wasn't having any effect in
either of its two remaining callers, since they always set an image
immediately afterwards. Therefore, deleted this method and removed both
calls to it. Note that we are keeping the default behaviour that an
ImageButton with no image has a size of 16x14. I wanted to remove this,
but the list of potential clients that could be relying on it is huge.

BUG=415909
Committed: https://crrev.com/306b915c2ea97c618aa011aa6a3f39c5d5b0600b
Cr-Commit-Position: refs/heads/master@{#295917}

[Matt Giuca, 2014-09-19 09:07:43]

mgiuca@chromium.org changed reviewers:
+ sky@chromium.org, zturner@chromium.org

[Matt Giuca, 2014-09-19 09:07:44]

sky: Please review changes in views. Also OWNERS for views and ash.
zturner: Please confirm the call to SetPreferredSize in volume_view.cc was
unnecessary.
calamity, kcarattini: FYI, I am removing the call to SetPreferredSize in
ContentsSwitcherView and removing the "0 is OK" semantics for AddSwitcherButton
(ContentsView always passes nonzero here).

[Matt Giuca, 2014-09-19 09:20:18]

FYI the reason I need SetMinimumImageSize is for the follow-up CL
https://codereview.chromium.org/581923002/. (I tried doing it using SetBorder
but it was a mess of calculations because you had to account for cases where the
image was bigger / smaller than the desired size.)

[sky, 2014-09-19 15:31:06]

LGTM

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
File ui/views/controls/button/image_button.cc (right):

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:33: minimum_image_size_(0, 0),
nit: remove this as 0,0 is the default.

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:78: minimum_image_size_ = size;
early out if size == minimum_image_size_

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:92:
size.set_width(std::max(size.width(), minimum_image_size_.width()));
size.SetToMax(minimum_image_size_);

[zturner, 2014-09-19 16:16:51]

On 2014/09/19 09:07:44, Matt Giuca wrote:
> sky: Please review changes in views. Also OWNERS for views and ash.
> zturner: Please confirm the call to SetPreferredSize in volume_view.cc was
> unnecessary.
> calamity, kcarattini: FYI, I am removing the call to SetPreferredSize in
> ContentsSwitcherView and removing the "0 is OK" semantics for
AddSwitcherButton
> (ContentsView always passes nonzero here).

lgtm

[Matt Giuca, 2014-09-22 00:41:06]

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
File ui/views/controls/button/image_button.cc (right):

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:33: minimum_image_size_(0, 0),
On 2014/09/19 15:31:06, sky wrote:
> nit: remove this as 0,0 is the default.

Done.

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:78: minimum_image_size_ = size;
On 2014/09/19 15:31:06, sky wrote:
> early out if size == minimum_image_size_

Done.

https://codereview.chromium.org/583843003/diff/60001/ui/views/controls/button...
ui/views/controls/button/image_button.cc:92:
size.set_width(std::max(size.width(), minimum_image_size_.width()));
On 2014/09/19 15:31:06, sky wrote:
> size.SetToMax(minimum_image_size_);

Done.

[Matt Giuca, 2014-09-22 00:41:11]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2014-09-22 00:41:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583843003/80001

[commit-bot: I haz the power, 2014-09-22 03:48:59]

Committed patchset #5 (id:80001) as 232dfea248864ddfa183136dae0799327bccf113

[commit-bot: I haz the power, 2014-09-22 03:50:00]

Patchset 5 (id:??) landed as
https://crrev.com/306b915c2ea97c618aa011aa6a3f39c5d5b0600b
Cr-Commit-Position: refs/heads/master@{#295917}