views::LabelButton should not call virtual methods from its constructor

ui/views/controls/button/label_button.h

Index: ui/views/controls/button/label_button.h
diff --git a/ui/views/controls/button/label_button.h b/ui/views/controls/button/label_button.h
index 2efd5b11f7d273e19eba4cdc95bbac37e5f4eee5..23bbbf3dd93da42010d93d3393a29ce3da08c2a7 100644
--- a/ui/views/controls/button/label_button.h
+++ b/ui/views/controls/button/label_button.h
@@ -28,9 +28,24 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
 
   static const char kViewClassName[];
 
+  // Construct an uninitialized LabelButton with a Listener. Client code must
+  // also call Init() or InitAsButton() to initialize the (possibly themed)
+  // button borders.
+  explicit LabelButton(ButtonListener* listener);
+
+  // Deprecated (calls virtual methods). TODO(tapted): Remove this overload.
   LabelButton(ButtonListener* listener, const base::string16& text);
+
   ~LabelButton() override;
 
+  // Initializes the LabelButton. If borders have not been set with SetBorder(),
+  // creates default themed borders for the current ButtonStyle. The default
+  // button style defaults to STYLE_TEXTBUTTON, or it be set by a subclass.

[sadrul, 2015/07/08 05:59:11]

Can there be a single 'Init(text, style)' instead? If not, let's call this one
InitAsTextButton().

[tapted, 2015/07/08 06:31:33]

I actually started with Init(style, text) in Patchset 2. But everything that
called it was passing STYLE_BUTTON. This was because a `default` style was
supported -- everything wants "default" or STYLE_BUTTON.

Then I tossed up InitAsTextButton for a long time too.. I went with `Init`
because it allows a subclass (like BlueButton) to change the default to
STYLE_BUTTON, without a caller having to know that they should call InitAsButton
instead of InitAsTextButton. This will come up more in follow-ups CLs, once
BlueButton's constructor no longer calls virtual methods.

Having `Init`+`SetDefaultStyle` also means that if, in future, a subclass finds
a need to override Init(), it's clear which Init() it should override. That is,
we can guarantee that InitAsFoo() always calls Init(), which may be overridden.

This can also be true for the `Init(text, style)`, flavour, but it means feeding
a lot of STYLE_TEXTBUTTON arguments into places that don't currently need to
care about it.

So, does that change your mind :) ?

If not, I'm fine with InitAsTextButton too. It would just mean discarding the
concept of a default style. I could get rid of SetDefaultStyle as well. (and if
the override-Init() thing comes up in future, it could be achieved with a
protected Init() method or some other solution).

[tapted, 2015/07/09 00:53:48]

Done. (went with Textbutton over TextButton to match the enum)

+  void Init(const base::string16& text);
+
+  // Calls Init() after setting the ButtonStyle to STYLE_BUTTON.
+  void InitAsButton(const base::string16& text);
+
   // Get or set the image shown for the specified button state.
   // GetImage returns the image for STATE_NORMAL if the state's image is empty.
   virtual const gfx::ImageSkia& GetImage(ButtonState for_state);
@@ -73,8 +88,11 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
   bool is_default() const { return is_default_; }
   void SetIsDefault(bool is_default);
 
-  // Get or set the button's overall style; the default is |STYLE_TEXTBUTTON|.
+  // Gets the button's overall style; the default is |STYLE_TEXTBUTTON|.
   ButtonStyle style() const { return style_; }
+
+  // Sets the button style and updates (possibly themed) borders. TODO(tapted):
+  // Make this method protected after callers have switched to InitAsButton().
   void SetStyle(ButtonStyle style);
 
   // Set the spacing between the image and the text. Shrinking the spacing
@@ -96,6 +114,10 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
   ImageView* image() const { return image_; }
   Label* label() const { return label_; }
 
+  // Called by a subclass constructor to cause the Init(const string16&)
+  // overload to use a default style other than STYLE_TEXTBUTTON.
+  void SetDefaultStyle(ButtonStyle style);
+
   // Returns the available area for the label and image. Subclasses can change
   // these bounds if they need room to do manual painting.
   virtual gfx::Rect GetChildAreaBounds();