Merge TextButton and LabelButton border images util structs, etc.

Merge TextButton and LabelButton border images util structs, etc.

Consolidate new BorderImages files from TextButton and LabelButton.
Rewrite image painting as a struct member function; support arbitrary rects.
Add static IDR arrays for TextButton and LabelButton shared image assets

Use LabelButton views as implicit NativeThemeDelegates for LabelButtonBorder.
Nix LabelButtonBorder layer kClear_Mode as in http://crrev.com/160227.
Use macros to define ordered image asset IDR arrays; misc cleanup/refactoring.

TODO(followup): Merge "nine-box" code in BorderImages, BubbleBorder, ImagePainter, etc.
TODO(followup): Stitch border image assets together and use ImagePainter?

BUG=155363
TEST=No observable button changes or regressions.
R=pkasting@chromium.org
TBR=sky@chromium.org

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

[msw, 2012-10-24 21:31:02]

Hey Peter, please take a look; thanks! I'm on the fence regarding the macros.

[Peter Kasting, 2012-10-25 23:56:38]

LGTM

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/bo...
File ui/views/controls/button/border_images.cc (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/bo...
ui/views/controls/button/border_images.cc:15: x ## _BOTTOM_LEFT, x ## _BOTTOM, x
## _BOTTOM_RIGHT,
Nit: If we're using this macro in multiple files, we should probably find a good
common place for it.

We should figure out what all the places are that use nine such images and see
if we can make them all use this macro, as well as (ideally) the same way of
painting.

Should the new border_images.* files you're adding in this change go somewhere
outside of controls/button/ so we can migrate other code to using them?

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/bo...
ui/views/controls/button/border_images.cc:56: canvas->DrawImageInt(top, 0, 0,
top.width(), top.height(),
Maybe the t/l/c/r/b images should be TileImageInt calls?

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/la...
File ui/views/controls/button/label_button.h (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/la...
ui/views/controls/button/label_button.h:58: // Overridden from CustomButton:
Nit: Can we make any of these overrides private, or do people access them all
directly through LabelButton?

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
File ui/views/controls/button/text_button.cc (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
ui/views/controls/button/text_button.cc:102: void TextButtonBorder::Paint(const
View& view, gfx::Canvas* canvas) const {
This now looks a lot like the label button paint function.  Is the plan to merge
them later?

[tfarina, 2012-10-26 00:38:12]

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
File ui/views/controls/button/text_button.h (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
ui/views/controls/button/text_button.h:17: #include
"ui/views/controls/button/border_images.h"
nit: sort

[tfarina, 2012-10-26 00:38:36]

http://codereview.chromium.org/11262002/diff/1008/ui/views/views.gyp
File ui/views/views.gyp (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/views.gyp#newcode70
ui/views/views.gyp:70: 'controls/button/border_images.cc',
nit: sort

[tfarina, 2012-10-26 00:40:18]

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
File ui/views/controls/button/text_button.cc (right):

http://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/te...
ui/views/controls/button/text_button.cc:30: static const int
kDefaultIconTextSpacing = 5;
nit: since we are here, can we remove the 'static' from all these constants?
since you wrapped then in the unnamed namespace.

[msw, 2012-10-27 07:40:57]

Comments addressed; landing (may TBR sky).

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/b...
File ui/views/controls/button/border_images.cc (right):

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/b...
ui/views/controls/button/border_images.cc:15: x ## _BOTTOM_LEFT, x ## _BOTTOM, x
## _BOTTOM_RIGHT,
On 2012/10/25 23:56:38, Peter Kasting wrote:
> Nit: If we're using this macro in multiple files, we should probably find a
good
> common place for it.
> 
> We should figure out what all the places are that use nine such images and see
> if we can make them all use this macro, as well as (ideally) the same way of
> painting.
> 
> Should the new border_images.* files you're adding in this change go somewhere
> outside of controls/button/ so we can migrate other code to using them?

The macros differ with the IDR naming schemes, but I'll followup.
(either by naming similarly and consolidating, or by stitching a single image)
I'll consolidate nine-box code in followup, as I mention in the header.

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/b...
ui/views/controls/button/border_images.cc:56: canvas->DrawImageInt(top, 0, 0,
top.width(), top.height(),
On 2012/10/25 23:56:38, Peter Kasting wrote:
> Maybe the t/l/c/r/b images should be TileImageInt calls?

Done for t/l/r/b, but scaling center makes more sense than tiling, I think.

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/l...
File ui/views/controls/button/label_button.h (right):

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/l...
ui/views/controls/button/label_button.h:58: // Overridden from CustomButton:
On 2012/10/25 23:56:38, Peter Kasting wrote:
> Nit: Can we make any of these overrides private, or do people access them all
> directly through LabelButton?

I made them all private, but now LabelButtonBorder does an extra cast or two.

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/t...
File ui/views/controls/button/text_button.cc (right):

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/t...
ui/views/controls/button/text_button.cc:30: static const int
kDefaultIconTextSpacing = 5;
On 2012/10/26 00:40:18, tfarina wrote:
> nit: since we are here, can we remove the 'static' from all these constants?
> since you wrapped then in the unnamed namespace.

Done (same for label_button.cc) and moved PrefixTypeToCanvasType into the
anonymous namespace and removed its static keyword as well.

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/t...
ui/views/controls/button/text_button.cc:102: void TextButtonBorder::Paint(const
View& view, gfx::Canvas* canvas) const {
On 2012/10/25 23:56:38, Peter Kasting wrote:
> This now looks a lot like the label button paint function.  Is the plan to
merge
> them later?

Yes, I hope to merge them in a followup.

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/t...
File ui/views/controls/button/text_button.h (right):

https://codereview.chromium.org/11262002/diff/1008/ui/views/controls/button/t...
ui/views/controls/button/text_button.h:17: #include
"ui/views/controls/button/border_images.h"
On 2012/10/26 00:38:12, tfarina wrote:
> nit: sort

Done.

https://codereview.chromium.org/11262002/diff/1008/ui/views/views.gyp
File ui/views/views.gyp (right):

https://codereview.chromium.org/11262002/diff/1008/ui/views/views.gyp#newcode70
ui/views/views.gyp:70: 'controls/button/border_images.cc',
On 2012/10/26 00:38:36, tfarina wrote:
> nit: sort

Done.

[commit-bot: I haz the power, 2012-10-27 07:41:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/11262002/16001

[commit-bot: I haz the power, 2012-10-27 07:41:19]

Presubmit check for 11262002-16001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ui/views

Presubmit checks took 1.3s to calculate.

[commit-bot: I haz the power, 2012-10-27 07:56:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/11262002/16001

[commit-bot: I haz the power, 2012-10-27 08:32:05]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-27 20:40:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/11262002/29001

[commit-bot: I haz the power, 2012-10-27 22:37:33]

Change committed as 164557