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

36 files changed, 202 insertions, 119 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
index 2a58b66..7aac332 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc
@@ -1621,6 +1621,16 @@ void AutocompleteEditViewWin::OnPaste() {
 }
 
 void AutocompleteEditViewWin::OnSetFocus(HWND focus_wnd) {
+  views::FocusManager* focus_manager =
+      views::FocusManager::GetFocusManager(m_hWnd);
+  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/browser.cc b/chrome/browser/browser.cc
index fe267cb..b1007c0 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1851,6 +1851,10 @@ void Browser::ContentsZoomChange(bool zoom_in) {
   ExecuteCommand(zoom_in ? IDC_ZOOM_PLUS : IDC_ZOOM_MINUS);
 }
 
+void Browser::TabContentsFocused(TabContents* tab_content) {
+  window_->TabContentsFocused(tab_content);
+}
+
 bool Browser::IsApplication() const {
   return (type_ & TYPE_APP) != 0;
 }
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 501fdb8..16af168f 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -478,6 +478,7 @@ class Browser : public TabStripModelDelegate,
   virtual void UpdateTargetURL(TabContents* source, const GURL& url);
 
   virtual void ContentsZoomChange(bool zoom_in);
+  virtual void TabContentsFocused(TabContents* tab_content);
   virtual bool IsApplication() const;
   virtual void ConvertContentsToApplication(TabContents* source);
   virtual bool ShouldDisplayURLField();
diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc
index 712f955..3681c1b 100644
--- a/chrome/browser/browser_focus_uitest.cc
+++ b/chrome/browser/browser_focus_uitest.cc
@@ -96,7 +96,7 @@ class TestInterstitialPage : public InterstitialPage {
 };
 }  // namespace
 
-IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
+IN_PROC_BROWSER_TEST_F(BrowserFocusTest, BrowsersRememberFocus) {
   HTTPTestServer* server = StartHTTPServer();
 
   // First we navigate to our test page.
@@ -111,7 +111,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
       views::FocusManager::GetFocusManager(hwnd);
   ASSERT_TRUE(focus_manager);
 
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 
   // Now hide the window, show it again, the focus should not have changed.
@@ -119,7 +119,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
   // using Windows API.
   ::ShowWindow(hwnd, SW_HIDE);
   ::ShowWindow(hwnd, SW_SHOW);
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 
   // Click on the location bar.
@@ -151,7 +151,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
   views::FocusManager* focus_manager2 =
       views::FocusManager::GetFocusManager(hwnd2);
   ASSERT_TRUE(focus_manager2);
-  EXPECT_EQ(browser_view2->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view2->GetTabContentsContainerView(),
             focus_manager2->GetFocusedView());
 
   // Switch to the 1st browser window, focus should still be on the location
@@ -163,7 +163,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
   // Switch back to the second browser, focus should still be on the page.
   browser2->window()->Activate();
   EXPECT_EQ(NULL, focus_manager->GetFocusedView());
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view2->GetTabContentsContainerView(),
             focus_manager2->GetFocusedView());
 
   // Close the 2nd browser to avoid a DCHECK().
@@ -205,9 +205,12 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, TabsRememberFocus) {
       browser()->SelectTabContentsAt(j, true);
 
       // Activate the location bar or the page.
-      views::View* view_to_focus = kFocusPage[i][j] ?
-          browser_view->contents_container()->GetFocusView() :
-          browser_view->GetLocationBarView();
+      views::View* view_to_focus;
+      if (kFocusPage[i][j]) {
+        view_to_focus = browser_view->GetTabContentsContainerView();
+      } else {
+        view_to_focus = browser_view->GetLocationBarView();
+      }
 
       ui_controls::MoveMouseToCenterAndPress(view_to_focus,
                                              ui_controls::LEFT,
@@ -223,9 +226,12 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, TabsRememberFocus) {
       browser()->SelectTabContentsAt(j, true);
 
       // Activate the location bar or the page.
-      views::View* view = kFocusPage[i][j] ?
-          browser_view->contents_container()->GetFocusView() :
-          browser_view->GetLocationBarView();
+      views::View* view;
+      if (kFocusPage[i][j]) {
+        view = browser_view->GetTabContentsContainerView();
+      } else {
+        view = browser_view->GetLocationBarView();
+      }
       EXPECT_EQ(view, focus_manager->GetFocusedView());
     }
   }
@@ -398,7 +404,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversalOnInterstitial) {
       views::FocusManager::GetFocusManager(hwnd);
 
   // Focus should be on the page.
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 
   // Let's show an interstitial.
@@ -493,12 +499,12 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, InterstitialFocus) {
       views::FocusManager::GetFocusManager(hwnd);
 
   // Page should have focus.
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
   EXPECT_TRUE(browser()->GetSelectedTabContents()->render_view_host()->view()->
       HasFocus());
 
-  // Let's show an interstitial.
+  // Let's show an interstitial.erstitial
   TestInterstitialPage* interstitial_page =
       new TestInterstitialPage(browser()->GetSelectedTabContents(),
                                true, GURL("http://interstitial.com"));
@@ -510,7 +516,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, InterstitialFocus) {
   ui_test_utils::RunMessageLoop();
 
   // The interstitial should have focus now.
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
   EXPECT_TRUE(interstitial_page->HasFocus());
 
@@ -518,7 +524,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, InterstitialFocus) {
   interstitial_page->DontProceed();
 
   // Focus should be back on the original page.
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
   EXPECT_TRUE(browser()->GetSelectedTabContents()->render_view_host()->view()->
       HasFocus());
@@ -577,12 +583,12 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FindFocusTest) {
 
   // Set focus to the page.
   ui_controls::MoveMouseToCenterAndPress(
-      browser_view->contents_container()->GetFocusView(),
+      browser_view->GetTabContentsContainerView(),
       ui_controls::LEFT,
       ui_controls::DOWN | ui_controls::UP,
       new MessageLoop::QuitTask());
   ui_test_utils::RunMessageLoop();
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 
   // Now press Ctrl+F again and focus should move to the Find box.
@@ -612,7 +618,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, TabInitialFocus) {
 
   // Open the history tab, focus should be on the tab contents.
   browser()->ShowHistoryTab();
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 
   // Open the new tab, focus should be on the location bar.
@@ -622,6 +628,6 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, TabInitialFocus) {
 
   // Open the download tab, focus should be on the tab contents.
   browser()->ShowDownloadsTab();
-  EXPECT_EQ(browser_view->contents_container()->GetFocusView(),
+  EXPECT_EQ(browser_view->GetTabContentsContainerView(),
             focus_manager->GetFocusedView());
 }
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index 011cab1..2b3ed63 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -16,6 +16,7 @@ class LocationBar;
 class HtmlDialogUIDelegate;
 class StatusBubble;
 class TabContents;
+class TabContentsContainer;
 
 namespace gfx {
 class Rect;
@@ -190,6 +191,10 @@ class BrowserWindow {
   // during infobar animations).
   virtual int GetExtraRenderViewHeight() const = 0;
 
+  // Notification that |tab_contents| got the focus through user action (click
+  // on the page).
+  virtual void TabContentsFocused(TabContents* tab_contents) = 0;
+
   // Construct a BrowserWindow implementation for the specified |browser|.
   static BrowserWindow* CreateBrowserWindow(Browser* browser);
 
@@ -217,6 +222,9 @@ class BrowserWindowTesting {
 
   // Returns the LocationBarView.
   virtual LocationBarView* GetLocationBarView() const = 0;
+
+  // Returns the TabContentsContainer.
+  virtual views::View* GetTabContentsContainerView() const = 0;
 #endif
 };
 
diff --git a/chrome/browser/cocoa/browser_window_cocoa.h b/chrome/browser/cocoa/browser_window_cocoa.h
index 69b9715..9dc9e72 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.h
+++ b/chrome/browser/cocoa/browser_window_cocoa.h
@@ -71,6 +71,7 @@ class BrowserWindowCocoa : public BrowserWindow,
                               gfx::NativeWindow parent_window);
   virtual void UserChangedTheme();
   virtual int GetExtraRenderViewHeight() const;
+  virtual void TabContentsFocused(TabContents* tab_contents);
 
   // Overridden from NotificationObserver
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm
index d052f61..f295167 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.mm
+++ b/chrome/browser/cocoa/browser_window_cocoa.mm
@@ -249,6 +249,10 @@ int BrowserWindowCocoa::GetExtraRenderViewHeight() const {
   return 0;
 }
 
+void BrowserWindowCocoa::TabContentsFocused(TabContents* tab_contents) {
+  NOTIMPLEMENTED();
+}
+
 void BrowserWindowCocoa::Observe(NotificationType type,
                                  const NotificationSource& source,
                                  const NotificationDetails& details) {
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index d78d8bc..aebf7cf 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -183,6 +183,9 @@ void ExtensionHost::StartDragging(const WebDropData& drop_data) {
 void ExtensionHost::UpdateDragCursor(bool is_drop_target) {
 }
 
+void ExtensionHost::GotFocus() {
+}
+
 void ExtensionHost::TakeFocus(bool reverse) {
 }
 
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index 04ee8f3..4634723 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -83,6 +83,7 @@ class ExtensionHost : public RenderViewHostDelegate,
   virtual void ShowContextMenu(const ContextMenuParams& params);
   virtual void StartDragging(const WebDropData& drop_data);
   virtual void UpdateDragCursor(bool is_drop_target);
+  virtual void GotFocus();
   virtual void TakeFocus(bool reverse);
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
   virtual void HandleMouseEvent();
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index bbc2497..91eaf3e 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -686,6 +686,10 @@ int BrowserWindowGtk::GetExtraRenderViewHeight() const {
   return sum;
 }
 
+void BrowserWindowGtk::TabContentsFocused(TabContents* tab_contents) {
+  NOTIMPLEMENTED();
+}
+
 void BrowserWindowGtk::ConfirmBrowserCloseWithPendingDownloads() {
   NOTIMPLEMENTED();
   browser_->InProgressDownloadResponse(false);
diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h
index c253832..ee43808 100644
--- a/chrome/browser/gtk/browser_window_gtk.h
+++ b/chrome/browser/gtk/browser_window_gtk.h
@@ -88,6 +88,7 @@ class BrowserWindowGtk : public BrowserWindow,
                               gfx::NativeWindow parent_window);
   virtual void UserChangedTheme();
   virtual int GetExtraRenderViewHeight() const;
+  virtual void TabContentsFocused(TabContents* tab_contents);
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 5e4fd06..d7d787e 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -664,6 +664,14 @@ void RenderViewHost::MakeNavigateParams(const NavigationEntry& entry,
   params->request_time = base::Time::Now();
 }
 
+void RenderViewHost::GotFocus() {
+  RenderWidgetHost::GotFocus();  // Notifies the renderer it got focus.
+
+  RenderViewHostDelegate::View* view = delegate_->GetViewDelegate();
+  if (view)
+    view->GotFocus();
+}
+
 bool RenderViewHost::CanBlur() const {
   return delegate_->CanBlur();
 }
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index efe3b39..0407e1b 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -418,6 +418,7 @@ class RenderViewHost : public RenderWidgetHost {
   virtual void Shutdown();
   virtual bool IsRenderView() { return true; }
   virtual void OnMessageReceived(const IPC::Message& msg);
+  virtual void GotFocus();
   virtual bool CanBlur() const;
   virtual void ForwardMouseEvent(const WebKit::WebMouseEvent& mouse_event);
   virtual gfx::Rect GetRootWindowResizerRect() const;
diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h
index 8c8ffe1..b91b69b 100644
--- a/chrome/browser/renderer_host/render_view_host_delegate.h
+++ b/chrome/browser/renderer_host/render_view_host_delegate.h
@@ -110,6 +110,9 @@ class RenderViewHostDelegate {
     // |is_drop_target| is true if the mouse is over a valid drop target.
     virtual void UpdateDragCursor(bool is_drop_target) = 0;
 
+    // Notification that view for this delegate got the focus.
+    virtual void GotFocus() = 0;
+
     // Callback to inform the browser it should take back focus. If reverse is
     // true, it means the focus was retrieved by doing a Shift-Tab.
     virtual void TakeFocus(bool reverse) = 0;
diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index fb6303b..fccd45d 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -188,6 +188,10 @@ void RenderWidgetHost::WasResized() {
     resize_ack_pending_ = false;
 }
 
+void RenderWidgetHost::GotFocus() {
+  Focus();
+}
+
 void RenderWidgetHost::Focus() {
   Send(new ViewMsg_SetFocus(routing_id_, true));
 }
diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h
index 8ac2376..0645496 100644
--- a/chrome/browser/renderer_host/render_widget_host.h
+++ b/chrome/browser/renderer_host/render_widget_host.h
@@ -165,6 +165,11 @@ class RenderWidgetHost : public IPC::Channel::Listener {
   // Called to notify the RenderWidget that it has been resized.
   void WasResized();
 
+  // Called to notify the RenderWidget that its associated native window got
+  // focused.
+  virtual void GotFocus();
+
+  // Tells the renderer it got/lost focus.
   void Focus();
   void Blur();
   void LostCapture();
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc
index c786e70..9c33865 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc
@@ -791,7 +791,7 @@ LRESULT RenderWidgetHostViewWin::OnSetCursor(HWND window, UINT hittest_code,
 }
 
 void RenderWidgetHostViewWin::OnSetFocus(HWND window) {
-  render_widget_host_->Focus();
+  render_widget_host_->GotFocus();
 }
 
 void RenderWidgetHostViewWin::OnKillFocus(HWND window) {
diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc
index a461414..15959be 100644
--- a/chrome/browser/tab_contents/interstitial_page.cc
+++ b/chrome/browser/tab_contents/interstitial_page.cc
@@ -83,6 +83,7 @@ class InterstitialPage::InterstitialPageRVHViewDelegate
   virtual void ShowContextMenu(const ContextMenuParams& params);
   virtual void StartDragging(const WebDropData& drop_data);
   virtual void UpdateDragCursor(bool is_drop_target);
+  virtual void GotFocus();
   virtual void TakeFocus(bool reverse);
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
   virtual void HandleMouseEvent();
@@ -210,6 +211,13 @@ void InterstitialPage::Hide() {
     old_view->Show();
   }
 
+  // If the focus was on the interstitial, let's keep it to the page.
+  // (Note that in unit-tests the RVH may not have a view).
+  if (render_view_host_->view() && render_view_host_->view()->HasFocus() &&
+      tab_->render_view_host()->view()) {
+    tab_->render_view_host()->view()->Focus();
+  }
+
   render_view_host_->Shutdown();
   render_view_host_ = NULL;
   if (tab_->interstitial_page())
@@ -519,6 +527,9 @@ void InterstitialPage::InterstitialPageRVHViewDelegate::UpdateDragCursor(
   NOTREACHED() << "InterstitialPage does not support dragging yet.";
 }
 
+void InterstitialPage::InterstitialPageRVHViewDelegate::GotFocus() {
+}
+
 void InterstitialPage::InterstitialPageRVHViewDelegate::UpdatePreferredWidth(
     int pref_width) {
 }
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 6b45c3e..77e29af 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -34,6 +34,7 @@
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
 #include "chrome/browser/renderer_host/web_cache_manager.h"
 #include "chrome/browser/tab_contents/infobar_delegate.h"
+#include "chrome/browser/tab_contents/interstitial_page.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
@@ -822,6 +823,10 @@ void TabContents::Focus() {
 }
 
 void TabContents::FocusThroughTabTraversal(bool reverse) {
+  if (showing_interstitial_page()) {
+    render_manager_.interstitial_page()->FocusThroughTabTraversal(reverse);
+    return;
+  }
   render_view_host()->SetInitialFocus(reverse);
 }
 
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h
index d7820e2..2fdaa7a 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents/tab_contents_delegate.h
@@ -171,6 +171,9 @@ class TabContentsDelegate {
     return false;
   }
 
+  // Notification that |tab_contents| has gained focus.
+  virtual void TabContentsFocused(TabContents* tab_content) { }
+
   // Return much extra vertical space should be allotted to the
   // render view widget during various animations (e.g. infobar closing).
   // This is used to make painting look smoother.
diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
index 8496b03..3443b84 100644
--- a/chrome/browser/tab_contents/tab_contents_view_gtk.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
@@ -264,6 +264,10 @@ void TabContentsViewGtk::UpdateDragCursor(bool is_drop_target) {
   NOTIMPLEMENTED();
 }
 
+void TabContentsViewGtk::GotFocus() {
+  NOTIMPLEMENTED();
+}
+
 // This is called when we the renderer asks us to take focus back (i.e., it has
 // iterated past the last focusable element on the page).
 void TabContentsViewGtk::TakeFocus(bool reverse) {
diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.h b/chrome/browser/tab_contents/tab_contents_view_gtk.h
index 7056ee9..8c85421 100644
--- a/chrome/browser/tab_contents/tab_contents_view_gtk.h
+++ b/chrome/browser/tab_contents/tab_contents_view_gtk.h
@@ -56,6 +56,7 @@ class TabContentsViewGtk : public TabContentsView,
   virtual void ShowContextMenu(const ContextMenuParams& params);
   virtual void StartDragging(const WebDropData& drop_data);
   virtual void UpdateDragCursor(bool is_drop_target);
+  virtual void GotFocus();
   virtual void TakeFocus(bool reverse);
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
 
diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.h b/chrome/browser/tab_contents/tab_contents_view_mac.h
index 91af1a6..5b464b9 100644
--- a/chrome/browser/tab_contents/tab_contents_view_mac.h
+++ b/chrome/browser/tab_contents/tab_contents_view_mac.h
@@ -63,6 +63,7 @@ class TabContentsViewMac : public TabContentsView,
   virtual void ShowContextMenu(const ContextMenuParams& params);
   virtual void StartDragging(const WebDropData& drop_data);
   virtual void UpdateDragCursor(bool is_drop_target);
+  virtual void GotFocus();
   virtual void TakeFocus(bool reverse);
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
 
diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.mm b/chrome/browser/tab_contents/tab_contents_view_mac.mm
index 9bf8e1e..1d2d59a 100644
--- a/chrome/browser/tab_contents/tab_contents_view_mac.mm
+++ b/chrome/browser/tab_contents/tab_contents_view_mac.mm
@@ -161,6 +161,10 @@ void TabContentsViewMac::UpdateDragCursor(bool is_drop_target) {
   NOTIMPLEMENTED();
 }
 
+void TabContentsViewMac::GotFocus() {
+  NOTIMPLEMENTED();
+}
+
 // This is called when we the renderer asks us to take focus back (i.e., it has
 // iterated past the last focusable element on the page).
 void TabContentsViewMac::TakeFocus(bool reverse) {
diff --git a/chrome/browser/tab_contents/tab_contents_view_win.cc b/chrome/browser/tab_contents/tab_contents_view_win.cc
index 966c41e..341eb50 100644
--- a/chrome/browser/tab_contents/tab_contents_view_win.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_win.cc
@@ -232,25 +232,34 @@ void TabContentsViewWin::SizeContents(const gfx::Size& size) {
 }
 
 void TabContentsViewWin::Focus() {
-  HWND container_hwnd = GetNativeView();
-  if (!container_hwnd)
+  views::FocusManager* focus_manager =
+      views::FocusManager::GetFocusManager(GetNativeView());
+
+  if (tab_contents()->interstitial_page()) {
+    tab_contents()->interstitial_page()->Focus();
+    return;
+  }
+
+  if (sad_tab_.get()) {
+    sad_tab_->RequestFocus();
     return;
+  }
 
-  views::FocusManager* focus_manager =
-      views::FocusManager::GetFocusManager(container_hwnd);
-  if (!focus_manager)
-    return;  // During testing we have no focus manager.
-  views::View* v = focus_manager->GetViewForWindow(container_hwnd, true);
-  DCHECK(v);
-  if (v)
-    v->RequestFocus();
+  RenderWidgetHostView* rwhv = tab_contents()->render_widget_host_view();
+  if (rwhv) {
+    ::SetFocus(rwhv->GetNativeView());
+    return;
+  }
+
+  // Default to focusing our HWND.
+  ::SetFocus(GetNativeView());
 }
 
 void TabContentsViewWin::SetInitialFocus() {
   if (tab_contents()->FocusLocationBarByDefault())
     tab_contents()->delegate()->SetFocusToLocationBar();
   else
-    ::SetFocus(GetNativeView());
+    Focus();
 }
 
 void TabContentsViewWin::StoreFocus() {
@@ -316,6 +325,10 @@ void TabContentsViewWin::UpdateDragCursor(bool is_drop_target) {
   drop_target_->set_is_drop_target(is_drop_target);
 }
 
+void TabContentsViewWin::GotFocus() {
+  tab_contents()->delegate()->TabContentsFocused(tab_contents());
+}
+
 void TabContentsViewWin::TakeFocus(bool reverse) {
   if (!tab_contents()->delegate()->TakeFocus(reverse)) {
     views::FocusManager* focus_manager =
@@ -485,19 +498,6 @@ LRESULT TabContentsViewWin::OnReflectedMessage(UINT msg, WPARAM w_param,
   return 0;
 }
 
-void TabContentsViewWin::OnSetFocus(HWND window) {
-  // TODO(jcampan): figure out why removing this prevents tabs opened in the
-  //                background from properly taking focus.
-  // We NULL-check the render_view_host_ here because Windows can send us
-  // messages during the destruction process after it has been destroyed.
-  if (tab_contents()->render_widget_host_view()) {
-    HWND inner_hwnd =
-        tab_contents()->render_widget_host_view()->GetNativeView();
-    if (::IsWindow(inner_hwnd))
-      ::SetFocus(inner_hwnd);
-  }
-}
-
 void TabContentsViewWin::OnVScroll(int scroll_type, short position,
                                    HWND scrollbar) {
   ScrollCommon(WM_VSCROLL, scroll_type, position, scrollbar);
diff --git a/chrome/browser/tab_contents/tab_contents_view_win.h b/chrome/browser/tab_contents/tab_contents_view_win.h
index b4d360b..07bd545 100644
--- a/chrome/browser/tab_contents/tab_contents_view_win.h
+++ b/chrome/browser/tab_contents/tab_contents_view_win.h
@@ -48,6 +48,7 @@ class TabContentsViewWin : public TabContentsView,
   virtual void ShowContextMenu(const ContextMenuParams& params);
   virtual void StartDragging(const WebDropData& drop_data);
   virtual void UpdateDragCursor(bool is_drop_target);
+  virtual void GotFocus();
   virtual void TakeFocus(bool reverse);
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
 
@@ -61,7 +62,6 @@ class TabContentsViewWin : public TabContentsView,
   virtual LRESULT OnMouseRange(UINT msg, WPARAM w_param, LPARAM l_param);
   virtual void OnPaint(HDC junk_dc);
   virtual LRESULT OnReflectedMessage(UINT msg, WPARAM w_param, LPARAM l_param);
-  virtual void OnSetFocus(HWND window);
   virtual void OnVScroll(int scroll_type, short position, HWND scrollbar);
   virtual void OnWindowPosChanged(WINDOWPOS* window_pos);
   virtual void OnSize(UINT param, const WTL::CSize& size);
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 1b1f6f9..1b3e42b 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -895,6 +895,10 @@ int BrowserView::GetExtraRenderViewHeight() const {
   return 0;
 }
 
+void BrowserView::TabContentsFocused(TabContents* tab_contents) {
+  contents_container_->TabContentsFocused(tab_contents);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, BrowserWindowTesting implementation:
 
@@ -906,6 +910,10 @@ LocationBarView* BrowserView::GetLocationBarView() const {
   return toolbar_->location_bar();
 }
 
+views::View* BrowserView::GetTabContentsContainerView() const {
+  return contents_container_->GetFocusView();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, NotificationObserver implementation:
 
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index a6c134b..b6d8dcc 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -185,12 +185,6 @@ class BrowserView : public BrowserWindow,
   void AttachBrowserBubble(BrowserBubble *bubble);
   void DetachBrowserBubble(BrowserBubble *bubble);
 
-#ifdef UNIT_TEST
-  TabContentsContainer* contents_container() const {
-    return contents_container_;
-  }
-#endif
-
   // Overridden from BrowserWindow:
   virtual void Show();
   virtual void SetBounds(const gfx::Rect& bounds);
@@ -237,10 +231,12 @@ class BrowserView : public BrowserWindow,
                               gfx::NativeWindow parent_window);
   virtual void UserChangedTheme();
   virtual int GetExtraRenderViewHeight() const;
+  virtual void TabContentsFocused(TabContents* source);
 
   // Overridden from BrowserWindowTesting:
   virtual BookmarkBarView* GetBookmarkBarView() const;
   virtual LocationBarView* GetLocationBarView() const;
+  virtual views::View* GetTabContentsContainerView() const;
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/views/tab_contents/native_tab_contents_container.h b/chrome/browser/views/tab_contents/native_tab_contents_container.h
index 4d17002..9c540b9 100644
--- a/chrome/browser/views/tab_contents/native_tab_contents_container.h
+++ b/chrome/browser/views/tab_contents/native_tab_contents_container.h
@@ -35,6 +35,9 @@ class NativeTabContentsContainer {
   virtual void RenderViewHostChanged(RenderViewHost* old_host,
                                      RenderViewHost* new_host) = 0;
 
+  // Tells the container that |tab_contents| got the focus.
+  virtual void TabContentsFocused(TabContents* tab_contents) = 0;
+
   // Retrieves the views::View that hosts the TabContents.
   virtual views::View* GetView() = 0;
 };
diff --git a/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.cc b/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.cc
index 08495fc..56fd42a 100644
--- a/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.cc
+++ b/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.cc
@@ -112,6 +112,18 @@ views::View* NativeTabContentsContainerGtk::GetView() {
   return this;
 }
 
+void NativeTabContentsContainerGtk::TabContentsFocused(
+    TabContents* tab_contents) {
+#if defined(OS_WIN)
+  views::FocusManager* focus_manager = GetFocusManager();
+  if (!focus_manager) {
+    NOTREACHED();
+    return;
+  }
+  focus_manager->SetFocusedView(this);
+#endif
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeTabContentsContainerGtk, views::View overrides:
 
diff --git a/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.h b/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.h
index 855faa2..0d4ed05 100644
--- a/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.h
+++ b/chrome/browser/views/tab_contents/native_tab_contents_container_gtk.h
@@ -20,6 +20,7 @@ class NativeTabContentsContainerGtk : public NativeTabContentsContainer,
   virtual void SetFastResize(bool fast_resize);
   virtual void RenderViewHostChanged(RenderViewHost* old_host,
                                      RenderViewHost* new_host);
+  virtual void TabContentsFocused(TabContents* tab_contents);
   virtual views::View* GetView();
 
   // Overridden from views::View:
diff --git a/chrome/browser/views/tab_contents/native_tab_contents_container_win.cc b/chrome/browser/views/tab_contents/native_tab_contents_container_win.cc
index 800bb54..1e4933f 100644
--- a/chrome/browser/views/tab_contents/native_tab_contents_container_win.cc
+++ b/chrome/browser/views/tab_contents/native_tab_contents_container_win.cc
@@ -9,8 +9,6 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/views/tab_contents/tab_contents_container.h"
 #include "views/focus/focus_manager.h"
-#include "views/widget/root_view.h"
-#include "views/widget/widget.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 // NativeTabContentsContainerWin, public:
@@ -33,13 +31,11 @@ void NativeTabContentsContainerWin::AttachContents(TabContents* contents) {
   set_focus_view(this);
 
   Attach(contents->GetNativeView());
-  HWND contents_hwnd = contents->GetContentNativeView();
-  if (contents_hwnd)
-    views::FocusManager::InstallFocusSubclass(contents_hwnd, this);
 }
 
 void NativeTabContentsContainerWin::DetachContents(TabContents* contents) {
-  // TODO(brettw) should this move to NativeViewHost::Detach which is called below?
+  // TODO(brettw) should this move to NativeViewHost::Detach which is called
+  // below?
   // It needs cleanup regardless.
   HWND container_hwnd = contents->GetNativeView();
   if (container_hwnd) {
@@ -49,16 +45,6 @@ void NativeTabContentsContainerWin::DetachContents(TabContents* contents) {
 
     // Reset the parent to NULL to ensure hidden tabs don't receive messages.
     ::SetParent(container_hwnd, NULL);
-
-    // Unregister the tab contents window from the FocusManager.
-    views::FocusManager::UninstallFocusSubclass(container_hwnd);
-  }
-
-  HWND hwnd = contents->GetContentNativeView();
-  if (hwnd) {
-    // We may not have an HWND anymore, if the renderer crashed and we are
-    // displaying the sad tab for example.
-    views::FocusManager::UninstallFocusSubclass(hwnd);
   }
 
   // Now detach the TabContents.
@@ -72,20 +58,8 @@ void NativeTabContentsContainerWin::SetFastResize(bool fast_resize) {
 void NativeTabContentsContainerWin::RenderViewHostChanged(
     RenderViewHost* old_host,
     RenderViewHost* new_host) {
-  if (old_host && old_host->view()) {
-    views::FocusManager::UninstallFocusSubclass(
-        old_host->view()->GetNativeView());
-  }
-
-  if (new_host && new_host->view()) {
-    views::FocusManager::InstallFocusSubclass(
-        new_host->view()->GetNativeView(), this);
-  }
-
   // If we are focused, we need to pass the focus to the new RenderViewHost.
-  views::FocusManager* focus_manager = views::FocusManager::GetFocusManager(
-      GetRootView()->GetWidget()->GetNativeView());
-  if (focus_manager->GetFocusedView() == this)
+  if (GetFocusManager()->GetFocusedView() == this)
     Focus();
 }
 
@@ -93,12 +67,22 @@ views::View* NativeTabContentsContainerWin::GetView() {
   return this;
 }
 
+void NativeTabContentsContainerWin::TabContentsFocused(
+    TabContents* tab_contents) {
+  views::FocusManager* focus_manager = GetFocusManager();
+  if (!focus_manager) {
+    NOTREACHED();
+    return;
+  }
+  focus_manager->SetFocusedView(this);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeTabContentsContainerWin, views::View overrides:
 
 bool NativeTabContentsContainerWin::SkipDefaultKeyEventProcessing(
     const views::KeyEvent& e) {
-  // Don't look-up accelerators or tab-traverse if we are showing a non-crashed
+  // Don't look-up accelerators or tab-traversal if we are showing a non-crashed
   // TabContents.
   // We'll first give the page a chance to process the key events.  If it does
   // not process them, they'll be returned to us and we'll treat them as
@@ -107,10 +91,6 @@ bool NativeTabContentsContainerWin::SkipDefaultKeyEventProcessing(
          !container_->tab_contents()->is_crashed();
 }
 
-views::FocusTraversable* NativeTabContentsContainerWin::GetFocusTraversable() {
-  return NULL;
-}
-
 bool NativeTabContentsContainerWin::IsFocusable() const {
   // We need to be focusable when our contents is not a view hierarchy, as
   // clicking on the contents needs to focus us.
@@ -118,40 +98,24 @@ bool NativeTabContentsContainerWin::IsFocusable() const {
 }
 
 void NativeTabContentsContainerWin::Focus() {
-  if (container_->tab_contents()) {
-    // Set the native focus on the actual content of the tab, that is the
-    // interstitial if one is showing.
-    if (container_->tab_contents()->interstitial_page()) {
-      container_->tab_contents()->interstitial_page()->Focus();
-      return;
-    }
-    SetFocus(container_->tab_contents()->GetContentNativeView());
-  }
+  if (container_->tab_contents())
+    container_->tab_contents()->Focus();
 }
 
 void NativeTabContentsContainerWin::RequestFocus() {
-  // This is a hack to circumvent the fact that a view does not explicitly get
-  // a call to set the focus if it already has the focus. This causes a problem
-  // with tabs such as the TabContents that instruct the RenderView that it got
-  // focus when they actually get the focus. When switching from one TabContents
-  // tab that has focus to another TabContents tab that had focus, since the
-  // TabContentsContainerView already has focus, Focus() would not be called and
-  // the RenderView would not get notified it got focused.
-  // By clearing the focused view before-hand, we ensure Focus() will be called.
-  GetRootView()->FocusView(NULL);
+  // This is a hack to circumvent the fact that a the Focus() method is not
+  // invoked when RequestFocus() is called on an already focused view.
+  // The TabContentsContainer is the view focused when the TabContents has
+  // focus.  When switching between from one tab that has focus to another tab
+  // that should also have focus, RequestFocus() is invoked one the
+  // TabContentsContainer.  In order to make sure Focus() is invoked we need to
+  // clear the focus before hands.
+  GetFocusManager()->ClearFocus();
   View::RequestFocus();
 }
 
 void NativeTabContentsContainerWin::AboutToRequestFocusFromTabTraversal(
     bool reverse) {
-  if (!container_->tab_contents())
-    return;
-  // Give an opportunity to the tab to reset its focus.
-  if (container_->tab_contents()->interstitial_page()) {
-    container_->tab_contents()->interstitial_page()->
-        FocusThroughTabTraversal(reverse);
-    return;
-  }
   container_->tab_contents()->FocusThroughTabTraversal(reverse);
 }
 
diff --git a/chrome/browser/views/tab_contents/native_tab_contents_container_win.h b/chrome/browser/views/tab_contents/native_tab_contents_container_win.h
index 301c26b..5fb5700 100644
--- a/chrome/browser/views/tab_contents/native_tab_contents_container_win.h
+++ b/chrome/browser/views/tab_contents/native_tab_contents_container_win.h
@@ -20,11 +20,11 @@ class NativeTabContentsContainerWin : public NativeTabContentsContainer,
   virtual void SetFastResize(bool fast_resize);
   virtual void RenderViewHostChanged(RenderViewHost* old_host,
                                      RenderViewHost* new_host);
+  virtual void TabContentsFocused(TabContents* tab_contents);
   virtual views::View* GetView();
 
   // Overridden from views::View:
   virtual bool SkipDefaultKeyEventProcessing(const views::KeyEvent& e);
-  virtual views::FocusTraversable* GetFocusTraversable();
   virtual bool IsFocusable() const;
   virtual void Focus();
   virtual void RequestFocus();
diff --git a/chrome/browser/views/tab_contents/tab_contents_container.cc b/chrome/browser/views/tab_contents/tab_contents_container.cc
index 8ffe870..5e1d2b0 100644
--- a/chrome/browser/views/tab_contents/tab_contents_container.cc
+++ b/chrome/browser/views/tab_contents/tab_contents_container.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/views/tab_contents/tab_contents_container.h"
 
+#include "chrome/browser/tab_contents/interstitial_page.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/view_ids.h"
 #include "chrome/browser/views/tab_contents/native_tab_contents_container.h"
@@ -38,6 +39,10 @@ void TabContentsContainer::ChangeTabContents(TabContents* contents) {
   }
 }
 
+void TabContentsContainer::TabContentsFocused(TabContents* tab_contents) {
+  native_container_->TabContentsFocused(tab_contents);
+}
+
 void TabContentsContainer::SetFastResize(bool fast_resize) {
   native_container_->SetFastResize(fast_resize);
 }
diff --git a/chrome/browser/views/tab_contents/tab_contents_container.h b/chrome/browser/views/tab_contents/tab_contents_container.h
index aba2565..d7a8afe 100644
--- a/chrome/browser/views/tab_contents/tab_contents_container.h
+++ b/chrome/browser/views/tab_contents/tab_contents_container.h
@@ -5,9 +5,7 @@
 #ifndef CHROME_BROWSER_VIEWS_TAB_CONTENTS_TAB_CONTENTS_CONTAINER_H_
 #define CHROME_BROWSER_VIEWS_TAB_CONTENTS_TAB_CONTENTS_CONTAINER_H_
 
-#ifdef UNIT_TEST
 #include "chrome/browser/views/tab_contents/native_tab_contents_container.h"
-#endif
 #include "chrome/common/notification_registrar.h"
 #include "views/view.h"
 
@@ -24,12 +22,13 @@ class TabContentsContainer : public views::View,
   // Changes the TabContents associated with this view.
   void ChangeTabContents(TabContents* contents);
 
+  View* GetFocusView() { return native_container_->GetView(); }
+
   // Accessor for |tab_contents_|.
   TabContents* tab_contents() const { return tab_contents_; }
 
-#ifdef UNIT_TEST
-  View* GetFocusView() { return native_container_->GetView(); }
-#endif
+  // Called by the BrowserView to notify that |tab_contents| got the focus.
+  void TabContentsFocused(TabContents* tab_contents);
 
   // Tells the container to update less frequently during resizing operations
   // so performance is better.
@@ -42,8 +41,8 @@ class TabContentsContainer : public views::View,
 
   // Overridden from views::View:
   virtual void Layout();
- protected:
 
+ protected:
   // Overridden from views::View:
   virtual void ViewHierarchyChanged(bool is_add, views::View* parent,
                                     views::View* child);
diff --git a/chrome/test/test_browser_window.h b/chrome/test/test_browser_window.h
index ccafcd6a..0e25938 100644
--- a/chrome/test/test_browser_window.h
+++ b/chrome/test/test_browser_window.h
@@ -66,6 +66,8 @@ class TestBrowserWindow : public BrowserWindow {
                               gfx::NativeWindow parent_window) {}
   virtual void UserChangedTheme() {}
   virtual int GetExtraRenderViewHeight() const { return 0; }
+  virtual void TabContentsFocused(TabContents* tab_contents) { }
+
  protected:
   virtual void DestroyBrowser() {}