Linux findbar improvements:

* clean up toolbar/infobar/findbar borders
* move findbar to BrowserWindowGtk's vbox (so it stacks on top of infobar, as on windows)
* properly show findbar when switching between tabs
Review URL: http://codereview.chromium.org/99166

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

7 files changed, 110 insertions, 74 deletions

diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 63596a4..85252ac 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -260,13 +260,6 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser)
                      infobar_container_->widget(),
                      FALSE, FALSE, 0);
 
-  // Insert a border between the toolbar and the web contents.
-  GtkWidget* border = gtk_event_box_new();
-  gtk_widget_set_size_request(border, -1, 1);
-  gtk_widget_modify_bg(border, GTK_STATE_NORMAL, &kBorderColor);
-  gtk_box_pack_start(GTK_BOX(content_vbox_), border, FALSE, FALSE, 0);
-  gtk_widget_show(border);
-
   contents_container_.reset(new TabContentsContainerGtk());
   contents_container_->AddContainerToBox(content_vbox_);
 
@@ -650,8 +643,11 @@ bool BrowserWindowGtk::ShouldShowWindowIcon() const {
   return browser_->SupportsWindowFeature(Browser::FEATURE_TITLEBAR);
 }
 
-void BrowserWindowGtk::AddFindBar(GtkWidget* findbar) {
-  contents_container_->AddFindBar(findbar);
+void BrowserWindowGtk::AddFindBar(FindBarGtk* findbar) {
+  contents_container_->set_find_bar(findbar);
+  gtk_box_pack_start(GTK_BOX(content_vbox_), findbar->widget(),
+                     FALSE, FALSE, 0);
+  gtk_box_reorder_child(GTK_BOX(content_vbox_), findbar->widget(), 2);
 }
 
 void BrowserWindowGtk::ConnectAccelerators() {
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 54bf690..69b9025 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -25,6 +25,9 @@ const GdkColor kBorderColor = GDK_COLOR_RGB(0xbe, 0xc8, 0xd4);
 // Padding around the container.
 const int kBarPadding = 4;
 
+// The vertical positioning of |container_| in |fixed_|.
+const int kVerticalOffset = -1;
+
 gboolean EntryContentsChanged(GtkWindow* window, FindBarGtk* find_bar) {
   find_bar->ContentsChanged();
   return FALSE;
@@ -45,28 +48,45 @@ FindBarGtk::FindBarGtk(BrowserWindowGtk* browser) {
   InitWidgets();
 
   // Insert the widget into the browser gtk hierarchy.
-  browser->AddFindBar(container_.get());
+  browser->AddFindBar(this);
 
   // Hook up signals after the widget has been added to the hierarchy so the
   // widget will be realized.
-  g_signal_connect(G_OBJECT(find_text_), "changed",
+  g_signal_connect(find_text_, "changed",
                    G_CALLBACK(EntryContentsChanged), this);
-  g_signal_connect(G_OBJECT(find_text_), "key-press-event",
+  g_signal_connect(find_text_, "key-press-event",
                    G_CALLBACK(KeyPressEvent), this);
+  g_signal_connect(widget(), "size-allocate",
+                   G_CALLBACK(OnSizeAllocate), this);
 }
 
 FindBarGtk::~FindBarGtk() {
-  container_.Destroy();
+  fixed_.Destroy();
 }
 
 void FindBarGtk::InitWidgets() {
   // The find bar is basically an hbox with a gtkentry (text box) followed by 3
   // buttons (previous result, next result, close).  We wrap the hbox in a gtk
   // alignment and a gtk event box to get the padding and light blue
-  // background.
+  // background. We put that event box in a fixed in order to control its
+  // position.
   GtkWidget* hbox = gtk_hbox_new(false, 0);
-  container_.Own(gfx::CreateGtkBorderBin(hbox, &kBackgroundColor, kBarPadding,
-      kBarPadding, kBarPadding, kBarPadding));
+  container_ = gfx::CreateGtkBorderBin(hbox, &kBackgroundColor,
+      kBarPadding, kBarPadding, kBarPadding, kBarPadding);
+  fixed_.Own(gtk_fixed_new());
+
+  // |fixed_| has to be at least one pixel tall. We color this pixel the same
+  // color as the border that separates the toolbar from the web contents.
+  // TODO(estade): find a better solution. (Ideally the tool bar shouldn't draw
+  // its own border, but the border is part of the background bitmap, so
+  // changing that would affect all platforms.)
+  border_ = gtk_event_box_new();
+  gtk_widget_set_size_request(border_, 1, 1);
+  gtk_widget_modify_bg(border_, GTK_STATE_NORMAL, &kBorderColor);
+
+  gtk_fixed_put(GTK_FIXED(widget()), border_, 0, 0);
+  gtk_fixed_put(GTK_FIXED(widget()), container_, 0, kVerticalOffset);
+  gtk_widget_set_size_request(widget(), -1, 0);
 
   close_button_.reset(CustomDrawButton::AddBarCloseButton(hbox));
   g_signal_connect(G_OBJECT(close_button_->widget()), "clicked",
@@ -102,17 +122,21 @@ void FindBarGtk::InitWidgets() {
   GtkWidget* centering_vbox = gtk_vbox_new(FALSE, 0);
   gtk_box_pack_start(GTK_BOX(centering_vbox), border_bin, TRUE, FALSE, 0);
   gtk_box_pack_end(GTK_BOX(hbox), centering_vbox, FALSE, FALSE, 0);
+
+  // We show just the GtkFixed and |border_| (not |container_|).
+  gtk_widget_show(widget());
+  gtk_widget_show(border_);
 }
 
 void FindBarGtk::Show() {
   // TODO(tc): This should be an animated slide in.
-  gtk_widget_show_all(container_.get());
+  gtk_widget_show_all(widget());
   gtk_widget_grab_focus(find_text_);
 }
 
 void FindBarGtk::Hide(bool animate) {
   // TODO(tc): Animated slide away.
-  gtk_widget_hide(container_.get());
+  gtk_widget_hide(container_);
 }
 
 void FindBarGtk::SetFocusAndSelection() {
@@ -143,16 +167,20 @@ void FindBarGtk::UpdateUIForFindResult(const FindNotificationDetails& result,
 }
 
 gfx::Rect FindBarGtk::GetDialogPosition(gfx::Rect avoid_overlapping_rect) {
-  return gfx::Rect();
+  // TODO(estade): Logic for the positioning of the find bar should be factored
+  // out of here and browser/views/* and into FindBarController.
+  int xposition = widget()->allocation.width - container_->allocation.width -
+                  50;
+
+  return gfx::Rect(xposition, 0, 1, 1);
 }
 
 void FindBarGtk::SetDialogPosition(const gfx::Rect& new_pos, bool no_redraw) {
-  if (!IsFindBarVisible())
-    Show();  // TODO(tc): This should be a no animation show.
+  gtk_fixed_move(GTK_FIXED(widget()), container_, new_pos.x(), kVerticalOffset);
 }
 
 bool FindBarGtk::IsFindBarVisible() {
-  return GTK_WIDGET_VISIBLE(container_.get());
+  return GTK_WIDGET_VISIBLE(widget());
 }
 
 void FindBarGtk::RestoreSavedFocus() {
@@ -168,6 +196,11 @@ bool FindBarGtk::GetFindBarWindowInfo(gfx::Point* position,
   return false;
 }
 
+void FindBarGtk::AssureOnTop() {
+  gtk_widget_hide(container_);
+  gtk_widget_show_all(container_);
+}
+
 void FindBarGtk::ContentsChanged() {
   WebContents* web_contents = find_bar_controller_->web_contents();
   if (!web_contents)
@@ -192,7 +225,7 @@ void FindBarGtk::OnButtonPressed(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() ||
-      button == find_bar->find_next_button_->widget()) {
+             button == find_bar->find_next_button_->widget()) {
     std::string find_text_utf8(
         gtk_entry_get_text(GTK_ENTRY(find_bar->find_text_)));
     find_bar->find_bar_controller_->web_contents()->StartFinding(
@@ -202,3 +235,26 @@ void FindBarGtk::OnButtonPressed(GtkWidget* button, FindBarGtk* find_bar) {
     NOTREACHED();
   }
 }
+
+// static
+void FindBarGtk::OnSizeAllocate(GtkWidget* fixed,
+                                GtkAllocation* allocation,
+                                FindBarGtk* findbar) {
+  // Set the background widget to the size of |fixed|.
+  if (findbar->border_->allocation.width != allocation->width) {
+    gtk_widget_set_size_request(findbar->border_, allocation->width, 1);
+  }
+
+  // Reposition |container_|.
+  GtkWidget* container = findbar->container_;
+  DCHECK(container);
+  if (!GTK_WIDGET_VISIBLE(container))
+    return;
+
+  int xposition = findbar->GetDialogPosition(gfx::Rect()).x();
+  if (xposition == container->allocation.x) {
+    return;
+  } else {
+    gtk_fixed_move(GTK_FIXED(fixed), container, xposition, kVerticalOffset);
+  }
+}
diff --git a/chrome/browser/gtk/find_bar_gtk.h b/chrome/browser/gtk/find_bar_gtk.h
index 07aa76f..eedbcdf 100644
--- a/chrome/browser/gtk/find_bar_gtk.h
+++ b/chrome/browser/gtk/find_bar_gtk.h
@@ -32,7 +32,7 @@ class FindBarGtk : public FindBar,
   // Callback when Escape is pressed.
   void EscapePressed();
 
-  GtkWidget* widget() const { return container_.get(); }
+  GtkWidget* widget() const { return fixed_.get(); }
 
   // Methods from FindBar.
   virtual FindBarController* GetFindBarController() const {
@@ -61,14 +61,32 @@ class FindBarGtk : public FindBar,
   virtual bool GetFindBarWindowInfo(gfx::Point* position,
                                     bool* fully_visible);
 
+  // Make sure the find bar is foremost on the z axis in the widget hierarchy
+  // by hiding and showing it.
+  void AssureOnTop();
+
  private:
   void InitWidgets();
 
   // Callback for previous, next, and close button.
   static void OnButtonPressed(GtkWidget* button, FindBarGtk* find_bar);
 
-  // GtkHBox containing the find bar widgets.
-  OwnedWidgetGtk container_;
+  // Called when |fixed_| changes sizes. Used to position |container_|.
+  static void OnSizeAllocate(GtkWidget* fixed,
+                             GtkAllocation* allocation,
+                             FindBarGtk* container_);
+
+  // GtkFixed containing the find bar widgets.
+  OwnedWidgetGtk fixed_;
+
+  // An event box which shows the background for |fixed_|. We could just set
+  // |fixed_| to have its own GdkWindow and draw the background directly, but
+  // then |container_| would clip to the bounds of |fixed_|.
+  GtkWidget* border_;
+
+  // A GtkAlignment which holds what the user perceives as the findbar (the text
+  // field, the buttons, etc.).
+  GtkWidget* container_;
 
   // The widget where text is entered.
   GtkWidget* find_text_;
diff --git a/chrome/browser/gtk/infobar_gtk.cc b/chrome/browser/gtk/infobar_gtk.cc
index 432691b..5504a5e 100644
--- a/chrome/browser/gtk/infobar_gtk.cc
+++ b/chrome/browser/gtk/infobar_gtk.cc
@@ -50,7 +50,7 @@ InfoBar::InfoBar(InfoBarDelegate* delegate)
   // Set the top border and background color.
   gtk_widget_modify_bg(bg_box, GTK_STATE_NORMAL, &kBackgroundColor);
   border_bin_.Own(gfx::CreateGtkBorderBin(bg_box, &kBorderColor,
-                                          1, 0, 0, 0));
+                                          0, 1, 0, 0));
   gtk_widget_set_size_request(border_bin_.get(), -1, kInfoBarHeight);
 
   // Add the icon on the left, if any.
diff --git a/chrome/browser/gtk/tab_contents_container_gtk.cc b/chrome/browser/gtk/tab_contents_container_gtk.cc
index 71c2b5f..c4563bb 100644
--- a/chrome/browser/gtk/tab_contents_container_gtk.cc
+++ b/chrome/browser/gtk/tab_contents_container_gtk.cc
@@ -5,21 +5,15 @@
 #include "chrome/browser/gtk/tab_contents_container_gtk.h"
 
 #include "base/gfx/native_widget_types.h"
+#include "chrome/browser/gtk/find_bar_gtk.h"
 #include "chrome/browser/tab_contents/web_contents.h"
 #include "chrome/browser/renderer_host/render_widget_host_view_gtk.h"
 #include "chrome/common/notification_service.h"
 
-
 TabContentsContainerGtk::TabContentsContainerGtk()
     : tab_contents_(NULL),
-      vbox_(gtk_vbox_new(FALSE, 0)),
-      fixed_(gtk_fixed_new()),
-      findbar_(NULL) {
-  gtk_widget_set_size_request(fixed_, -1, 0);
-  gtk_box_pack_start(GTK_BOX(vbox_), fixed_, FALSE, FALSE, 0);
+      vbox_(gtk_vbox_new(FALSE, 0)) {
   gtk_widget_show_all(vbox_);
-  g_signal_connect(fixed_, "size-allocate",
-                   G_CALLBACK(OnSizeAllocate), this);
 }
 
 TabContentsContainerGtk::~TabContentsContainerGtk() {
@@ -31,12 +25,6 @@ void TabContentsContainerGtk::AddContainerToBox(GtkWidget* box) {
   gtk_box_pack_start(GTK_BOX(box), vbox_, TRUE, TRUE, 0);
 }
 
-void TabContentsContainerGtk::AddFindBar(GtkWidget* findbar) {
-  findbar_ = findbar;
-  // We will reposition it later (when we get a size-allocate event).
-  gtk_fixed_put(GTK_FIXED(fixed_), findbar, 0, 0);
-}
-
 void TabContentsContainerGtk::SetTabContents(TabContents* tab_contents) {
   if (tab_contents_) {
     gfx::NativeView widget = tab_contents_->GetNativeView();
@@ -60,6 +48,10 @@ void TabContentsContainerGtk::SetTabContents(TabContents* tab_contents) {
       gtk_box_pack_end(GTK_BOX(vbox_), widget, TRUE, TRUE, 0);
       gtk_widget_show_all(widget);
     }
+    // We need to make sure that the find bar is on top before any painting
+    // is done.
+    if (tab_contents_->find_ui_active())
+      findbar_->AssureOnTop();
   }
 }
 
@@ -122,20 +114,3 @@ void TabContentsContainerGtk::TabContentsDestroyed(TabContents* contents) {
   DCHECK(contents == tab_contents_);
   SetTabContents(NULL);
 }
-
-void TabContentsContainerGtk::OnSizeAllocate(GtkWidget* fixed,
-    GtkAllocation* allocation, TabContentsContainerGtk* contents_container) {
-  GtkWidget* findbar = contents_container->findbar_;
-  DCHECK(findbar);
-  if (!GTK_WIDGET_VISIBLE(findbar))
-    return;
-
-  // TODO(port): Logic for the positioning of the find bar should be factored
-  // out of here and browser/views/* and into FindBarController.
-  int xposition = allocation->width - findbar->allocation.width - 50;
-  if (xposition == findbar->allocation.x) {
-    return;
-  } else {
-    gtk_fixed_move(GTK_FIXED(fixed), findbar, xposition, 0);
-  }
-}
diff --git a/chrome/browser/gtk/tab_contents_container_gtk.h b/chrome/browser/gtk/tab_contents_container_gtk.h
index bb914d0..d57adae 100644
--- a/chrome/browser/gtk/tab_contents_container_gtk.h
+++ b/chrome/browser/gtk/tab_contents_container_gtk.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "chrome/common/notification_observer.h"
 
+class FindBarGtk;
 class RenderViewHost;
 class TabContents;
 
@@ -21,9 +22,6 @@ class TabContentsContainerGtk : public NotificationObserver {
   // Inserts our GtkWidget* hierarchy into a GtkBox managed by our owner.
   void AddContainerToBox(GtkWidget* widget);
 
-  // Add the findbar to the top of the tab contents container.
-  void AddFindBar(GtkWidget* widget);
-
   // Make the specified tab visible.
   void SetTabContents(TabContents* tab_contents);
   TabContents* GetTabContents() const { return tab_contents_; }
@@ -33,6 +31,8 @@ class TabContentsContainerGtk : public NotificationObserver {
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
+  void set_find_bar(FindBarGtk* findbar) { findbar_ = findbar; }
+
  private:
   // Add or remove observers for events that we care about.
   void AddObservers();
@@ -48,11 +48,6 @@ class TabContentsContainerGtk : public NotificationObserver {
   // get notified.
   void TabContentsDestroyed(TabContents* contents);
 
-  // Called when |fixed_| changes sizes. Used to position the findbar.
-  static void OnSizeAllocate(GtkWidget* fixed,
-                             GtkAllocation* allocation,
-                             TabContentsContainerGtk* contents_container);
-
   // The currently visible TabContents.
   TabContents* tab_contents_;
 
@@ -61,11 +56,9 @@ class TabContentsContainerGtk : public NotificationObserver {
   // vbox_.
   GtkWidget* vbox_;
 
-  // This GtkFixed widget helps us position the find bar.
-  GtkWidget* fixed_;
-
-  // The findbar widget. We do not own it.
-  GtkWidget* findbar_;
+  // We have to make sure we are always underneath the findbar, hence this
+  // pointer.
+  FindBarGtk* findbar_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContentsContainerGtk);
 };
diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
index 6c6bc30..35304d9 100644
--- a/chrome/browser/tab_contents/tab_contents_view_gtk.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_gtk.cc
@@ -182,9 +182,7 @@ void TabContentsViewGtk::StoreFocus() {
 
 void TabContentsViewGtk::RestoreFocus() {
   // TODO(estade): implement this function.
-  // For now just assume we are viewing the tab for the first time.
-  SetInitialFocus();
-  NOTIMPLEMENTED() << " --  need to restore the focus position on this page.";
+  NOTIMPLEMENTED() << " Need to restore the focus position on this page.";
 }
 
 void TabContentsViewGtk::UpdateDragCursor(bool is_drop_target) {