Add VectorIconButton functionality to ImageButton.

ui/views/controls/button/image_button_util.h

Index: ui/views/controls/button/image_button_util.h
diff --git a/ui/views/controls/button/image_button_util.h b/ui/views/controls/button/image_button_util.h
new file mode 100644
index 0000000000000000000000000000000000000000..d586abce04db3c995d3bc84aaca022c3bb1e872c
--- /dev/null
+++ b/ui/views/controls/button/image_button_util.h
@@ -0,0 +1,33 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+#ifndef UI_VIEWS_CONTROLS_BUTTON_IMAGE_BUTTON_UTIL_H_

[Evan Stade, 2017/03/17 13:54:44]

nit: not sure how many more things will be added to this file. If we're trying
to consolidate button classes though, there may be similar things added for
other button types. Should we make one button_util file?

[sky, 2017/03/17 15:38:12]

I prefer not having files that become dumping grounds. I might go with
image_button_factory, which isn't entirely true but close. If you prefer the
name you have here, stick with it.

[Bret, 2017/03/17 23:41:41]

I went with image_button_factory as the more specific name.

+#define UI_VIEWS_CONTROLS_BUTTON_IMAGE_BUTTON_UTIL_H_
+
+#include "third_party/skia/include/core/SkColor.h"
+#include "ui/views/views_export.h"
+
+namespace gfx {
+struct VectorIcon;
+}
+
+namespace views {
+
+class ButtonListener;
+class ImageButton;
+
+// Creates an ImageButton with an ink drop and a centered image in preparation
+// for applying a vector icon with SetImageFromVectorIcon below.
+VIEWS_EXPORT ImageButton* CreateImageButtonWithVectorIconStyling(

[Evan Stade, 2017/03/17 13:54:45]

nit: naming is hard, but I'd give this a shorter name, like
CreateVectorImageButton

[Bret, 2017/03/17 23:41:41]

This one is pretty tough. I went with your suggestion.

+    ButtonListener* listener);
+
+// Sets images on |button| for STATE_NORMAL and STATE_DISABLED from the given
+// vector icon and color.

[sky, 2017/03/17 15:38:12]

optional: as a common pattern is calling this from theme changed it worth
documenting an example of that?

[Bret, 2017/03/17 23:41:41]

I put in a sentence about calling this again if the color changes.

+VIEWS_EXPORT void SetImageFromVectorIcon(ImageButton* button,
+                                         const gfx::VectorIcon& icon,
+                                         SkColor base_color);

[Evan Stade, 2017/03/17 13:54:45]

nit: the nomenclature here is probably my fault, but it's weird that "base"
color is used in two different senses in this function ("base" for the vector
icon is not "base" for the ink drop). We might want to call the parameter
adjacent_text_color or something (because it's the color value for nearby/would
be nearby text, e.g. for the tab close button it would be tab text). It also
bears mentioning in the documentation above what this value means.

[Evan Stade, 2017/03/17 13:54:45]

nit: I think we should make base_color optional, and if not passed use
GetNativeTheme()->GetSystemColor(kColorId_LabelEnabledColor)

[Bret, 2017/03/17 23:41:41]

I updated the names and comment. I'm a bit hesitant to make the color actually
optional, since for this patch I'd rather all callers just pass in the color
they're using right now.

[Evan Stade, 2017/03/18 00:27:58]

It was already optional. Callers that didn't specify a color would get black.
Let's continue making the color optional and default to black. I think the label
text color trick won't work until Harmony is on.

[Bret, 2017/03/18 20:12:53]

Okay, done.

+
+}  // namespace views
+
+#endif  // UI_VIEWS_CONTROLS_BUTTON_IMAGE_BUTTON_UTIL_H_