Changing the focus manager to not subclass HWNDs (but for the top-windows).Components that have HWND now need to specifically let the FocusManager know when they get the native focus.This is the reason for the new GotFocus() notification on the RenderWidgetHostViewWin class.BUG=NoneTEST=Run the interactive tests, the unit-tests. Test that the focus is remembered correctly when switching windows, switching tabs. Test that focus traversal in the browser and in the option dialog works as expected.

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

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

10 files changed, 73 insertions, 44 deletions

diff --git a/views/controls/native/native_view_host_gtk.cc b/views/controls/native/native_view_host_gtk.cc
index 4b88067c..7f59144 100644
--- a/views/controls/native/native_view_host_gtk.cc
+++ b/views/controls/native/native_view_host_gtk.cc
@@ -71,8 +71,6 @@ void NativeViewHostGtk::NativeViewAttached() {
   host_->Layout();
 
   // TODO(port): figure out focus.
-  // FocusManager::InstallFocusSubclass(
-  // hwnd, associated_focus_view()_ ? associated_focus_view() : this);
 }
 
 void NativeViewHostGtk::NativeViewDetaching() {
diff --git a/views/controls/native/native_view_host_win.cc b/views/controls/native/native_view_host_win.cc
index 04c9edc..d1a5217 100644
--- a/views/controls/native/native_view_host_win.cc
+++ b/views/controls/native/native_view_host_win.cc
@@ -37,16 +37,9 @@ void NativeViewHostWin::NativeViewAttached() {
   // Need to set the HWND's parent before changing its size to avoid flashing.
   SetParent(host_->native_view(), host_->GetWidget()->GetNativeView());
   host_->Layout();
-
-  // Register with the focus manager so the associated view is focused when the
-  // native control gets the focus.
-  View* focus_view = host_->focus_view() ? host_->focus_view() : host_;
-  FocusManager::InstallFocusSubclass(host_->native_view(), focus_view);
 }
 
 void NativeViewHostWin::NativeViewDetaching() {
-  DCHECK(host_->native_view());
-  FocusManager::UninstallFocusSubclass(host_->native_view());
   installed_clip_ = false;
 }
 
diff --git a/views/controls/native_control.cc b/views/controls/native_control.cc
index cbb375a..2df961e 100644
--- a/views/controls/native_control.cc
+++ b/views/controls/native_control.cc
@@ -81,8 +81,6 @@ class NativeControlContainer : public CWindowImpl<NativeControlContainer,
   LRESULT OnCreate(LPCREATESTRUCT create_struct) {
     control_ = parent_->CreateNativeControl(m_hWnd);
 
-    FocusManager::InstallFocusSubclass(control_, parent_);
-
     // We subclass the control hwnd so we get the WM_KEYDOWN messages.
     WNDPROC original_handler =
         win_util::SetWindowProc(control_,
@@ -367,6 +365,15 @@ LRESULT CALLBACK NativeControl::NativeControlWndProc(HWND window, UINT message,
   if (message == WM_KEYDOWN && native_control->NotifyOnKeyDown()) {
     if (native_control->OnKeyDown(static_cast<int>(w_param)))
       return 0;
+  } else if (message == WM_SETFOCUS) {
+    // Let the focus manager know that the focus changed.
+    FocusManager* focus_manager =
+        FocusManager::GetFocusManager(native_control->GetNativeControlHWND());
+    if (focus_manager) {
+      focus_manager->SetFocusedView(native_control);
+    } else {
+      NOTREACHED();
+    }
   } else if (message == WM_DESTROY) {
     win_util::SetWindowProc(window,
                             reinterpret_cast<WNDPROC>(original_handler));
diff --git a/views/controls/native_control_win.cc b/views/controls/native_control_win.cc
index 2d53f28..b6111fd 100644
--- a/views/controls/native_control_win.cc
+++ b/views/controls/native_control_win.cc
@@ -7,6 +7,7 @@
 #include "app/l10n_util_win.h"
 #include "base/logging.h"
 #include "base/win_util.h"
+#include "views/focus/focus_manager.h"
 
 namespace views {
 
@@ -193,6 +194,15 @@ LRESULT NativeControlWin::NativeControlWndProc(HWND window,
   if (message == WM_KEYDOWN && native_control->NotifyOnKeyDown()) {
     if (native_control->OnKeyDown(static_cast<int>(w_param)))
       return 0;
+  } else if (message == WM_SETFOCUS) {
+    // Let the focus manager know that the focus changed.
+    FocusManager* focus_manager =
+        FocusManager::GetFocusManager(native_control->native_view());
+    if (focus_manager) {
+      focus_manager->SetFocusedView(native_control->focus_view());
+    } else {
+      NOTREACHED();
+    }
   } else if (message == WM_DESTROY) {
     win_util::SetWindowProc(window, native_control->original_wndproc_);
   }
diff --git a/views/controls/textfield/native_textfield_win.cc b/views/controls/textfield/native_textfield_win.cc
index ff20496..d2044a5 100644
--- a/views/controls/textfield/native_textfield_win.cc
+++ b/views/controls/textfield/native_textfield_win.cc
@@ -16,6 +16,7 @@
 #include "views/controls/native/native_view_host.h"
 #include "views/controls/textfield/native_textfield_win.h"
 #include "views/controls/textfield/textfield.h"
+#include "views/focus/focus_manager.h"
 #include "views/focus/focus_util_win.h"
 #include "views/views_delegate.h"
 #include "views/widget/widget.h"
@@ -667,6 +668,18 @@ void NativeTextfieldWin::OnPaste() {
   }
 }
 
+void NativeTextfieldWin::OnSetFocus(HWND hwnd) {
+  SetMsgHandled(FALSE);  // We still want the default processing of the message.
+
+  views::FocusManager* focus_manager =
+      views::FocusManager::GetFocusManager(m_hWnd);
+  if (!focus_manager) {
+    NOTREACHED();
+    return;
+  }
+  focus_manager->SetFocusedView(textfield_);
+}
+
 void NativeTextfieldWin::OnSysChar(TCHAR ch, UINT repeat_count, UINT flags) {
   // Nearly all alt-<xxx> combos result in beeping rather than doing something
   // useful, so we discard most.  Exceptions:
diff --git a/views/controls/textfield/native_textfield_win.h b/views/controls/textfield/native_textfield_win.h
index 194614a..58034c0 100644
--- a/views/controls/textfield/native_textfield_win.h
+++ b/views/controls/textfield/native_textfield_win.h
@@ -74,6 +74,7 @@ class NativeTextfieldWin
     MSG_WM_NCPAINT(OnNCPaint)
     MSG_WM_RBUTTONDOWN(OnNonLButtonDown)
     MSG_WM_PASTE(OnPaste)
+    MSG_WM_SETFOCUS(OnSetFocus)
     MSG_WM_SYSCHAR(OnSysChar)  // WM_SYSxxx == WM_xxx with ALT down
     MSG_WM_SYSKEYDOWN(OnKeyDown)
   END_MSG_MAP()
@@ -122,6 +123,7 @@ class NativeTextfieldWin
   void OnNCPaint(HRGN region);
   void OnNonLButtonDown(UINT keys, const CPoint& point);
   void OnPaste();
+  void OnSetFocus(HWND hwnd);
   void OnSysChar(TCHAR ch, UINT repeat_count, UINT flags);
 
   // Helper function for OnChar() and OnKeyDown() that handles keystrokes that
diff --git a/views/focus/focus_manager.cc b/views/focus/focus_manager.cc
index 046d6ea..ab67301 100644
--- a/views/focus/focus_manager.cc
+++ b/views/focus/focus_manager.cc
@@ -71,10 +71,6 @@ static LRESULT CALLBACK FocusWindowCallback(HWND window, UINT message,
   // but that window may not have an associated FocusManager.
   if (focus_manager) {
     switch (message) {
-      case WM_SETFOCUS:
-        if (!focus_manager->OnSetFocus(window))
-          return 0;
-        break;
       case WM_NCDESTROY:
         if (!focus_manager->OnNCDestroy(window))
           return 0;
@@ -189,21 +185,6 @@ FocusManager::~FocusManager() {
 
 #if defined(OS_WIN)
 // Message handlers.
-bool FocusManager::OnSetFocus(HWND window) {
-  if (ignore_set_focus_msg_)
-    return true;
-
-  // Focus the view associated with that window.
-  View* v = static_cast<View*>(GetProp(window, kViewKey));
-  if (v && v->IsFocusable()) {
-    v->GetRootView()->FocusView(v);
-  } else {
-    SetFocusedView(NULL);
-  }
-
-  return true;
-}
-
 bool FocusManager::OnNCDestroy(HWND window) {
   // Window is being destroyed, undo the subclassing.
   FocusManager::UninstallFocusSubclass(window);
diff --git a/views/focus/focus_manager.h b/views/focus/focus_manager.h
index 4c7f6ce..c5e2be9a 100644
--- a/views/focus/focus_manager.h
+++ b/views/focus/focus_manager.h
@@ -160,16 +160,6 @@ class FocusManager {
   // The RootView specified should be the top RootView of the window.
   // This also invokes InstallFocusSubclass.
   static FocusManager* CreateFocusManager(HWND window, RootView* root_view);
-
-  // Subclasses the specified window. The subclassed window procedure listens
-  // for WM_SETFOCUS notification and keeps the FocusManager's focus owner
-  // property in sync.
-  // It's not necessary to explicitly invoke Uninstall, it's automatically done
-  // when the window is destroyed and Uninstall wasn't invoked.
-  static void InstallFocusSubclass(HWND window, View* view);
-
-  // Uninstalls the window subclass installed by InstallFocusSubclass.
-  static void UninstallFocusSubclass(HWND window);
 #endif
 
   static FocusManager* GetFocusManager(gfx::NativeView window);
@@ -294,6 +284,16 @@ class FocusManager {
  private:
 #if defined(OS_WIN)
   explicit FocusManager(HWND root, RootView* root_view);
+
+  // Subclasses the specified window. The subclassed window procedure listens
+  // for WM_SETFOCUS notification and keeps the FocusManager's focus owner
+  // property in sync.
+  // It's not necessary to explicitly invoke Uninstall, it's automatically done
+  // when the window is destroyed and Uninstall wasn't invoked.
+  static void InstallFocusSubclass(HWND window, View* view);
+
+  // Uninstalls the window subclass installed by InstallFocusSubclass.
+  static void UninstallFocusSubclass(HWND window);
 #endif
   ~FocusManager();
 
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index 95257ec..f4142db 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -699,6 +699,24 @@ class TestNativeButton : public NativeButton {
   }
 };
 
+class TestCheckbox : public Checkbox {
+ public:
+  explicit TestCheckbox(const std::wstring& text) : Checkbox(text) {
+  };
+  virtual HWND TestGetNativeControlHWND() {
+    return native_wrapper_->GetTestingHandle();
+  }
+};
+
+class TestRadioButton : public RadioButton {
+ public:
+  explicit TestRadioButton(const std::wstring& text) : RadioButton(text, 1) {
+  };
+  virtual HWND TestGetNativeControlHWND() {
+    return native_wrapper_->GetTestingHandle();
+  }
+};
+
 class TestTextfield : public Textfield {
  public:
   TestTextfield() { }
@@ -719,11 +737,15 @@ class TestTabbedPane : public TabbedPane {
 // FocusManager.
 TEST_F(FocusManagerTest, FocusNativeControls) {
   TestNativeButton* button = new TestNativeButton(L"Press me");
+  TestCheckbox* checkbox = new TestCheckbox(L"Checkbox");
+  TestRadioButton* radio_button = new TestRadioButton(L"RadioButton");
   TestTextfield* textfield = new TestTextfield();
   TestTabbedPane* tabbed_pane = new TestTabbedPane();
   TestNativeButton* tab_button = new TestNativeButton(L"tab button");
 
   content_view_->AddChildView(button);
+  content_view_->AddChildView(checkbox);
+  content_view_->AddChildView(radio_button);
   content_view_->AddChildView(textfield);
   content_view_->AddChildView(tabbed_pane);
   tabbed_pane->AddTab(L"Awesome tab", tab_button);
@@ -732,6 +754,13 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
   ::SendMessage(button->TestGetNativeControlHWND(), WM_SETFOCUS, NULL, NULL);
   EXPECT_EQ(button, GetFocusManager()->GetFocusedView());
 
+  ::SendMessage(checkbox->TestGetNativeControlHWND(), WM_SETFOCUS, NULL, NULL);
+  EXPECT_EQ(checkbox, GetFocusManager()->GetFocusedView());
+
+  ::SendMessage(radio_button->TestGetNativeControlHWND(), WM_SETFOCUS,
+                NULL, NULL);
+  EXPECT_EQ(radio_button, GetFocusManager()->GetFocusedView());
+
   ::SendMessage(textfield->TestGetNativeComponent(), WM_SETFOCUS, NULL, NULL);
   EXPECT_EQ(textfield, GetFocusManager()->GetFocusedView());
 
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 41c7309..7d8aced 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -189,10 +189,6 @@ void WidgetWin::Init(HWND parent, const gfx::Rect& bounds,
 
   if (has_own_focus_manager) {
     FocusManager::CreateFocusManager(hwnd_, GetRootView());
-  } else {
-    // Subclass the window so we get the tab key messages when a view with no
-    // associated native window is focused.
-    FocusManager::InstallFocusSubclass(hwnd_, NULL);
   }
 
   // Sets the RootView as a property, so the automation can introspect windows.