Reafactor Textfield to bring the native widget handling more in line with what

we do in button. We still don't have NativeTextfieldWin deriving from
NativeControlWin, which is what the addition of the hack function works around.
Review URL: http://codereview.chromium.org/173588

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

4 files changed, 48 insertions, 20 deletions

diff --git a/views/controls/textfield/native_textfield_win.cc b/views/controls/textfield/native_textfield_win.cc
index cda6629..e99c073 100644
--- a/views/controls/textfield/native_textfield_win.cc
+++ b/views/controls/textfield/native_textfield_win.cc
@@ -66,6 +66,7 @@ NativeTextfieldWin::NativeTextfieldWin(Textfield* textfield)
       ime_discard_composition_(false),
       ime_composition_start_(0),
       ime_composition_length_(0),
+      container_view_(new NativeViewHost),
       bg_color_(0) {
   if (!did_load_library_)
     did_load_library_ = !!LoadLibrary(L"riched20.dll");
@@ -96,11 +97,6 @@ NativeTextfieldWin::NativeTextfieldWin(Textfield* textfield)
   CComPtr<IRichEditOle> ole_interface;
   ole_interface.Attach(GetOleInterface());
   text_object_model_ = ole_interface;
-
-  container_view_ = new NativeViewHost;
-  textfield_->AddChildView(container_view_);
-  container_view_->set_focus_view(textfield_);
-  container_view_->Attach(m_hWnd);
 }
 
 NativeTextfieldWin::~NativeTextfieldWin() {
@@ -108,6 +104,12 @@ NativeTextfieldWin::~NativeTextfieldWin() {
     DestroyWindow();
 }
 
+void NativeTextfieldWin::AttachHack() {
+  // See the code in textfield.cc that calls this for why this is here.
+  container_view_->set_focus_view(textfield_);
+  container_view_->Attach(m_hWnd);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeTextfieldWin, NativeTextfieldWrapper implementation:
 
diff --git a/views/controls/textfield/native_textfield_win.h b/views/controls/textfield/native_textfield_win.h
index 514c27f..aaf7e7c 100644
--- a/views/controls/textfield/native_textfield_win.h
+++ b/views/controls/textfield/native_textfield_win.h
@@ -36,6 +36,9 @@ class NativeTextfieldWin
   explicit NativeTextfieldWin(Textfield* parent);
   ~NativeTextfieldWin();
 
+  // See the code in textfield.cc that calls this for why this is here.
+  void AttachHack();
+
   // Overridden from NativeTextfieldWrapper:
   virtual std::wstring GetText() const;
   virtual void UpdateText();
diff --git a/views/controls/textfield/textfield.cc b/views/controls/textfield/textfield.cc
index 5e953ed..486ac19 100644
--- a/views/controls/textfield/textfield.cc
+++ b/views/controls/textfield/textfield.cc
@@ -12,6 +12,12 @@
 #include "views/controls/textfield/native_textfield_wrapper.h"
 #include "views/widget/widget.h"
 
+#if defined(OS_WIN)
+// TODO(beng): this should be removed when the OS_WIN hack from
+// ViewHierarchyChanged is removed.
+#include "views/controls/textfield/native_textfield_win.h"
+#endif
+
 namespace views {
 
 // static
@@ -49,8 +55,6 @@ Textfield::Textfield(StyleFlags style)
 }
 
 Textfield::~Textfield() {
-  if (native_wrapper_)
-    delete native_wrapper_;
 }
 
 void Textfield::SetController(Controller* controller) {
@@ -242,20 +246,21 @@ void Textfield::Focus() {
 void Textfield::ViewHierarchyChanged(bool is_add, View* parent, View* child) {
   if (is_add && !native_wrapper_ && GetWidget() && !initialized_) {
     initialized_ = true;
-    native_wrapper_ = NativeTextfieldWrapper::CreateWrapper(this);
-    //AddChildView(native_wrapper_->GetView());
-    // TODO(beng): Move this initialization to NativeTextfieldWin once it
-    //             subclasses NativeControlWin.
-    native_wrapper_->UpdateText();
-    native_wrapper_->UpdateBackgroundColor();
-    native_wrapper_->UpdateReadOnly();
-    native_wrapper_->UpdateFont();
-    native_wrapper_->UpdateEnabled();
-    native_wrapper_->UpdateBorder();
 
-    // We need to call Layout here because any previous calls to Layout
-    // will have short-circuited and we don't call AddChildView.
-    Layout();
+    // The native wrapper's lifetime will be managed by the view hierarchy after
+    // we call AddChildView.
+    native_wrapper_ = CreateWrapper();
+    AddChildView(native_wrapper_->GetView());
+
+#if defined(OS_WIN)
+    // TODO(beng): remove this once NativeTextfieldWin subclasses
+    // NativeControlWin. This is currently called to perform post-AddChildView
+    // initialization for the wrapper. The GTK version subclasses things
+    // correctly and doesn't need this.
+    //
+    // Remove the include for native_textfield_win.h above when you fix this.
+    static_cast<NativeTextfieldWin*>(native_wrapper_)->AttachHack();
+#endif
   }
 }
 
@@ -263,4 +268,18 @@ std::string Textfield::GetClassName() const {
   return kViewClassName;
 }
 
+NativeTextfieldWrapper* Textfield::CreateWrapper() {
+  NativeTextfieldWrapper* native_wrapper =
+      NativeTextfieldWrapper::CreateWrapper(this);
+
+  native_wrapper->UpdateText();
+  native_wrapper->UpdateBackgroundColor();
+  native_wrapper->UpdateReadOnly();
+  native_wrapper->UpdateFont();
+  native_wrapper->UpdateEnabled();
+  native_wrapper->UpdateBorder();
+
+  return native_wrapper;
+}
+
 }  // namespace views
diff --git a/views/controls/textfield/textfield.h b/views/controls/textfield/textfield.h
index a008bea..68f3be5 100644
--- a/views/controls/textfield/textfield.h
+++ b/views/controls/textfield/textfield.h
@@ -183,6 +183,10 @@ class Textfield : public View {
   virtual void ViewHierarchyChanged(bool is_add, View* parent, View* child);
   virtual std::string GetClassName() const;
 
+  // Creates a new native wrapper properly initialized and returns it. Ownership
+  // is passed to the caller.
+  NativeTextfieldWrapper* CreateWrapper();
+
   // The object that actually implements the native text field.
   NativeTextfieldWrapper* native_wrapper_;