Refactoring some of the NativeViewHost and NativeControl focus management so their consumers don't have to explicitly set the focused view.

BUG=None
TEST=Run all tests. Make sure focus is stored/restored properly in Chrome.

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

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

23 files changed, 268 insertions, 121 deletions

diff --git a/base/win_util.cc b/base/win_util.cc
index 37fc1ae..9b061bf 100644
--- a/base/win_util.cc
+++ b/base/win_util.cc
@@ -279,6 +279,8 @@ bool Subclass(HWND window, WNDPROC subclass_proc) {
   WNDPROC original_handler =
       reinterpret_cast<WNDPROC>(GetWindowLongPtr(window, GWLP_WNDPROC));
   if (original_handler != subclass_proc) {
+    DCHECK(!GetProp(window, kHandlerKey)) << "subclassing a HWND more than "
+        "once";
     win_util::SetWindowProc(window, subclass_proc);
     SetProp(window, kHandlerKey, original_handler);
     return true;
diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h
index 6a8c6d6..7b29b7f 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.h
+++ b/chrome/browser/autocomplete/autocomplete_edit.h
@@ -47,9 +47,6 @@ class AutocompleteEditController {
   // the edit is guaranteed to be showing the permanent text.
   virtual void OnInputInProgress(bool in_progress) = 0;
 
-  // Called whenever the autocomplete edit gets focused.
-  virtual void OnSetFocus() = 0;
-
   // Returns the favicon of the current page.
   virtual SkBitmap GetFavIcon() const = 0;
 
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view.h b/chrome/browser/autocomplete/autocomplete_edit_view.h
index cba7342..cdca4c8 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view.h
@@ -128,6 +128,10 @@ class AutocompleteEditView {
 
   // Returns the gfx::NativeView of the edit view.
   virtual gfx::NativeView GetNativeView() const = 0;
+
+  // Returns the gfx::NativeView of the edit view that should receive focus when
+  // the location bar is focused.
+  virtual gfx::NativeView GetFocusNativeView() const = 0;
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_EDIT_VIEW_H_
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
index 6dfb45d..2798fc2 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
@@ -505,6 +505,10 @@ gfx::NativeView AutocompleteEditViewGtk::GetNativeView() const {
   return alignment_.get();
 }
 
+gfx::NativeView AutocompleteEditViewGtk::GetFocusNativeView() const {
+  return text_view_;
+}
+
 void AutocompleteEditViewGtk::Observe(NotificationType type,
                                       const NotificationSource& source,
                                       const NotificationDetails& details) {
@@ -756,7 +760,6 @@ gboolean AutocompleteEditViewGtk::HandleViewFocusIn() {
   GdkModifierType modifiers;
   gdk_window_get_pointer(text_view_->window, NULL, NULL, &modifiers);
   model_->OnSetFocus((modifiers & GDK_CONTROL_MASK) != 0);
-  controller_->OnSetFocus();
   // TODO(deanm): Some keyword hit business, etc here.
 
   return FALSE;  // Continue propagation.
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
index 3f5c87a..b9c9280 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
@@ -110,6 +110,7 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
   virtual void OnBeforePossibleChange();
   virtual bool OnAfterPossibleChange();
   virtual gfx::NativeView GetNativeView() const;
+  virtual gfx::NativeView GetFocusNativeView() const;
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
index 56d7cee..b293d8b 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
@@ -89,6 +89,7 @@ class AutocompleteEditViewMac : public AutocompleteEditView,
   virtual void OnBeforePossibleChange();
   virtual bool OnAfterPossibleChange();
   virtual gfx::NativeView GetNativeView() const;
+  virtual gfx::NativeView GetFocusNativeView() const;
 
   // Implement the AutocompleteTextFieldObserver interface.
   virtual void OnControlKeyChanged(bool pressed);
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
index ed26081..051e7ac 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
@@ -550,6 +550,10 @@ gfx::NativeView AutocompleteEditViewMac::GetNativeView() const {
   return field_;
 }
 
+gfx::NativeView AutocompleteEditViewMac::GetFocusNativeView() const {
+  return field_;
+}
+
 void AutocompleteEditViewMac::OnUpOrDownKeyPressed(bool up, bool by_page) {
   // We should only arrive here when the field is focussed.
   DCHECK(IsFirstResponder());
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
index e6643f3..1ada16a 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
@@ -869,6 +869,10 @@ gfx::NativeView AutocompleteEditViewWin::GetNativeView() const {
   return m_hWnd;
 }
 
+gfx::NativeView AutocompleteEditViewWin::GetFocusNativeView() const {
+  return m_hWnd;
+}
+
 void AutocompleteEditViewWin::PasteAndGo(const std::wstring& text) {
   if (CanPasteAndGo(text))
     model_->PasteAndGo();
@@ -1636,15 +1640,6 @@ void AutocompleteEditViewWin::OnPaste() {
 }
 
 void AutocompleteEditViewWin::OnSetFocus(HWND focus_wnd) {
-  views::FocusManager* focus_manager = parent_view_->GetFocusManager();
-  if (focus_manager) {
-    // Notify the FocusManager that the focused view is now the location bar
-    // (our parent view).
-    focus_manager->SetFocusedView(parent_view_);
-  } else {
-    NOTREACHED();
-  }
-
   model_->OnSetFocus(GetKeyState(VK_CONTROL) < 0);
 
   // Notify controller if it needs to show hint UI of some kind.
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.h b/chrome/browser/autocomplete/autocomplete_edit_view_win.h
index 8946c4c..a4cf616 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_win.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.h
@@ -120,6 +120,7 @@ class AutocompleteEditViewWin
   virtual void OnBeforePossibleChange();
   virtual bool OnAfterPossibleChange();
   virtual gfx::NativeView GetNativeView() const;
+  virtual gfx::NativeView GetFocusNativeView() const;
 
   // Exposes custom IAccessible implementation to the overall MSAA hierarchy.
   IAccessible* GetIAccessible();
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 4becb2f..0bff973 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -157,6 +157,8 @@ void LocationBarView::Init() {
   location_entry_view_->SetID(VIEW_ID_AUTOCOMPLETE);
   AddChildView(location_entry_view_);
   location_entry_view_->set_focus_view(this);
+  location_entry_view_->set_focus_native_view(location_entry_->
+        GetFocusNativeView());
   location_entry_view_->Attach(
 #if defined(OS_WIN)
                                location_entry_->m_hWnd
@@ -290,8 +292,9 @@ void LocationBarView::InvalidatePageActions() {
 }
 
 void LocationBarView::Focus() {
-  // Focus the location entry native view.
-  location_entry_->SetFocus();
+  // Forward the focus to the NativeViewHost that will focus the right
+  // native-view.
+  location_entry_view_->Focus();
 }
 
 void LocationBarView::SetProfile(Profile* profile) {
@@ -417,15 +420,6 @@ void LocationBarView::OnChanged() {
   DoLayout(false);
 }
 
-void LocationBarView::OnSetFocus() {
-  views::FocusManager* focus_manager = GetFocusManager();
-  if (!focus_manager) {
-    NOTREACHED();
-    return;
-  }
-  focus_manager->SetFocusedView(this);
-}
-
 SkBitmap LocationBarView::GetFavIcon() const {
   DCHECK(delegate_);
   DCHECK(delegate_->GetTabContents());
diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h
index 343e7e6..e60edc9 100644
--- a/chrome/browser/views/location_bar_view.h
+++ b/chrome/browser/views/location_bar_view.h
@@ -125,7 +125,6 @@ class LocationBarView : public LocationBar,
   virtual void OnInputInProgress(bool in_progress) {
     delegate_->OnInputInProgress(in_progress);
   }
-  virtual void OnSetFocus();
   virtual SkBitmap GetFavIcon() const;
   virtual std::wstring GetTitle() const;
 
diff --git a/views/controls/native/native_view_host.cc b/views/controls/native/native_view_host.cc
index 9cbeaf7..b5eb509 100644
--- a/views/controls/native/native_view_host.cc
+++ b/views/controls/native/native_view_host.cc
@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "app/gfx/canvas.h"
 #include "views/controls/native/native_view_host_wrapper.h"
+#include "views/focus/focus_manager.h"
 #include "views/widget/widget.h"
 
 namespace views {
@@ -20,7 +21,10 @@ const char NativeViewHost::kViewClassName[] = "views/NativeViewHost";
 NativeViewHost::NativeViewHost()
     : native_view_(NULL),
       fast_resize_(false),
-      focus_view_(NULL) {
+      focus_native_view_(NULL) {
+  // By default we make this view the one that should be set as the focused view
+  // when the native view gets the focus.
+  focus_view_ = this;
   // The native widget is placed relative to the root. As such, we need to
   // know when the position of any ancestor changes, or our visibility relative
   // to other views changed as it'll effect our position relative to the root.
@@ -33,10 +37,12 @@ NativeViewHost::~NativeViewHost() {
 void NativeViewHost::Attach(gfx::NativeView native_view) {
   DCHECK(!native_view_);
   native_view_ = native_view;
-  // If set_focus_view() has not been invoked, this view is the one that should
-  // be seen as focused when the native view receives focus.
-  if (!focus_view_)
-    focus_view_ = this;
+  // If this NativeViewHost is the focus view, than it should obviously be
+  // focusable.  If it is not, then it acts as a container and we don't want it
+  // to get focus through tab-traversal, so we make it not focusable.
+  SetFocusable(focus_view_ == this);
+  if (!focus_native_view_)
+    focus_native_view_ = native_view;
   native_wrapper_->NativeViewAttached();
 }
 
@@ -44,6 +50,7 @@ void NativeViewHost::Detach() {
   DCHECK(native_view_);
   native_wrapper_->NativeViewDetaching();
   native_view_ = NULL;
+  focus_native_view_ = NULL;
 }
 
 void NativeViewHost::SetPreferredSize(const gfx::Size& size) {
@@ -57,6 +64,22 @@ void NativeViewHost::NativeViewDestroyed() {
   native_view_ = NULL;
 }
 
+void NativeViewHost::GotNativeFocus() {
+  // Some NativeViewHost may not have an associated focus view.  This is the
+  // case for containers for example.  They might still get the native focus,
+  // if one non native view they contain gets focused.  In that case, we should
+  // not change the focused view.
+  if (!focus_view_ || !focus_view_->IsFocusable())
+    return;
+
+  FocusManager* focus_manager = GetFocusManager();
+  if (!focus_manager) {
+    NOTREACHED();
+    return;
+  }
+  focus_manager->SetFocusedView(focus_view_);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeViewHost, View overrides:
 
@@ -134,7 +157,12 @@ std::string NativeViewHost::GetClassName() const {
 }
 
 void NativeViewHost::Focus() {
-  native_wrapper_->SetFocus();
+  FocusManager* focus_manager = GetFocusManager();
+  if (!focus_manager) {
+    NOTREACHED();
+    return;
+  }
+  focus_manager->FocusNativeView(focus_native_view_);
 }
 
 }  // namespace views
diff --git a/views/controls/native/native_view_host.h b/views/controls/native/native_view_host.h
index 679e176..d27a816e 100644
--- a/views/controls/native/native_view_host.h
+++ b/views/controls/native/native_view_host.h
@@ -42,15 +42,27 @@ class NativeViewHost : public View {
   // Sets a preferred size for the native view attached to this View.
   void SetPreferredSize(const gfx::Size& size);
 
-  // A NativeViewHost has an associated focus View so that the focus of the
-  // native control and of the View are kept in sync. In simple cases where the
-  // NativeViewHost directly wraps a native window as is, the associated view
-  // is this View. In other cases where the NativeViewHost is part of another
-  // view (such as TextField), the actual View is not the NativeViewHost and
-  // this method must be called to set that.
-  // This method must be called before Attach().
+  // A NativeViewHost must keep the focused view in the focus manager and the
+  // native focus with in sync . There are 2 aspects to this:
+  // - when the native view receives focus, the focus manager must be notified
+  //   that the associated view is now the focused view.
+  //   In simple cases where the NativeViewHost directly wraps a native window
+  //   as is, the associated view is this NativeViewHost. In other cases where
+  //   the NativeViewHost is part of another view (such as for the TextField
+  //   class), the actual View is not the NativeViewHost and set_focus_view()
+  //   must be called to set the associated view before Attach() is called.
+  // - when the view is focused (by calling View::RequestFocus()), it must focus
+  //   the appropriate native view.  In simple cases where the native view does
+  //   not have children or is the native view that should really get the focus,
+  //   this works without doing anything.  In case where the native view that
+  //   should get the focus is not the native view passed to Attach(), then
+  //   the appropriate native view should be specified to
+  //   set_focus_native_view() before Attach() is called.
   void set_focus_view(View* view) { focus_view_ = view; }
-  View* focus_view() { return focus_view_; }
+  void set_focus_native_view(gfx::NativeView native_view) {
+    focus_native_view_ = native_view;
+  }
+  gfx::NativeView focus_native_view() const { return focus_native_view_; }
 
   // Fast resizing will move the native view and clip its visible region, this
   // will result in white areas and will not resize the content (so scrollbars
@@ -64,6 +76,10 @@ class NativeViewHost : public View {
   // Accessor for |native_view_|.
   gfx::NativeView native_view() const { return native_view_; }
 
+  // Called by the NativeViewHostWrapper to notify that the |focus_native_view_|
+  // got focus.
+  void GotNativeFocus();
+
   void NativeViewDestroyed();
 
   // Overridden from View:
@@ -96,6 +112,9 @@ class NativeViewHost : public View {
   // The view that should be given focus when this NativeViewHost is focused.
   View* focus_view_;
 
+  // The native view that should get the focus when this View gets focused.
+  gfx::NativeView focus_native_view_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeViewHost);
 };
 
diff --git a/views/controls/native/native_view_host_gtk.cc b/views/controls/native/native_view_host_gtk.cc
index ea9bd62..4b9fc0c 100644
--- a/views/controls/native/native_view_host_gtk.cc
+++ b/views/controls/native/native_view_host_gtk.cc
@@ -48,7 +48,7 @@ void NativeViewHostGtk::NativeViewAttached() {
   }
 
   if (!focus_signal_id_) {
-    focus_signal_id_ = g_signal_connect(G_OBJECT(host_->native_view()),
+    focus_signal_id_ = g_signal_connect(G_OBJECT(host_->focus_native_view()),
                                         "focus-in-event",
                                         G_CALLBACK(CallFocusIn), this);
   }
@@ -70,7 +70,7 @@ void NativeViewHostGtk::NativeViewDetaching() {
                               destroy_signal_id_);
   destroy_signal_id_ = 0;
 
-  g_signal_handler_disconnect(G_OBJECT(host_->native_view()),
+  g_signal_handler_disconnect(G_OBJECT(host_->focus_native_view()),
                               focus_signal_id_);
   focus_signal_id_ = 0;
 
@@ -164,11 +164,6 @@ void NativeViewHostGtk::HideWidget() {
   gtk_widget_hide(fixed_);
 }
 
-void NativeViewHostGtk::SetFocus() {
-  DCHECK(host_->native_view());
-  gtk_widget_grab_focus(host_->native_view());
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // NativeViewHostGtk, private:
 
@@ -215,19 +210,11 @@ void NativeViewHostGtk::CallDestroy(GtkObject* object,
 }
 
 // static
-void NativeViewHostGtk::CallFocusIn(GtkWidget* widget,
-                                    GdkEventFocus* event,
-                                    NativeViewHostGtk* host) {
-  FocusManager* focus_manager =
-      FocusManager::GetFocusManagerForNativeView(widget);
-  if (!focus_manager) {
-    // TODO(jcampan): http://crbug.com/21378 Reenable this NOTREACHED() when the
-    // options page is only based on views.
-    // NOTREACHED();
-    NOTIMPLEMENTED();
-    return;
-  }
-  focus_manager->SetFocusedView(host->host_->focus_view());
+gboolean NativeViewHostGtk::CallFocusIn(GtkWidget* widget,
+                                        GdkEventFocus* event,
+                                        NativeViewHostGtk* host) {
+  host->host_->GotNativeFocus();
+  return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/views/controls/native/native_view_host_gtk.h b/views/controls/native/native_view_host_gtk.h
index 81a2a7a1..2e58bef 100644
--- a/views/controls/native/native_view_host_gtk.h
+++ b/views/controls/native/native_view_host_gtk.h
@@ -36,7 +36,6 @@ class NativeViewHostGtk : public NativeViewHostWrapper {
   virtual void UninstallClip();
   virtual void ShowWidget(int x, int y, int w, int h);
   virtual void HideWidget();
-  virtual void SetFocus();
 
  private:
   // Create and Destroy the GtkFixed that performs clipping on our hosted
@@ -55,9 +54,9 @@ class NativeViewHostGtk : public NativeViewHostWrapper {
   static void CallDestroy(GtkObject* object, NativeViewHostGtk* host);
 
   // Invoked from the 'focus-in-event' signal.
-  static void CallFocusIn(GtkWidget* widget,
-                          GdkEventFocus* event,
-                          NativeViewHostGtk* button);
+  static gboolean CallFocusIn(GtkWidget* widget,
+                              GdkEventFocus* event,
+                              NativeViewHostGtk* button);
 
   // Our associated NativeViewHost.
   NativeViewHost* host_;
@@ -86,4 +85,3 @@ class NativeViewHostGtk : public NativeViewHostWrapper {
 }  // namespace views
 
 #endif  // VIEWS_CONTROLS_NATIVE_HOST_VIEW_GTK_H_
-
diff --git a/views/controls/native/native_view_host_win.cc b/views/controls/native/native_view_host_win.cc
index d1a5217..3c05e2a 100644
--- a/views/controls/native/native_view_host_win.cc
+++ b/views/controls/native/native_view_host_win.cc
@@ -6,10 +6,13 @@
 
 #include "app/gfx/canvas.h"
 #include "base/logging.h"
+#include "base/win_util.h"
 #include "views/controls/native/native_view_host.h"
 #include "views/focus/focus_manager.h"
 #include "views/widget/widget.h"
 
+const wchar_t* kNativeViewHostWinKey = L"__NATIVE_VIEW_HOST_WIN__";
+
 namespace views {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -17,7 +20,8 @@ namespace views {
 
 NativeViewHostWin::NativeViewHostWin(NativeViewHost* host)
     : host_(host),
-      installed_clip_(false) {
+      installed_clip_(false),
+      original_wndproc_(NULL) {
 }
 
 NativeViewHostWin::~NativeViewHostWin() {
@@ -37,10 +41,32 @@ 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();
+
+  // Subclass the appropriate HWND to get focus notifications.
+  HWND focus_hwnd = host_->focus_native_view();
+  DCHECK(focus_hwnd == host_->native_view() ||
+         ::IsChild(host_->native_view(), focus_hwnd));
+  original_wndproc_ =
+      win_util::SetWindowProc(focus_hwnd,
+                              &NativeViewHostWin::NativeViewHostWndProc);
+
+  // We use a property to retrieve the NativeViewHostWin from the window
+  // procedure.
+  ::SetProp(focus_hwnd, kNativeViewHostWinKey, this);
 }
 
 void NativeViewHostWin::NativeViewDetaching() {
   installed_clip_ = false;
+
+  // Restore the original Windows procedure.
+  DCHECK(original_wndproc_);
+  WNDPROC wndproc = win_util::SetWindowProc(host_->focus_native_view(),
+                                            original_wndproc_);
+  DCHECK(wndproc == &NativeViewHostWin::NativeViewHostWndProc);
+
+  // Also remove the property, it's not needed anymore.
+  HANDLE h = ::RemoveProp(host_->focus_native_view(), kNativeViewHostWinKey);
+  DCHECK(h == this);
 }
 
 void NativeViewHostWin::AddedToWidget() {
@@ -116,8 +142,22 @@ void NativeViewHostWin::HideWidget() {
                SWP_NOREDRAW | SWP_NOOWNERZORDER);
 }
 
-void NativeViewHostWin::SetFocus() {
-  ::SetFocus(host_->native_view());
+// static
+LRESULT CALLBACK NativeViewHostWin::NativeViewHostWndProc(HWND window,
+                                                          UINT message,
+                                                          WPARAM w_param,
+                                                          LPARAM l_param) {
+  NativeViewHostWin* native_view_host =
+      static_cast<NativeViewHostWin*>(::GetProp(window, kNativeViewHostWinKey));
+  DCHECK(native_view_host);
+
+  if (message == WM_SETFOCUS)
+    native_view_host->host_->GotNativeFocus();
+  if (message == WM_DESTROY)
+    native_view_host->host_->Detach();
+
+  return CallWindowProc(native_view_host->original_wndproc_,
+                        window, message, w_param, l_param);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/views/controls/native/native_view_host_win.h b/views/controls/native/native_view_host_win.h
index 51bc616..6224693 100644
--- a/views/controls/native/native_view_host_win.h
+++ b/views/controls/native/native_view_host_win.h
@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef VIEWS_CONTROLS_HWND_VIEW_H_
-#define VIEWS_CONTROLS_HWND_VIEW_H_
+#ifndef VIEWS_CONTROL_NATIVE_NATIVE_VIEW_HOST_WIN_H_
+#define VIEWS_CONTROL_NATIVE_NATIVE_VIEW_HOST_WIN_H_
+
+#include "windows.h"
 
 #include "base/logging.h"
 #include "views/controls/native/native_view_host_wrapper.h"
@@ -28,9 +30,14 @@ class NativeViewHostWin : public NativeViewHostWrapper {
   virtual void UninstallClip();
   virtual void ShowWidget(int x, int y, int w, int h);
   virtual void HideWidget();
-  virtual void SetFocus();
 
  private:
+  // Our subclass window procedure.
+  static LRESULT CALLBACK NativeViewHostWndProc(HWND window,
+                                                UINT message,
+                                                WPARAM w_param,
+                                                LPARAM l_param);
+
   // Our associated NativeViewHost.
   NativeViewHost* host_;
 
@@ -38,9 +45,12 @@ class NativeViewHostWin : public NativeViewHostWrapper {
   // visible portion of the gfx::NativeView ?
   bool installed_clip_;
 
+  // The window procedure before we subclassed.
+  WNDPROC original_wndproc_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeViewHostWin);
 };
 
 }  // namespace views
 
-#endif  // VIEWS_CONTROLS_HWND_VIEW_H_
+#endif  // VIEWS_CONTROL_NATIVE_NATIVE_VIEW_HOST_WIN_H_
diff --git a/views/controls/native/native_view_host_wrapper.h b/views/controls/native/native_view_host_wrapper.h
index 948f0ce..483aff5 100644
--- a/views/controls/native/native_view_host_wrapper.h
+++ b/views/controls/native/native_view_host_wrapper.h
@@ -51,9 +51,6 @@ class NativeViewHostWrapper {
   // is already hidden.
   virtual void HideWidget() = 0;
 
-  // Sets focus to the gfx::NativeView.
-  virtual void SetFocus() = 0;
-
   // Creates a platform-specific instance of an object implementing this
   // interface.
   static NativeViewHostWrapper* CreateWrapper(NativeViewHost* host);
diff --git a/views/controls/native_control.cc b/views/controls/native_control.cc
index 0c89664..86fd311 100644
--- a/views/controls/native_control.cc
+++ b/views/controls/native_control.cc
@@ -286,8 +286,6 @@ HWND NativeControl::GetNativeControlHWND() {
 }
 
 void NativeControl::NativeControlDestroyed() {
-  if (hwnd_view_)
-    hwnd_view_->Detach();
   container_ = NULL;
 }
 
diff --git a/views/controls/native_control_gtk.cc b/views/controls/native_control_gtk.cc
index 082e610..390e00d 100644
--- a/views/controls/native_control_gtk.cc
+++ b/views/controls/native_control_gtk.cc
@@ -63,26 +63,6 @@ void NativeControlGtk::NativeControlCreated(GtkWidget* native_control) {
 
   // Update the newly created GtkWdigetwith any resident enabled state.
   gtk_widget_set_sensitive(native_view(), IsEnabled());
-
-  // Listen for focus change event to update the FocusManager focused view.
-  g_signal_connect(G_OBJECT(native_control), "focus-in-event",
-                   G_CALLBACK(CallFocusIn), this);
-}
-
-// static
-void NativeControlGtk::CallFocusIn(GtkWidget* widget,
-                                   GdkEventFocus* event,
-                                   NativeControlGtk* control) {
-  FocusManager* focus_manager =
-      FocusManager::GetFocusManagerForNativeView(widget);
-  if (!focus_manager) {
-    // TODO(jcampan): http://crbug.com/21378 Reenable this NOTREACHED() when the
-    // options page is only based on views.
-    // NOTREACHED();
-    NOTIMPLEMENTED();
-    return;
-  }
-  focus_manager->SetFocusedView(control->focus_view());
 }
 
 }  // namespace views
diff --git a/views/controls/native_control_gtk.h b/views/controls/native_control_gtk.h
index ce2f187..69f650d 100644
--- a/views/controls/native_control_gtk.h
+++ b/views/controls/native_control_gtk.h
@@ -36,10 +36,6 @@ class NativeControlGtk : public NativeViewHost {
   virtual void NativeControlCreated(GtkWidget* widget);
 
  private:
-  static void CallFocusIn(GtkWidget* widget,
-                          GdkEventFocus* event,
-                          NativeControlGtk* button);
-
   DISALLOW_COPY_AND_ASSIGN(NativeControlGtk);
 };
 
diff --git a/views/controls/native_control_win.cc b/views/controls/native_control_win.cc
index 9f4d2d6..037066c 100644
--- a/views/controls/native_control_win.cc
+++ b/views/controls/native_control_win.cc
@@ -24,10 +24,9 @@ NativeControlWin::NativeControlWin() {
 NativeControlWin::~NativeControlWin() {
   HWND hwnd = native_view();
   if (hwnd) {
-    // Destroy the hwnd if it still exists. Otherwise we won't have shut things
-    // down correctly, leading to leaking and crashing if another message
-    // comes in for the hwnd.
-    Detach();
+    // Destroy the hwnd if it still exists. Otherwise we won't shut things down
+    // correctly, leading to leaking and crashing if another message comes in
+    // for the hwnd.
     DestroyWindow(hwnd);
   }
 }
@@ -74,9 +73,7 @@ void NativeControlWin::VisibilityChanged(View* starting_from, bool is_visible) {
   if (!is_visible) {
     // We destroy the child control HWND when we become invisible because of the
     // performance cost of maintaining many HWNDs.
-    HWND hwnd = native_view();
-    Detach();
-    DestroyWindow(hwnd);
+    DestroyWindow(native_view());
   } else if (!native_view()) {
     if (GetWidget())
       CreateNativeControl();
@@ -117,14 +114,14 @@ void NativeControlWin::NativeControlCreated(HWND native_control) {
   // Note that we never unset this property. We don't have to.
   SetProp(native_control, kNativeControlWinKey, this);
 
-  // Subclass so we get WM_KEYDOWN and WM_SETFOCUS messages.
+  Attach(native_control);
+  // native_view() is now valid.
+
+  // Subclass so we get WM_KEYDOWN message.
   original_wndproc_ =
       win_util::SetWindowProc(native_control,
                               &NativeControlWin::NativeControlWndProc);
 
-  Attach(native_control);
-  // native_view() is now valid.
-
   // Update the newly created HWND with any resident enabled state.
   EnableWindow(native_view(), IsEnabled());
 
@@ -186,16 +183,10 @@ LRESULT NativeControlWin::NativeControlWndProc(HWND window,
   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.
-    FocusManager* focus_manager = native_control->GetFocusManager();
-    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_);
+    WNDPROC old_wndproc =
+        win_util::SetWindowProc(window, native_control->original_wndproc_);
+    DCHECK(old_wndproc == &NativeControlWin::NativeControlWndProc);
   }
 
   return CallWindowProc(native_control->original_wndproc_, window, message,
diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index 29175ec..75cbd2c 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -165,6 +165,46 @@ class FocusManagerTest : public testing::Test, public WindowDelegate {
   virtual void InitContentView() {
   }
 
+  gfx::NativeView CreateChildNativeView(gfx::NativeView parent) {
+#if defined(OS_WIN)
+    const wchar_t* kChildClassName = L"FocusTestChildClass";
+    WNDCLASS wnd_class = { 0 };
+    if (!::GetClassInfo(::GetModuleHandle(NULL), kChildClassName, &wnd_class)) {
+      // Let's register our dummy class.
+      wnd_class.lpfnWndProc = ::DefWindowProc;
+      wnd_class.hInstance = ::GetModuleHandle(NULL);
+      wnd_class.lpszClassName = kChildClassName;
+      ATOM atom = RegisterClass(&wnd_class);
+    }
+    return ::CreateWindow(kChildClassName, NULL, WS_CHILD, 0, 0, 0, 0, parent,
+                          NULL, NULL, NULL);
+#else
+    GtkWidget* widget =  gtk_link_button_new("stupid button");
+    if (parent)
+      gtk_container_add(GTK_CONTAINER(parent), widget);
+    return widget;
+#endif
+  }
+
+  gfx::NativeView CreateContainerNativeView() {
+#if defined(OS_WIN)
+    const wchar_t* kTopClassName = L"FocusTestTopClass";
+    WNDCLASS wnd_class = { 0 };
+    if (!::GetClassInfo(::GetModuleHandle(NULL), kTopClassName, &wnd_class)) {
+      // Let's register our dummy class.
+      wnd_class.lpfnWndProc = ::DefWindowProc;
+      wnd_class.hInstance = ::GetModuleHandle(NULL);
+      wnd_class.lpszClassName = kTopClassName;
+      ATOM atom = RegisterClass(&wnd_class);
+    }
+    // Create a top window HWND
+    return ::CreateWindow(kTopClassName, NULL, 0, 0, 0, 0, 0, 0,
+                          NULL, NULL, NULL);
+#else
+    return gtk_fixed_new();
+#endif
+  }
+
  protected:
   virtual gfx::Rect bounds() {
     return gfx::Rect(0, 0, 500, 500);
@@ -215,7 +255,9 @@ class BorderView : public NativeViewHost {
  public:
   explicit BorderView(View* child) : child_(child), widget_(NULL) {
     DCHECK(child);
-    SetFocusable(false);
+     // This is a container and no view should get focused when its associated
+    // native view gets the focus.
+    set_focus_view(NULL);
   }
 
   virtual ~BorderView() {}
@@ -816,6 +858,66 @@ TEST_F(FocusManagerTest, FocusNativeControls) {
 }
 #endif
 
+// A simple view we use to contain a NativeViewHost.
+// The only thing it does is not mess with the native focus when focused.
+class NoNativeFocusView : public View {
+ public:
+  NoNativeFocusView() {
+    SetFocusable(true);
+  }
+  virtual void Focus() {
+  }
+};
+
+// Tests that the NativeViewHost class sets the focus View appropriately on the
+// FocusManager.
+TEST_F(FocusManagerTest, FocusNativeViewHost) {
+  {
+    // Test wrapping a simple native view.
+    gfx::NativeView native_view =
+        CreateChildNativeView(window_->GetNativeWindow());
+    NativeViewHost* native_view_host = new NativeViewHost();
+    content_view_->AddChildView(native_view_host);
+    native_view_host->Attach(native_view);
+    FocusNativeView(native_view);
+    EXPECT_EQ(native_view_host, GetFocusManager()->GetFocusedView());
+    GetFocusManager()->ClearFocus();
+  }
+
+  {
+    // Test with nested native views, making sure set_focus_native_view() works.
+    gfx::NativeView top_native_view  = CreateContainerNativeView();
+    gfx::NativeView child_native_view = CreateChildNativeView(top_native_view);
+    NativeViewHost* native_view_host = new NativeViewHost();
+    native_view_host->set_focus_native_view(child_native_view);
+    content_view_->AddChildView(native_view_host);
+    native_view_host->Attach(top_native_view);
+
+    // Focus the top native view, that shouldn't change the focus.
+    // (Note this isn't a case that we expect to happen.)
+    FocusNativeView(top_native_view);
+    EXPECT_EQ(NULL, GetFocusManager()->GetFocusedView());
+    // Focus the inner HWND, the focused view should change.
+    FocusNativeView(child_native_view);
+    EXPECT_EQ(native_view_host, GetFocusManager()->GetFocusedView());
+    GetFocusManager()->ClearFocus();
+  }
+
+  {
+    // Now also make sure set_focused_view() works.
+    gfx::NativeView native_view = CreateChildNativeView(
+        window_->GetNativeWindow());
+    NativeViewHost* native_view_host = new NativeViewHost();
+    NoNativeFocusView* container_view = new NoNativeFocusView();
+    container_view->AddChildView(native_view_host);
+    content_view_->AddChildView(container_view);
+    native_view_host->set_focus_view(container_view);
+    native_view_host->Attach(native_view);
+    FocusNativeView(native_view);
+    EXPECT_EQ(container_view, GetFocusManager()->GetFocusedView());
+  }
+}
+
 // Test that when activating/deactivating the top window, the focus is stored/
 // restored properly.
 TEST_F(FocusManagerTest, FocusStoreRestore) {