Make native button wrapper construction return the new wrapper rather than

setting it on the class. I did this for the textbox and I think it's a bit
better. The old way set it, but that function didn't add it to the view
hierarchy. This means that the pointer is technically "unowned" when the
CreateWrapper function returns.

This new way makes pointer ownership more clear. It becomes the caller's
problem, and the caller immediately inserts it into the view hierarchy which
then manages the lifetime.

TEST=none
BUG=none
Review URL: http://codereview.chromium.org/180041

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@25043 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 30 insertions, 19 deletions

diff --git a/views/controls/button/checkbox.cc b/views/controls/button/checkbox.cc
index c3ea2ff..c45bd52 100644
--- a/views/controls/button/checkbox.cc
+++ b/views/controls/button/checkbox.cc
@@ -153,10 +153,12 @@ std::string Checkbox::GetClassName() const {
 ////////////////////////////////////////////////////////////////////////////////
 // Checkbox, NativeButton overrides:
 
-void Checkbox::CreateWrapper() {
-  native_wrapper_ = NativeButtonWrapper::CreateCheckboxWrapper(this);
-  native_wrapper_->UpdateLabel();
-  native_wrapper_->UpdateChecked();
+NativeButtonWrapper* Checkbox::CreateWrapper() {
+  NativeButtonWrapper* native_wrapper =
+      NativeButtonWrapper::CreateCheckboxWrapper(this);
+  native_wrapper->UpdateLabel();
+  native_wrapper->UpdateChecked();
+  return native_wrapper;
 }
 
 void Checkbox::InitBorder() {
diff --git a/views/controls/button/checkbox.h b/views/controls/button/checkbox.h
index e515b4f..bae6bde 100644
--- a/views/controls/button/checkbox.h
+++ b/views/controls/button/checkbox.h
@@ -57,8 +57,8 @@ class Checkbox : public NativeButton {
  protected:
   virtual std::string GetClassName() const;
 
-  // Overridden from NativeButton2:
-  virtual void CreateWrapper();
+  // Overridden from NativeButton:
+  virtual NativeButtonWrapper* CreateWrapper();
   virtual void InitBorder();
 
   // Returns true if the event (in Checkbox coordinates) is within the bounds of
diff --git a/views/controls/button/native_button.cc b/views/controls/button/native_button.cc
index ccb600a..8a90aaa 100644
--- a/views/controls/button/native_button.cc
+++ b/views/controls/button/native_button.cc
@@ -149,7 +149,9 @@ void NativeButton::SetEnabled(bool flag) {
 void NativeButton::ViewHierarchyChanged(bool is_add, View* parent,
                                          View* child) {
   if (is_add && !native_wrapper_ && GetWidget()) {
-    CreateWrapper();
+    // The native wrapper's lifetime will be managed by the view hierarchy after
+    // we call AddChildView.
+    native_wrapper_ = CreateWrapper();
     AddChildView(native_wrapper_->GetView());
   }
 }
@@ -178,10 +180,12 @@ void NativeButton::Focus() {
 ////////////////////////////////////////////////////////////////////////////////
 // NativeButton, protected:
 
-void NativeButton::CreateWrapper() {
-  native_wrapper_ = NativeButtonWrapper::CreateNativeButtonWrapper(this);
-  native_wrapper_->UpdateLabel();
-  native_wrapper_->UpdateEnabled();
+NativeButtonWrapper* NativeButton::CreateWrapper() {
+  NativeButtonWrapper* native_wrapper =
+      NativeButtonWrapper::CreateNativeButtonWrapper(this);
+  native_wrapper->UpdateLabel();
+  native_wrapper->UpdateEnabled();
+  return native_wrapper;
 }
 
 void NativeButton::InitBorder() {
diff --git a/views/controls/button/native_button.h b/views/controls/button/native_button.h
index 3b77d5f..22dc60a 100644
--- a/views/controls/button/native_button.h
+++ b/views/controls/button/native_button.h
@@ -63,9 +63,12 @@ class NativeButton : public Button {
   virtual std::string GetClassName() const;
   virtual bool AcceleratorPressed(const Accelerator& accelerator);
 
-  // Create the button wrapper. Can be overridden by subclass to create a
-  // wrapper of a particular type. See NativeButtonWrapper interface for types.
-  virtual void CreateWrapper();
+  // Create the button wrapper and returns it. Ownership of the returned
+  // value is passed to the caller.
+  //
+  // This can be overridden by subclass to create a wrapper of a particular
+  // type. See NativeButtonWrapper interface for types.
+  virtual NativeButtonWrapper* CreateWrapper();
 
   // Sets a border to the button. Override to set a different border or to not
   // set one (the default is 0,8,0,8 for push buttons).
diff --git a/views/controls/button/radio_button.cc b/views/controls/button/radio_button.cc
index 744dbe1..44cf426 100644
--- a/views/controls/button/radio_button.cc
+++ b/views/controls/button/radio_button.cc
@@ -98,10 +98,12 @@ std::string RadioButton::GetClassName() const {
 ////////////////////////////////////////////////////////////////////////////////
 // RadioButton, NativeButton overrides:
 
-void RadioButton::CreateWrapper() {
-  native_wrapper_ = NativeButtonWrapper::CreateRadioButtonWrapper(this);
-  native_wrapper_->UpdateLabel();
-  native_wrapper_->UpdateChecked();
+NativeButtonWrapper* RadioButton::CreateWrapper() {
+  NativeButtonWrapper* native_wrapper =
+      NativeButtonWrapper::CreateRadioButtonWrapper(this);
+  native_wrapper->UpdateLabel();
+  native_wrapper->UpdateChecked();
+  return native_wrapper;
 }
 
 }  // namespace views
diff --git a/views/controls/button/radio_button.h b/views/controls/button/radio_button.h
index 9a7d82e..9039117 100644
--- a/views/controls/button/radio_button.h
+++ b/views/controls/button/radio_button.h
@@ -30,7 +30,7 @@ class RadioButton : public Checkbox {
   virtual std::string GetClassName() const;
 
   // Overridden from NativeButton:
-  virtual void CreateWrapper();
+  virtual NativeButtonWrapper* CreateWrapper();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(RadioButton);