Linux: Make findbar restore focus to last focused widget on close.

Factor out some common code to FocusStoreGtk so it can be shared by anyone who cares about remembering which widget last had focus. Currently used in FindBarGtk and TabContentsViewGtk.

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

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

9 files changed, 248 insertions, 46 deletions

diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj
index b712615..07b0fb27c 100644
--- a/chrome/browser/browser.vcproj
+++ b/chrome/browser/browser.vcproj
@@ -1870,6 +1870,10 @@
 				>
 			</File>
 			<File
+				RelativePath=".\download\download_started_animation.h"
+				>
+			</File>
+			<File
 				RelativePath=".\download\download_util.cc"
 				>
 			</File>
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 2a2c27d..beba4ae 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -77,6 +77,13 @@ FindBarGtk::FindBarGtk(BrowserWindowGtk* browser)
                    G_CALLBACK(OnChanged), this);
   g_signal_connect(text_entry_, "key-press-event",
                    G_CALLBACK(OnKeyPressEvent), this);
+  // When the user tabs to us or clicks on us, save where the focus used to
+  // be.
+  g_signal_connect(text_entry_, "focus",
+                   G_CALLBACK(OnFocus), this);
+  gtk_widget_add_events(text_entry_, GDK_BUTTON_PRESS_MASK);
+  g_signal_connect(text_entry_, "button-press-event",
+                   G_CALLBACK(OnButtonPress), this);
   g_signal_connect(widget(), "size-allocate",
                    G_CALLBACK(OnFixedSizeAllocate), this);
   // We can't call ContourWidget() until after |container_| has been
@@ -122,14 +129,14 @@ void FindBarGtk::InitWidgets() {
   close_button_.reset(
       CustomDrawButton::AddBarCloseButton(hbox, kCloseButtonPaddingLeft));
   g_signal_connect(G_OBJECT(close_button_->widget()), "clicked",
-                   G_CALLBACK(OnButtonPressed), this);
+                   G_CALLBACK(OnClicked), this);
   gtk_widget_set_tooltip_text(close_button_->widget(),
       l10n_util::GetStringUTF8(IDS_FIND_IN_PAGE_CLOSE_TOOLTIP).c_str());
 
   find_next_button_.reset(new CustomDrawButton(IDR_FINDINPAGE_NEXT,
       IDR_FINDINPAGE_NEXT_H, IDR_FINDINPAGE_NEXT_H, IDR_FINDINPAGE_NEXT_P));
   g_signal_connect(G_OBJECT(find_next_button_->widget()), "clicked",
-                   G_CALLBACK(OnButtonPressed), this);
+                   G_CALLBACK(OnClicked), this);
   gtk_widget_set_tooltip_text(find_next_button_->widget(),
       l10n_util::GetStringUTF8(IDS_FIND_IN_PAGE_NEXT_TOOLTIP).c_str());
   gtk_box_pack_end(GTK_BOX(hbox), find_next_button_->widget(),
@@ -138,7 +145,7 @@ void FindBarGtk::InitWidgets() {
   find_previous_button_.reset(new CustomDrawButton(IDR_FINDINPAGE_PREV,
       IDR_FINDINPAGE_PREV_H, IDR_FINDINPAGE_PREV_H, IDR_FINDINPAGE_PREV_P));
   g_signal_connect(G_OBJECT(find_previous_button_->widget()), "clicked",
-                   G_CALLBACK(OnButtonPressed), this);
+                   G_CALLBACK(OnClicked), this);
   gtk_widget_set_tooltip_text(find_previous_button_->widget(),
       l10n_util::GetStringUTF8(IDS_FIND_IN_PAGE_PREVIOUS_TOOLTIP).c_str());
   gtk_box_pack_end(GTK_BOX(hbox), find_previous_button_->widget(),
@@ -186,7 +193,6 @@ void FindBarGtk::Show() {
   slide_widget_->Open();
   if (container_->window)
     gdk_window_raise(container_->window);
-  gtk_widget_grab_focus(text_entry_);
 }
 
 void FindBarGtk::Hide(bool animate) {
@@ -197,6 +203,7 @@ void FindBarGtk::Hide(bool animate) {
 }
 
 void FindBarGtk::SetFocusAndSelection() {
+  StoreOutsideFocus();
   gtk_widget_grab_focus(text_entry_);
   // Select all the text.
   gtk_entry_select_region(GTK_ENTRY(text_entry_), 0, -1);
@@ -221,6 +228,11 @@ void FindBarGtk::SetFindText(const string16& find_text) {
 
 void FindBarGtk::UpdateUIForFindResult(const FindNotificationDetails& result,
                                        const string16& find_text) {
+  // Once we find a match we no longer want to keep track of what had
+  // focus. EndFindSession will then set the focus to the page content.
+  if (result.number_of_matches() > 0)
+    focus_store_.Store(NULL);
+
   std::string text_entry_utf8 = UTF16ToUTF8(find_text);
   bool have_valid_range =
       result.number_of_matches() != -1 && result.active_match_ordinal() != -1;
@@ -275,15 +287,15 @@ bool FindBarGtk::IsFindBarVisible() {
 }
 
 void FindBarGtk::RestoreSavedFocus() {
-  // TODO(estade): We should save focus and restore its previous location if we
-  // don't find any matches in our search. For now just give focus to the tab
-  // contents.
   // This function sometimes gets called when we don't have focus. We should do
   // nothing in this case.
-  if (!GTK_WIDGET_HAS_FOCUS(text_entry_))
+  if (!gtk_widget_is_focus(text_entry_))
     return;
 
-  find_bar_controller_->tab_contents()->Focus();
+  if (focus_store_.widget())
+    gtk_widget_grab_focus(focus_store_.widget());
+  else
+    find_bar_controller_->tab_contents()->Focus();
 }
 
 FindBarTesting* FindBarGtk::GetFindBarTesting() {
@@ -311,6 +323,15 @@ void FindBarGtk::FindEntryTextInContents(bool forward_search) {
   }
 }
 
+void FindBarGtk::StoreOutsideFocus() {
+  // |text_entry_| is the only widget in the find bar that can be focused,
+  // so it's the only one we have to check.
+  // TODO(estade): when we make the find bar buttons focusable, we'll have
+  // to change this (same above in RestoreSavedFocus).
+  if (!gtk_widget_is_focus(text_entry_))
+    focus_store_.Store(text_entry_);
+}
+
 // static
 gboolean FindBarGtk::OnChanged(GtkWindow* window, FindBarGtk* find_bar) {
   find_bar->FindEntryTextInContents(true);
@@ -331,7 +352,7 @@ gboolean FindBarGtk::OnKeyPressEvent(GtkWindow* window, GdkEventKey* event,
 }
 
 // static
-void FindBarGtk::OnButtonPressed(GtkWidget* button, FindBarGtk* find_bar) {
+void FindBarGtk::OnClicked(GtkWidget* button, FindBarGtk* find_bar) {
   if (button == find_bar->close_button_->widget()) {
     find_bar->find_bar_controller_->EndFindSession();
   } else if (button == find_bar->find_previous_button_->widget() ||
@@ -377,3 +398,21 @@ void FindBarGtk::OnContainerSizeAllocate(GtkWidget* container,
     findbar->container_shaped_ = true;
   }
 }
+
+// static
+gboolean FindBarGtk::OnFocus(GtkWidget* text_entry, GtkDirectionType focus,
+                             FindBarGtk* find_bar) {
+  find_bar->StoreOutsideFocus();
+
+  // Continue propagating the event.
+  return FALSE;
+}
+
+// static
+gboolean FindBarGtk::OnButtonPress(GtkWidget* text_entry, GdkEventButton* e,
+                                   FindBarGtk* find_bar) {
+  find_bar->StoreOutsideFocus();
+
+  // Continue propagating the event.
+  return FALSE;
+}
diff --git a/chrome/browser/gtk/find_bar_gtk.h b/chrome/browser/gtk/find_bar_gtk.h
index 571d763..07171ed 100644
--- a/chrome/browser/gtk/find_bar_gtk.h
+++ b/chrome/browser/gtk/find_bar_gtk.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/find_bar.h"
+#include "chrome/browser/gtk/focus_store_gtk.h"
 #include "chrome/common/owned_widget_gtk.h"
 
 class BrowserWindowGtk;
@@ -59,6 +60,10 @@ class FindBarGtk : public FindBar,
  private:
   void InitWidgets();
 
+  // Store the currently focused widget if it is not in the find bar.
+  // This should always be called before we claim focus.
+  void StoreOutsideFocus();
+
   // Returns the child of |fixed_| that holds what the user perceives as the
   // findbar.
   GtkWidget* slide_widget();
@@ -75,7 +80,7 @@ class FindBarGtk : public FindBar,
                                   FindBarGtk* find_bar);
 
   // Callback for previous, next, and close button.
-  static void OnButtonPressed(GtkWidget* button, FindBarGtk* find_bar);
+  static void OnClicked(GtkWidget* button, FindBarGtk* find_bar);
 
   // Called when |fixed_| changes sizes. Used to position the dialog (the
   // "dialog" is the widget hierarchy rooted at |slide_widget_|).
@@ -88,6 +93,12 @@ class FindBarGtk : public FindBar,
                                       GtkAllocation* allocation,
                                       FindBarGtk* findbar);
 
+  // These are both used for focus management.
+  static gboolean OnFocus(GtkWidget* text_entry, GtkDirectionType focus,
+                          FindBarGtk* find_bar);
+  static gboolean OnButtonPress(GtkWidget* text_entry, GdkEventButton* e,
+                                FindBarGtk* find_bar);
+
   // GtkFixed containing the find bar widgets.
   OwnedWidgetGtk fixed_;
 
@@ -125,6 +136,9 @@ class FindBarGtk : public FindBar,
   // Pointer back to the owning controller.
   FindBarController* find_bar_controller_;
 
+  // Saves where the focus used to be whenever we get it.
+  FocusStoreGtk focus_store_;
+
   DISALLOW_COPY_AND_ASSIGN(FindBarGtk);
 };
 
diff --git a/chrome/browser/gtk/focus_store_gtk.cc b/chrome/browser/gtk/focus_store_gtk.cc
new file mode 100644
index 0000000..82b740f
--- /dev/null
+++ b/chrome/browser/gtk/focus_store_gtk.cc
@@ -0,0 +1,92 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/gtk/focus_store_gtk.h"
+
+#include <gtk/gtk.h>
+
+#include "base/logging.h"
+#include "chrome/common/platform_util.h"
+
+FocusStoreGtk::FocusStoreGtk() : widget_(NULL) {
+}
+
+FocusStoreGtk::~FocusStoreGtk() {
+  DisconnectDestroyHandler();
+}
+
+void FocusStoreGtk::Store(GtkWidget* widget) {
+  if (!widget) {
+    DisconnectDestroyHandler();
+    widget_ = NULL;
+    return;
+  }
+
+  GtkWindow* window = platform_util::GetTopLevel(widget);
+  if (!window) {
+    NOTREACHED();
+    return;
+  }
+
+  DisconnectDestroyHandler();
+
+  widget_ = window->focus_widget;
+  if (widget_) {
+    // When invoked, |gtk_widget_destroyed| will set |widget_| to NULL.
+    destroy_handler_id_ = g_signal_connect(widget_, "destroy",
+                                           G_CALLBACK(gtk_widget_destroyed),
+                                           &widget_);
+  }
+}
+
+void FocusStoreGtk::DisconnectDestroyHandler() {
+  if (widget_)
+    g_signal_handler_disconnect(widget_, destroy_handler_id_);
+}
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/gtk/focus_store_gtk.h"
+
+#include <gtk/gtk.h>
+
+#include "base/logging.h"
+#include "chrome/common/platform_util.h"
+
+FocusStoreGtk::FocusStoreGtk() : widget_(NULL) {
+}
+
+FocusStoreGtk::~FocusStoreGtk() {
+  DisconnectDestroyHandler();
+}
+
+void FocusStoreGtk::Store(GtkWidget* widget) {
+  if (!widget) {
+    DisconnectDestroyHandler();
+    widget_ = NULL;
+    return;
+  }
+
+  GtkWindow* window = platform_util::GetTopLevel(widget);
+  if (!window) {
+    NOTREACHED();
+    return;
+  }
+
+  DisconnectDestroyHandler();
+
+  widget_ = window->focus_widget;
+  if (widget_) {
+    // When invoked, |gtk_widget_destroyed| will set |widget_| to NULL.
+    destroy_handler_id_ = g_signal_connect(widget_, "destroy",
+                                           G_CALLBACK(gtk_widget_destroyed),
+                                           &widget_);
+  }
+}
+
+void FocusStoreGtk::DisconnectDestroyHandler() {
+  if (widget_)
+    g_signal_handler_disconnect(widget_, destroy_handler_id_);
+}
diff --git a/chrome/browser/gtk/focus_store_gtk.h b/chrome/browser/gtk/focus_store_gtk.h
new file mode 100644
index 0000000..7cb7efc
--- /dev/null
+++ b/chrome/browser/gtk/focus_store_gtk.h
@@ -0,0 +1,78 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
+#define CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
+
+#include "base/basictypes.h"
+
+typedef struct _GtkWidget GtkWidget;
+
+class FocusStoreGtk {
+ public:
+  FocusStoreGtk();
+
+  virtual ~FocusStoreGtk();
+
+  GtkWidget* widget() const { return widget_; }
+
+  // Save the widget that is currently focused for |widget|'s toplevel (NOT
+  // |widget|). Call with NULL to clear |widget_|.
+  void Store(GtkWidget* widget);
+
+ private:
+  // Disconnect the previous destroy handler (if any).
+  void DisconnectDestroyHandler();
+
+  // The widget which last had focus.
+  GtkWidget* widget_;
+
+  // The widget for which we've stored focus might be destroyed by the time we
+  // want to restore focus. Thus we connect to the "destroy" signal on that
+  // widget. This is the ID for the destroy handler.
+  unsigned int destroy_handler_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusStoreGtk);
+};
+
+#endif  // CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
+#define CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
+
+#include "base/basictypes.h"
+
+typedef struct _GtkWidget GtkWidget;
+
+class FocusStoreGtk {
+ public:
+  FocusStoreGtk();
+
+  virtual ~FocusStoreGtk();
+
+  GtkWidget* widget() const { return widget_; }
+
+  // Save the widget that is currently focused for |widget|'s toplevel (NOT
+  // |widget|). Call with NULL to clear |widget_|.
+  void Store(GtkWidget* widget);
+
+ private:
+  // Disconnect the previous destroy handler (if any).
+  void DisconnectDestroyHandler();
+
+  // The widget which last had focus.
+  GtkWidget* widget_;
+
+  // The widget for which we've stored focus might be destroyed by the time we
+  // want to restore focus. Thus we connect to the "destroy" signal on that
+  // widget. This is the ID for the destroy handler.
+  unsigned int destroy_handler_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusStoreGtk);
+};
+
+#endif  // CHROME_BROWSER_GTK_FOCUS_STORE_GTK_H_
diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
index 67f4d73..b9671be 100644
--- a/chrome/browser/tab_contents/tab_contents_view_gtk.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
@@ -33,7 +33,7 @@ gboolean OnFocus(GtkWidget* widget, GtkDirectionType focus,
   // If we already have focus, let the next widget have a shot at it. We will
   // reach this situation after the call to gtk_widget_child_focus() in
   // TakeFocus().
-  if (GTK_WIDGET_HAS_FOCUS(widget))
+  if (gtk_widget_is_focus(widget))
     return FALSE;
 
   gtk_widget_grab_focus(widget);
@@ -67,8 +67,7 @@ TabContentsView* TabContentsView::Create(TabContents* tab_contents) {
 
 TabContentsViewGtk::TabContentsViewGtk(TabContents* tab_contents)
     : TabContentsView(tab_contents),
-      vbox_(gtk_vbox_new(FALSE, 0)),
-      stored_focus_widget_(NULL) {
+      vbox_(gtk_vbox_new(FALSE, 0)) {
   registrar_.Add(this, NotificationType::TAB_CONTENTS_CONNECTED,
                  Source<TabContents>(tab_contents));
   registrar_.Add(this, NotificationType::TAB_CONTENTS_DISCONNECTED,
@@ -192,29 +191,12 @@ void TabContentsViewGtk::SetInitialFocus() {
 }
 
 void TabContentsViewGtk::StoreFocus() {
-  GtkWindow* window = GetTopLevelNativeWindow();
-  if (!window) {
-    NOTREACHED();
-    return;
-  }
-
-  // Disconnect the previous destroy handler (if any).
-  if (stored_focus_widget_)
-    g_signal_handler_disconnect(stored_focus_widget_, destroy_handler_id_);
-
-  stored_focus_widget_ = window->focus_widget;
-  if (stored_focus_widget_) {
-    // gtk_widget_destroyed() will set |stored_focus_widget_| to NULL when it is
-    // invoked during handling of the "destroy" signal.
-    destroy_handler_id_ = g_signal_connect(stored_focus_widget_, "destroy",
-                                           G_CALLBACK(gtk_widget_destroyed),
-                                           &stored_focus_widget_);
-  }
+  focus_store_.Store(GetNativeView());
 }
 
 void TabContentsViewGtk::RestoreFocus() {
-  if (stored_focus_widget_)
-    gtk_widget_grab_focus(stored_focus_widget_);
+  if (focus_store_.widget())
+    gtk_widget_grab_focus(focus_store_.widget());
   else
     SetInitialFocus();
 }
diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.h b/chrome/browser/tab_contents/tab_contents_view_gtk.h
index 1dc2c76..3120cc3 100644
--- a/chrome/browser/tab_contents/tab_contents_view_gtk.h
+++ b/chrome/browser/tab_contents/tab_contents_view_gtk.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_TAB_CONTENTS_TAB_CONTENTS_VIEW_GTK_H_
 
 #include "base/scoped_ptr.h"
+#include "chrome/browser/gtk/focus_store_gtk.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
@@ -87,13 +88,7 @@ class TabContentsViewGtk : public TabContentsView,
 
   scoped_ptr<SadTabGtk> sad_tab_;
 
-  // The widget that was focused the last time we were asked to store focus.
-  GtkWidget* stored_focus_widget_;
-
-  // The widget for which we've stored focus might be destroyed by the time we
-  // want to restore focus. Thus we connect to the "destroy" signal on that
-  // widget. This is the handler ID for the destroy handler.
-  guint destroy_handler_id_;
+  FocusStoreGtk focus_store_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContentsViewGtk);
 };
diff --git a/chrome/browser/views/browser_views.vcproj b/chrome/browser/views/browser_views.vcproj
index d2594d0..75e2939 100644
--- a/chrome/browser/views/browser_views.vcproj
+++ b/chrome/browser/views/browser_views.vcproj
@@ -546,11 +546,7 @@
 			>
 		</File>
 		<File
-			RelativePath=".\download_started_animation.cc"
-			>
-		</File>
-		<File
-			RelativePath=".\download_started_animation.h"
+			RelativePath=".\download_started_animation_win.cc"
 			>
 		</File>
 		<File
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index a07fac0..3afa478 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -971,6 +971,8 @@
         'browser/gtk/infobar_gtk.h',
         'browser/gtk/find_bar_gtk.cc',
         'browser/gtk/find_bar_gtk.h',
+        'browser/gtk/focus_store_gtk.cc',
+        'browser/gtk/focus_store_gtk.h',
         'browser/gtk/link_button_gtk.cc',
         'browser/gtk/link_button_gtk.cc',
         'browser/gtk/location_bar_view_gtk.cc',