Review URL: http://codereview.chromium.org/150051

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

12 files changed, 49 insertions, 55 deletions

diff --git a/views/controls/button/native_button_win.cc b/views/controls/button/native_button_win.cc
index fa00456..86dd91f 100644
--- a/views/controls/button/native_button_win.cc
+++ b/views/controls/button/native_button_win.cc
@@ -97,10 +97,6 @@ bool NativeButtonWin::OnKeyDown(int vkey) {
   return enter_pressed;
 }
 
-bool NativeButtonWin::NotifyOnKeyDown() const {
-  return true;
-}
-
 void NativeButtonWin::CreateNativeControl() {
   DWORD flags = WS_CHILD | WS_CLIPSIBLINGS | WS_CLIPCHILDREN | BS_PUSHBUTTON;
   if (native_button_->is_default())
diff --git a/views/controls/button/native_button_win.h b/views/controls/button/native_button_win.h
index 2b9d016..ea7e7cb 100644
--- a/views/controls/button/native_button_win.h
+++ b/views/controls/button/native_button_win.h
@@ -37,7 +37,6 @@ class NativeButtonWin : public NativeControlWin,
   virtual bool OnKeyDown(int vkey);
 
  protected:
-  virtual bool NotifyOnKeyDown() const;
   virtual void CreateNativeControl();
   virtual void NativeControlCreated(HWND control_hwnd);
 
diff --git a/views/controls/combobox/combobox.h b/views/controls/combobox/combobox.h
index 2196974..a66e379 100644
--- a/views/controls/combobox/combobox.h
+++ b/views/controls/combobox/combobox.h
@@ -67,11 +67,11 @@ class Combobox : public View {
   virtual void ViewHierarchyChanged(bool is_add, View* parent,
                                     View* child);
   virtual std::string GetClassName() const;
-   
- private:
+
   // The object that actually implements the native combobox.
   NativeComboboxWrapper* native_wrapper_;
 
+ private:
   // Our model.
   Model* model_;
 
diff --git a/views/controls/combobox/native_combobox_win.cc b/views/controls/combobox/native_combobox_win.cc
index 43e1979..dee1108 100644
--- a/views/controls/combobox/native_combobox_win.cc
+++ b/views/controls/combobox/native_combobox_win.cc
@@ -136,6 +136,10 @@ void NativeComboboxWin::SetFocus() {
   Focus();
 }
 
+gfx::NativeView NativeComboboxWin::GetTestingHandle() const {
+  return native_view();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeComboboxWin, NativeControlWin overrides:
 
@@ -158,7 +162,7 @@ void NativeComboboxWin::CreateNativeControl() {
   DWORD flags = WS_CHILD | WS_CLIPSIBLINGS | WS_CLIPCHILDREN |
                 CBS_DROPDOWNLIST | WS_VSCROLL;
   HWND control_hwnd = ::CreateWindowEx(GetAdditionalExStyle(), L"COMBOBOX", L"",
-                                       flags, 0, 0, 100, 20, //width(), height(),
+                                       flags, 0, 0, width(), height(),
                                        GetWidget()->GetNativeView(), NULL, NULL,
                                        NULL);
   NativeControlCreated(control_hwnd);
diff --git a/views/controls/combobox/native_combobox_win.h b/views/controls/combobox/native_combobox_win.h
index 627b551..46c25a6 100644
--- a/views/controls/combobox/native_combobox_win.h
+++ b/views/controls/combobox/native_combobox_win.h
@@ -25,6 +25,7 @@ class NativeComboboxWin : public NativeControlWin,
   virtual gfx::Size GetPreferredSize() const;
   virtual View* GetView();
   virtual void SetFocus();
+  virtual gfx::NativeView GetTestingHandle() const;
 
  protected:
   // Overridden from NativeControlWin:
@@ -34,7 +35,7 @@ class NativeComboboxWin : public NativeControlWin,
                               LRESULT* result);
   virtual void CreateNativeControl();
   virtual void NativeControlCreated(HWND native_control);
-  
+
  private:
   void UpdateFont();
 
diff --git a/views/controls/combobox/native_combobox_wrapper.h b/views/controls/combobox/native_combobox_wrapper.h
index 27e1c8b..3cecde0 100644
--- a/views/controls/combobox/native_combobox_wrapper.h
+++ b/views/controls/combobox/native_combobox_wrapper.h
@@ -5,6 +5,8 @@
 #ifndef VIEWS_CONTROLS_COMBOBOX_NATIVE_COMBOBOX_WRAPPER_H_
 #define VIEWS_CONTROLS_COMBOBOX_NATIVE_COMBOBOX_WRAPPER_H_
 
+#include "base/gfx/native_widget_types.h"
+
 namespace gfx{
 class Size;
 }
@@ -33,13 +35,16 @@ class NativeComboboxWrapper {
 
   // Returns the preferred size of the combobox.
   virtual gfx::Size GetPreferredSize() const = 0;
-  
+
   // Retrieves the views::View that hosts the native control.
   virtual View* GetView() = 0;
 
   // Sets the focus to the button.
   virtual void SetFocus() = 0;
 
+  // Returns a handle to the underlying native view for testing.
+  virtual gfx::NativeView GetTestingHandle() const = 0;
+
   static NativeComboboxWrapper* CreateWrapper(Combobox* combobox);
 };
 
diff --git a/views/controls/native_control.cc b/views/controls/native_control.cc
index 2df961e..8e65689 100644
--- a/views/controls/native_control.cc
+++ b/views/controls/native_control.cc
@@ -362,8 +362,8 @@ LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message,
       static_cast<NativeControl*>(GetProp(window, kNativeControlKey));
   DCHECK(native_control);
 
-  if (message == WM_KEYDOWN && native_control->NotifyOnKeyDown()) {
-    if (native_control->OnKeyDown(static_cast<int>(w_param)))
+  if (message == WM_KEYDOWN &&
+      native_control->OnKeyDown(static_cast<int>(w_param))) {
       return 0;
   } else if (message == WM_SETFOCUS) {
     // Let the focus manager know that the focus changed.
diff --git a/views/controls/native_control.h b/views/controls/native_control.h
index f6f9752..0f68fe1 100644
--- a/views/controls/native_control.h
+++ b/views/controls/native_control.h
@@ -81,17 +81,8 @@ class NativeControl : public View {
   void SetFixedWidth(int width, Alignment alignment);
   void SetFixedHeight(int height, Alignment alignment);
 
-  // Derived classes interested in receiving key down notification should
-  // override this method and return true.  In which case OnKeyDown is called
-  // when a key down message is sent to the control.
-  // Note that this method is called at the time of the control creation: the
-  // behavior will not change if the returned value changes after the control
-  // has been created.
-  virtual bool NotifyOnKeyDown() const { return false; }
-
-  // Invoked when a key is pressed on the control (if NotifyOnKeyDown returns
-  // true).  Should return true if the key message was processed, false
-  // otherwise.
+  // Invoked when a key is pressed on the control.
+  // Should return true if the key message was processed, false otherwise.
   virtual bool OnKeyDown(int virtual_key_code) { return false; }
 
   // Returns additional extended style flags. When subclasses call
diff --git a/views/controls/native_control_win.cc b/views/controls/native_control_win.cc
index b6111fd..1082adb 100644
--- a/views/controls/native_control_win.cc
+++ b/views/controls/native_control_win.cc
@@ -15,12 +15,6 @@ namespace views {
 const wchar_t* NativeControlWin::kNativeControlWinKey =
     L"__NATIVE_CONTROL_WIN__";
 
-static const wchar_t* kNativeControlOriginalWndProcKey =
-    L"__NATIVE_CONTROL_ORIGINAL_WNDPROC__";
-
-// static
-WNDPROC NativeControlWin::original_wndproc_ = NULL;
-
 ////////////////////////////////////////////////////////////////////////////////
 // NativeControlWin, public:
 
@@ -118,17 +112,10 @@ void NativeControlWin::NativeControlCreated(HWND native_control) {
   // Note that we never unset this property. We don't have to.
   SetProp(native_control, kNativeControlWinKey, this);
 
-  // Subclass the window so we can monitor for key presses. It's important that
-  // we *only* do this if the derived class wants to intercept keypresses,
-  // because otherwise the subclass can mysteriously interfere with certain
-  // other controls, like the combobox, and cause weird effects.
-  if (NotifyOnKeyDown()) {
-    original_wndproc_ =
-        win_util::SetWindowProc(native_control,
-                                &NativeControlWin::NativeControlWndProc);
-    SetProp(native_control, kNativeControlOriginalWndProcKey,
-            original_wndproc_);
-  }
+  // Subclass so we get WM_KEYDOWN and WM_SETFOCUS messages.
+  original_wndproc_ =
+      win_util::SetWindowProc(native_control,
+                              &NativeControlWin::NativeControlWndProc);
 
   Attach(native_control);
   // native_view() is now valid.
@@ -191,8 +178,8 @@ LRESULT NativeControlWin::NativeControlWndProc(HWND window,
       static_cast<NativeControlWin*>(GetProp(window, kNativeControlWinKey));
   DCHECK(native_control);
 
-  if (message == WM_KEYDOWN && native_control->NotifyOnKeyDown()) {
-    if (native_control->OnKeyDown(static_cast<int>(w_param)))
+  if (message == WM_KEYDOWN &&
+      native_control->OnKeyDown(static_cast<int>(w_param))) {
       return 0;
   } else if (message == WM_SETFOCUS) {
     // Let the focus manager know that the focus changed.
diff --git a/views/controls/native_control_win.h b/views/controls/native_control_win.h
index 4fb5244..c9c18a6 100644
--- a/views/controls/native_control_win.h
+++ b/views/controls/native_control_win.h
@@ -45,14 +45,6 @@ class NativeControlWin : public NativeViewHost {
   // received from the HWND created by an object derived from NativeControlWin.
   virtual void ShowContextMenu(const gfx::Point& location);
 
-  // Derived classes interested in receiving key down notification should
-  // override this method and return true.  In which case OnKeyDown is called
-  // when a key down message is sent to the control.
-  // Note that this method is called at the time of the control creation: the
-  // behavior will not change if the returned value changes after the control
-  // has been created.
-  virtual bool NotifyOnKeyDown() const { return false; }
-
   // Called when the NativeControlWin is attached to a View hierarchy with a
   // valid Widget. The NativeControlWin should use this opportunity to create
   // its associated HWND.
@@ -90,7 +82,7 @@ class NativeControlWin : public NativeViewHost {
                                                LPARAM l_param);
 
   // The window procedure before we subclassed.
-  static WNDPROC original_wndproc_;
+  WNDPROC original_wndproc_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeControlWin);
 };
diff --git a/views/controls/tree/tree_view.h b/views/controls/tree/tree_view.h
index 7e35097..e3427d0 100644
--- a/views/controls/tree/tree_view.h
+++ b/views/controls/tree/tree_view.h
@@ -156,10 +156,9 @@ class TreeView : public NativeControl, TreeModelObserver {
   // Handles a variety of potential TreeView messages.
   virtual LRESULT OnNotify(int w_param, LPNMHDR l_param);
 
-  // Yes, we want to be notified of key down for two reasons. To circumvent
-  // VK_ENTER from toggling the expaned state when processes_enter_ is false,
-  // and to have F2 start editting.
-  virtual bool NotifyOnKeyDown() const { return true; }
+  // We pay attention to key down for two reasons: to circumvent VK_ENTER from
+  // toggling the expaned state when processes_enter_ is false, and to have F2
+  // start editting.
   virtual bool OnKeyDown(int virtual_key_code);
 
   virtual void OnContextMenu(const WTL::CPoint& location);
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index cc56271..4f50527 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -18,6 +18,7 @@
 #include "views/controls/button/native_button.h"
 #include "views/controls/button/radio_button.h"
 #include "views/controls/combobox/combobox.h"
+#include "views/controls/combobox/native_combobox_wrapper.h"
 #include "views/controls/label.h"
 #include "views/controls/link.h"
 #include "views/controls/scroll_view.h"
@@ -725,6 +726,20 @@ class TestTextfield : public Textfield {
   }
 };
 
+class TestCombobox : public Combobox, public Combobox::Model {
+ public:
+  TestCombobox() : Combobox(this) { }
+  virtual HWND TestGetNativeComponent() {
+    return native_wrapper_->GetTestingHandle();
+  }
+  virtual int GetItemCount(Combobox* source) {
+    return 10;
+  }
+  virtual std::wstring GetItemAt(Combobox* source, int index) {
+    return L"Hello combo";
+  }
+};
+
 class TestTabbedPane : public TabbedPane {
  public:
   TestTabbedPane() { }
@@ -740,6 +755,7 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
   TestCheckbox* checkbox = new TestCheckbox(L"Checkbox");
   TestRadioButton* radio_button = new TestRadioButton(L"RadioButton");
   TestTextfield* textfield = new TestTextfield();
+  TestCombobox* combobox = new TestCombobox();
   TestTabbedPane* tabbed_pane = new TestTabbedPane();
   TestNativeButton* tab_button = new TestNativeButton(L"tab button");
 
@@ -747,6 +763,7 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
   content_view_->AddChildView(checkbox);
   content_view_->AddChildView(radio_button);
   content_view_->AddChildView(textfield);
+  content_view_->AddChildView(combobox);
   content_view_->AddChildView(tabbed_pane);
   tabbed_pane->AddTab(L"Awesome tab", tab_button);
 
@@ -764,6 +781,9 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
   ::SendMessage(textfield->TestGetNativeComponent(), WM_SETFOCUS, NULL, NULL);
   EXPECT_EQ(textfield, GetFocusManager()->GetFocusedView());
 
+  ::SendMessage(combobox->TestGetNativeComponent(), WM_SETFOCUS, NULL, NULL);
+  EXPECT_EQ(combobox, GetFocusManager()->GetFocusedView());
+
   ::SendMessage(tabbed_pane->TestGetNativeControlHWND(), WM_SETFOCUS,
                 NULL, NULL);
   EXPECT_EQ(tabbed_pane, GetFocusManager()->GetFocusedView());