Make the findbar slide in/out.

Change SlideAnimatorGtk so that its GtkFixed doesn't have its own window. Giving that fixed its own GdkWindow meant that as a client of SlideAnimatorGtk you were pretty much cornered into having rectangular widgets (unless you want to perform a lot of expensive operations during every frame of the slide). We solve the clipping problem this creates by making a "render area" vbox in BrowserWindowGtk that is all part of a single GdkWindow hierarchy.

This isn't yet perfect as there are sometimes occlusion problems with infobars. That will probably need to be solved in the same way it was solved for the tab contents container. This patch is already hard enough to grok, so I'm going to leave that problem for another patch.

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

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

7 files changed, 97 insertions, 50 deletions

diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index 87f4818..5f42f77 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -34,7 +34,7 @@
 namespace {
 
 // Height of the toolbar in pixels.
-const int kToolbarHeight = 38;
+const int kToolbarHeight = 37;
 
 // The amount of space between the bottom of the star and the top of the
 // Omnibox results popup window.  We want a two pixel space between the bottom
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 54ab11e..ceb2b56 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -257,13 +257,19 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser)
   bookmark_bar_.reset(new BookmarkBarGtk(browser_->profile(), browser_.get()));
   bookmark_bar_->AddBookmarkbarToBox(content_vbox_);
 
+  // This vbox surrounds the render area: find bar, info bars and render view.
+  // The reason is that this area as a whole needs to be grouped in its own
+  // GdkWindow hierarchy so that animations originating inside it (infobar,
+  // download shelf, find bar) are all clipped to that area. This is why
+  // |render_area_vbox_| is packed in |event_box|.
+  render_area_vbox_ = gtk_vbox_new(FALSE, 0);
   infobar_container_.reset(new InfoBarContainerGtk(this));
-  gtk_box_pack_start(GTK_BOX(content_vbox_),
+  gtk_box_pack_start(GTK_BOX(render_area_vbox_),
                      infobar_container_->widget(),
                      FALSE, FALSE, 0);
 
   contents_container_.reset(new TabContentsContainerGtk());
-  contents_container_->AddContainerToBox(content_vbox_);
+  contents_container_->AddContainerToBox(render_area_vbox_);
 
   // Note that calling this the first time is necessary to get the
   // proper control layout.
@@ -272,10 +278,12 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser)
 
   status_bubble_.reset(new StatusBubbleGtk(window_));
 
+  GtkWidget* event_box = gtk_event_box_new();
+  gtk_container_add(GTK_CONTAINER(event_box), render_area_vbox_);
+  gtk_container_add(GTK_CONTAINER(content_vbox_), event_box);
   gtk_container_add(GTK_CONTAINER(window_vbox_), content_vbox_);
   gtk_container_add(GTK_CONTAINER(window_), window_vbox_);
-  gtk_widget_show(content_vbox_);
-  gtk_widget_show(window_vbox_);
+  gtk_widget_show_all(window_vbox_);
   browser_->tabstrip_model()->AddObserver(this);
 
   NotificationService* ns = NotificationService::current();
@@ -654,9 +662,9 @@ bool BrowserWindowGtk::ShouldShowWindowIcon() const {
 
 void BrowserWindowGtk::AddFindBar(FindBarGtk* findbar) {
   contents_container_->set_find_bar(findbar);
-  gtk_box_pack_start(GTK_BOX(content_vbox_), findbar->widget(),
+  gtk_box_pack_start(GTK_BOX(render_area_vbox_), findbar->widget(),
                      FALSE, FALSE, 0);
-  gtk_box_reorder_child(GTK_BOX(content_vbox_), findbar->widget(), 2);
+  gtk_box_reorder_child(GTK_BOX(render_area_vbox_), findbar->widget(), 0);
 }
 
 void BrowserWindowGtk::SetGeometryHints() {
diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h
index 9974214..5cecbe7 100644
--- a/chrome/browser/gtk/browser_window_gtk.h
+++ b/chrome/browser/gtk/browser_window_gtk.h
@@ -111,9 +111,14 @@ class BrowserWindowGtk : public BrowserWindow,
 
  protected:
   virtual void DestroyBrowser();
+  // Top level window.
   GtkWindow* window_;
+  // VBox that holds the interior components of the chromium window.
   GtkWidget* window_vbox_;
+  // VBox that holds everything below the tabs.
   GtkWidget* content_vbox_;
+  // VBox that holds everything below the toolbar.
+  GtkWidget* render_area_vbox_;
 
   scoped_ptr<Browser> browser_;
 
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 14b6d51..b37d8e7 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/gtk/browser_window_gtk.h"
 #include "chrome/browser/gtk/custom_button.h"
 #include "chrome/browser/gtk/nine_box.h"
+#include "chrome/browser/gtk/slide_animator_gtk.h"
 #include "chrome/browser/gtk/tab_contents_container_gtk.h"
 #include "chrome/browser/tab_contents/web_contents.h"
 #include "chrome/common/gtk_util.h"
@@ -26,8 +27,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;
+// The height of the findbar dialog, as dictated by the size of the background
+// images.
+const int kFindBarHeight = 32;
 
 gboolean EntryContentsChanged(GtkWindow* window, FindBarGtk* find_bar) {
   find_bar->ContentsChanged();
@@ -86,10 +88,12 @@ FindBarGtk::FindBarGtk(BrowserWindowGtk* browser)
                    G_CALLBACK(KeyPressEvent), this);
   g_signal_connect(widget(), "size-allocate",
                    G_CALLBACK(OnFixedSizeAllocate), this);
-  // We can't call ContourWidget() until after |container| has been
+  // We can't call ContourWidget() until after |container_| has been
   // allocated, hence we connect to this signal.
   g_signal_connect(container_, "size-allocate",
                    G_CALLBACK(OnContainerSizeAllocate), this);
+  g_signal_connect(container_, "expose-event",
+                   G_CALLBACK(OnExpose), NULL);
 }
 
 FindBarGtk::~FindBarGtk() {
@@ -101,26 +105,26 @@ void FindBarGtk::InitWidgets() {
   // 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. We put that event box in a fixed in order to control its
-  // position.
+  // lateral position. We put that fixed in a SlideAnimatorGtk in order to get
+  // the slide effect.
   GtkWidget* hbox = gtk_hbox_new(false, 0);
   container_ = gfx::CreateGtkBorderBin(hbox, &kBackgroundColor,
       kBarPadding, kBarPadding, kBarPadding, kBarPadding);
   gtk_widget_set_app_paintable(container_, TRUE);
-  g_signal_connect(container_, "expose-event",
-                   G_CALLBACK(OnExpose), NULL);
+
+  slide_widget_.reset(new SlideAnimatorGtk(container_,
+                                           SlideAnimatorGtk::DOWN,
+                                           0, false, NULL));
 
   // |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.)
   fixed_.Own(gtk_fixed_new());
   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_fixed_put(GTK_FIXED(widget()), slide_widget(), 0, 0);
   gtk_widget_set_size_request(widget(), -1, 0);
 
   close_button_.reset(CustomDrawButton::AddBarCloseButton(hbox));
@@ -158,20 +162,27 @@ void FindBarGtk::InitWidgets() {
   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_|).
+  // We show just the GtkFixed and |border_| (but not the dialog).
   gtk_widget_show(widget());
   gtk_widget_show(border_);
 }
 
+GtkWidget* FindBarGtk::slide_widget() {
+  return slide_widget_->widget();
+}
+
 void FindBarGtk::Show() {
-  // TODO(tc): This should be an animated slide in.
-  gtk_widget_show_all(widget());
+  AssureOnTop();
+  gtk_widget_show_all(slide_widget());
   gtk_widget_grab_focus(find_text_);
+  slide_widget_->Open();
 }
 
 void FindBarGtk::Hide(bool animate) {
-  // TODO(tc): Animated slide away.
-  gtk_widget_hide(container_);
+  if (animate)
+    slide_widget_->Close();
+  else
+    gtk_widget_hide(slide_widget());
 }
 
 void FindBarGtk::SetFocusAndSelection() {
@@ -184,7 +195,7 @@ void FindBarGtk::ClearResults(const FindNotificationDetails& results) {
 }
 
 void FindBarGtk::StopAnimation() {
-  // No animation yet, so do nothing.
+  NOTIMPLEMENTED();
 }
 
 void FindBarGtk::MoveWindowIfNecessary(const gfx::Rect& selection_rect,
@@ -204,14 +215,15 @@ void FindBarGtk::UpdateUIForFindResult(const FindNotificationDetails& result,
 gfx::Rect FindBarGtk::GetDialogPosition(gfx::Rect avoid_overlapping_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;
+  int xposition = widget()->allocation.width -
+                  slide_widget()->allocation.width - 50;
 
   return gfx::Rect(xposition, 0, 1, 1);
 }
 
 void FindBarGtk::SetDialogPosition(const gfx::Rect& new_pos, bool no_redraw) {
-  gtk_fixed_move(GTK_FIXED(widget()), container_, new_pos.x(), kVerticalOffset);
+  gtk_fixed_move(GTK_FIXED(widget()), slide_widget(), new_pos.x(), 0);
+  gtk_widget_show_all(slide_widget());
 }
 
 bool FindBarGtk::IsFindBarVisible() {
@@ -232,8 +244,8 @@ bool FindBarGtk::GetFindBarWindowInfo(gfx::Point* position,
 }
 
 void FindBarGtk::AssureOnTop() {
-  gtk_widget_hide(container_);
-  gtk_widget_show_all(container_);
+  if (container_->window)
+    gdk_window_raise(container_->window);
 }
 
 void FindBarGtk::ContentsChanged() {
@@ -283,17 +295,16 @@ void FindBarGtk::OnFixedSizeAllocate(GtkWidget* fixed,
     gtk_widget_size_allocate(findbar->border_, allocation);
   }
 
-  // Reposition |container_|.
-  GtkWidget* container = findbar->container_;
-  DCHECK(container);
-  if (!GTK_WIDGET_VISIBLE(container))
+  // Reposition the dialog.
+  GtkWidget* dialog = findbar->slide_widget();
+  if (!GTK_WIDGET_VISIBLE(dialog))
     return;
 
   int xposition = findbar->GetDialogPosition(gfx::Rect()).x();
-  if (xposition == container->allocation.x) {
+  if (xposition == dialog->allocation.x) {
     return;
   } else {
-    gtk_fixed_move(GTK_FIXED(fixed), container, xposition, kVerticalOffset);
+    gtk_fixed_move(GTK_FIXED(fixed), findbar->slide_widget(), xposition, 0);
   }
 }
 
diff --git a/chrome/browser/gtk/find_bar_gtk.h b/chrome/browser/gtk/find_bar_gtk.h
index a25d5be..90b0338 100644
--- a/chrome/browser/gtk/find_bar_gtk.h
+++ b/chrome/browser/gtk/find_bar_gtk.h
@@ -15,6 +15,7 @@
 class BrowserWindowGtk;
 class CustomDrawButton;
 class FindBarController;
+class SlideAnimatorGtk;
 class TabContentsContainerGtk;
 class WebContents;
 
@@ -68,10 +69,15 @@ class FindBarGtk : public FindBar,
  private:
   void InitWidgets();
 
+  // Returns the child of |fixed_| that holds what the user perceives as the
+  // findbar.
+  GtkWidget* slide_widget();
+
   // Callback for previous, next, and close button.
   static void OnButtonPressed(GtkWidget* button, FindBarGtk* find_bar);
 
-  // Called when |fixed_| changes sizes. Used to position |container_|.
+  // Called when |fixed_| changes sizes. Used to position the dialog (the
+  // "dialog" is the widget hierarchy rooted at |slide_widget_|).
   static void OnFixedSizeAllocate(GtkWidget* fixed,
                                   GtkAllocation* allocation,
                                   FindBarGtk* findbar);
@@ -81,7 +87,6 @@ class FindBarGtk : public FindBar,
                                       GtkAllocation* allocation,
                                       FindBarGtk* findbar);
 
-
   // GtkFixed containing the find bar widgets.
   OwnedWidgetGtk fixed_;
 
@@ -90,8 +95,10 @@ class FindBarGtk : public FindBar,
   // 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.).
+  // The widget that animates the slide-in and -out of the findbar.
+  scoped_ptr<SlideAnimatorGtk> slide_widget_;
+
+  // A GtkAlignment that is the child of |slide_widget_|.
   GtkWidget* container_;
 
   // This will be set to true after ContourWidget() has been called so we don't
diff --git a/chrome/browser/gtk/slide_animator_gtk.cc b/chrome/browser/gtk/slide_animator_gtk.cc
index 702c355..083fab7 100644
--- a/chrome/browser/gtk/slide_animator_gtk.cc
+++ b/chrome/browser/gtk/slide_animator_gtk.cc
@@ -13,11 +13,13 @@ namespace {
 void OnFixedSizeAllocate(GtkWidget* fixed,
                          GtkAllocation* allocation,
                          GtkWidget* child) {
-  // The size of the GtkFixed has changed. We want |child_| to match widths,
-  // but the height should not change.
-  GtkAllocation new_allocation = child->allocation;
-  new_allocation.width = allocation->width;
-  gtk_widget_size_allocate(child, &new_allocation);
+  if (allocation->width != child->allocation.width) {
+    // The size of the GtkFixed has changed. We want |child_| to match widths,
+    // but the height should not change.
+    GtkAllocation new_allocation = child->allocation;
+    new_allocation.width = allocation->width;
+    gtk_widget_size_allocate(child, &new_allocation);
+  }
 }
 
 }  // namespace
@@ -32,9 +34,6 @@ SlideAnimatorGtk::SlideAnimatorGtk(GtkWidget* child,
       delegate_(delegate),
       fixed_needs_resize_(false) {
   widget_.Own(gtk_fixed_new());
-  // We need to give the GtkFixed its own window so that painting will clip
-  // correctly.
-  gtk_fixed_set_has_window(GTK_FIXED(widget_.get()), TRUE);
   gtk_fixed_put(GTK_FIXED(widget_.get()), child, 0, 0);
   gtk_widget_set_size_request(widget_.get(), -1, 0);
   // We have to manually set the size request for |child_| every time the
@@ -50,6 +49,8 @@ SlideAnimatorGtk::SlideAnimatorGtk(GtkWidget* child,
   g_signal_connect(child, "size-allocate",
                    G_CALLBACK(OnChildSizeAllocate), this);
 
+  child_needs_move_ = (direction == DOWN);
+
   animation_.reset(new SlideAnimation(this));
   // Default tween type is EASE_OUT.
   if (linear)
@@ -100,9 +101,13 @@ void SlideAnimatorGtk::AnimationEnded(const Animation* animation) {
 void SlideAnimatorGtk::OnChildSizeAllocate(GtkWidget* child,
                                            GtkAllocation* allocation,
                                            SlideAnimatorGtk* slider) {
-  if (!slider->fixed_needs_resize_)
-    return;
+  if (slider->child_needs_move_) {
+    gtk_fixed_move(GTK_FIXED(slider->widget()), child, 0, -allocation->height);
+    slider->child_needs_move_ = false;
+  }
 
-  slider->fixed_needs_resize_ = false;
-  slider->AnimationProgressed(slider->animation_.get());
+  if (slider->fixed_needs_resize_) {
+    slider->AnimationProgressed(slider->animation_.get());
+    slider->fixed_needs_resize_ = false;
+  }
 }
diff --git a/chrome/browser/gtk/slide_animator_gtk.h b/chrome/browser/gtk/slide_animator_gtk.h
index bb8d0a9..984827b 100644
--- a/chrome/browser/gtk/slide_animator_gtk.h
+++ b/chrome/browser/gtk/slide_animator_gtk.h
@@ -5,6 +5,11 @@
 // A helper class for animating the display of native widget content.
 // Currently only handle vertical sliding, but could be extended to handle
 // horizontal slides or other types of animations.
+//
+// NOTE: This does not handle clipping. If you are not careful, you will
+// wind up with visibly overlapping widgets. If you need clipping, you can
+// extend the constructor to take an option to give |fixed| its own GdkWindow
+// (via gtk_fixed_set_has_window).
 
 #ifndef CHROME_BROWSER_GTK_SLIDE_ANIMATOR_GTK_H_
 #define CHROME_BROWSER_GTK_SLIDE_ANIMATOR_GTK_H_
@@ -86,6 +91,12 @@ class SlideAnimatorGtk : public AnimationDelegate {
   // If true, we should resize |widget_| on the next "size-allocate" event that
   // is received by |child_|. See the comment in SlideAnimatorGtk constructor.
   bool fixed_needs_resize_;
+
+  // We need to move the child widget to (0, -height), but we don't know its
+  // height until it has been allocated. This variable will be true until the
+  // child widget has been allocated, at which point we will move it, and then
+  // set this variable to false to indicate it should not be moved again.
+  bool child_needs_move_;
 };
 
 #endif  // CHROME_BROWSER_GTK_SLIDE_ANIMATOR_GTK_H_