This CL makes the focus remembered across tab switches for Chrome Linux with toolkit views.

It also contains clean-ups and implementation of different minor focus related things.

BUG=None
TEST=Open several tabs. Focus the location bar for some tabs, the page for others. Make sure that when switching between tabs the focus is restored to the last focused place (location bar or page) for each tab.
     Also test that deactivating/reactivating the browser window still restores focus properly.

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


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

4 files changed, 63 insertions, 65 deletions

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 29a6ef2..6eb37c3 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
@@ -17,7 +17,8 @@
 
 NativeTabContentsContainerGtk::NativeTabContentsContainerGtk(
     TabContentsContainer* container)
-    : container_(container) {
+    : container_(container),
+      focus_callback_id_(0) {
 }
 
 NativeTabContentsContainerGtk::~NativeTabContentsContainerGtk() {
@@ -27,48 +28,11 @@ NativeTabContentsContainerGtk::~NativeTabContentsContainerGtk() {
 // NativeTabContentsContainerGtk, NativeTabContentsContainer overrides:
 
 void NativeTabContentsContainerGtk::AttachContents(TabContents* contents) {
-  // We need to register the tab contents window with the BrowserContainer so
-  // that the BrowserContainer is the focused view when the focus is on the
-  // TabContents window (for the TabContents case).
-  set_focus_view(this);
-
   Attach(contents->GetNativeView());
-
-  // TODO(port): figure out focus interception
-#if defined(OS_WIN)
-  HWND contents_hwnd = contents->GetContentNativeView();
-  if (contents_hwnd)
-    views::FocusManager::InstallFocusSubclass(contents_hwnd, this);
-#else
-  NOTIMPLEMENTED();
-#endif
 }
 
 void NativeTabContentsContainerGtk::DetachContents(TabContents* contents) {
-  // TODO(port): figure out focus interception
-#if defined(OS_WIN)
-  // TODO(brettw) should this move to NativeViewHost::Detach which is called
-  // below? It needs cleanup regardless.
-  HWND container_hwnd = contents->GetNativeView();
-
-  // Hide the contents before adjusting its parent to avoid a full desktop
-  // flicker.
-  ShowWindow(container_hwnd, SW_HIDE);
-
-  // 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);
-  }
-#else
   gtk_widget_hide(contents->GetNativeView());
-#endif
 
   // Now detach the TabContents.
   Detach();
@@ -81,31 +45,10 @@ void NativeTabContentsContainerGtk::SetFastResize(bool fast_resize) {
 void NativeTabContentsContainerGtk::RenderViewHostChanged(
     RenderViewHost* old_host,
     RenderViewHost* new_host) {
-  // TODO(port): figure out focus interception
-#if defined(OS_WIN)
-  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());
+  views::FocusManager* focus_manager = GetFocusManager();
   if (focus_manager->GetFocusedView() == this)
     Focus();
-#else
-  // If we are focused, we need to pass the focus to the new RenderViewHost.
-  // TODO: uncomment this once FocusManager has been ported.
-  // views::FocusManager* focus_manager = views::FocusManager::GetFocusManager(
-  // GetRootView()->GetWidget()->GetNativeView());
-  // if (focus_manager->GetFocusedView() == this)
-  // Focus();
-#endif
 }
 
 views::View* NativeTabContentsContainerGtk::GetView() {
@@ -114,14 +57,15 @@ views::View* NativeTabContentsContainerGtk::GetView() {
 
 void NativeTabContentsContainerGtk::TabContentsFocused(
     TabContents* tab_contents) {
-#if defined(OS_WIN)
+  // Called when the tab contents native view gets focused (typically through a
+  // user click).  We make ourself the focused view, so the focus is restored
+  // properly when the browser window is deactivated/reactivated.
   views::FocusManager* focus_manager = GetFocusManager();
   if (!focus_manager) {
     NOTREACHED();
     return;
   }
   focus_manager->SetFocusedView(this);
-#endif
 }
 
 ////////////////////////////////////////////////////////////////////////////////
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 0d4ed05..e30c11f 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
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_VIEWS_TAB_CONTENTS_NATIVE_TAB_CONTENTS_CONTAINER_GTK_H_
 #define CHROME_BROWSER_VIEWS_TAB_CONTENTS_NATIVE_TAB_CONTENTS_CONTAINER_GTK_H_
 
+#include <gtk/gtk.h>
+
 #include "chrome/browser/views/tab_contents/native_tab_contents_container.h"
 #include "views/controls/native/native_view_host.h"
 
@@ -35,6 +37,8 @@ class NativeTabContentsContainerGtk : public NativeTabContentsContainer,
  private:
   TabContentsContainer* container_;
 
+  gulong focus_callback_id_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeTabContentsContainerGtk);
 };
 
diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
index 8ca4d16..e39bfde 100644
--- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
+++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/views/sad_tab_view.h"
 #include "chrome/browser/views/tab_contents/render_view_context_menu_win.h"
+#include "views/focus/view_storage.h"
 #include "views/controls/native/native_view_host.h"
 #include "views/widget/root_view.h"
 
@@ -97,9 +98,19 @@ TabContentsViewGtk::TabContentsViewGtk(TabContents* tab_contents)
       views::WidgetGtk(TYPE_CHILD),
       ignore_next_char_event_(false) {
   drag_source_.reset(new TabContentsDragSource(this));
+  last_focused_view_storage_id_ =
+      views::ViewStorage::GetSharedInstance()->CreateStorageID();
 }
 
 TabContentsViewGtk::~TabContentsViewGtk() {
+  // Make sure to remove any stored view we may still have in the ViewStorage.
+  //
+  // It is possible the view went away before us, so we only do this if the
+  // view is registered.
+  views::ViewStorage* view_storage = views::ViewStorage::GetSharedInstance();
+  if (view_storage->RetrieveView(last_focused_view_storage_id_) != NULL)
+    view_storage->RemoveView(last_focused_view_storage_id_);
+
   // Just deleting the object doesn't destroy the GtkWidget. We need to do that
   // manually, and synchronously, since subsequent signal handlers may expect
   // to locate this object.
@@ -218,12 +229,48 @@ void TabContentsViewGtk::SetInitialFocus() {
 }
 
 void TabContentsViewGtk::StoreFocus() {
-  NOTIMPLEMENTED();
+  views::ViewStorage* view_storage = views::ViewStorage::GetSharedInstance();
+
+  if (view_storage->RetrieveView(last_focused_view_storage_id_) != NULL)
+    view_storage->RemoveView(last_focused_view_storage_id_);
+
+  views::FocusManager* focus_manager =
+      views::FocusManager::GetFocusManagerForNativeView(GetNativeView());
+  if (focus_manager) {
+    // |focus_manager| can be NULL if the tab has been detached but still
+    // exists.
+    views::View* focused_view = focus_manager->GetFocusedView();
+    if (focused_view)
+      view_storage->StoreView(last_focused_view_storage_id_, focused_view);
+  }
 }
 
 void TabContentsViewGtk::RestoreFocus() {
-  NOTIMPLEMENTED();
-  SetInitialFocus();
+  views::ViewStorage* view_storage = views::ViewStorage::GetSharedInstance();
+  views::View* last_focused_view =
+      view_storage->RetrieveView(last_focused_view_storage_id_);
+  if (!last_focused_view) {
+    SetInitialFocus();
+  } else {
+    views::FocusManager* focus_manager =
+        views::FocusManager::GetFocusManagerForNativeView(GetNativeView());
+
+    // If you hit this DCHECK, please report it to Jay (jcampan).
+    DCHECK(focus_manager != NULL) << "No focus manager when restoring focus.";
+
+    if (last_focused_view->IsFocusable() && focus_manager &&
+        focus_manager->ContainsView(last_focused_view)) {
+      last_focused_view->RequestFocus();
+    } else {
+      // The focused view may not belong to the same window hierarchy (e.g.
+      // if the location bar was focused and the tab is dragged out), or it may
+      // no longer be focusable (e.g. if the location bar was focused and then
+      // we switched to fullscreen mode).  In that case we default to the
+      // default focus.
+      SetInitialFocus();
+    }
+    view_storage->RemoveView(last_focused_view_storage_id_);
+  }
 }
 
 void TabContentsViewGtk::UpdateDragCursor(WebDragOperation operation) {
diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h
index 71fbdf6..8788468 100644
--- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h
+++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h
@@ -81,6 +81,9 @@ class TabContentsViewGtk : public TabContentsView,
   // Whether to ignore the next CHAR keyboard event.
   bool ignore_next_char_event_;
 
+  // The id used in the ViewStorage to store the last focused view.
+  int last_focused_view_storage_id_;
+
   // The context menu. Callbacks are asynchronous so we need to keep it around.
   scoped_ptr<RenderViewContextMenuWin> context_menu_;