Squash the bookmark bar into the toolbar by 4 pixels.

On windows they do this by having the two views overlap. We can't so instead move the bottom 4 pixels of padding from the toolbar widget into the bookmark bar widget. When the bookmark bar is closed/hidden, it still shows those 4 pixels. When it opens, it is able to make use of those 4 pixels. Since we get 4 extra pixels, we can make the overall height of the bookmark bar 4 pixels less.

BUG=18570

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

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

6 files changed, 67 insertions, 64 deletions

diff --git a/chrome/browser/gtk/bookmark_bar_gtk.cc b/chrome/browser/gtk/bookmark_bar_gtk.cc
index 867d39e..d2dc5eb 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bar_gtk.cc
@@ -39,9 +39,13 @@
 
 namespace {
 
-// The height of the bar.
+// The showing height of the bar.
 const int kBookmarkBarHeight = 29;
 
+// The height of the bar when it is "hidden". It is never completely hidden
+// because even when it is closed it forms the bottom few pixels of the toolbar.
+const int kBookmarkBarMinimumHeight = 4;
+
 // Left-padding for the instructional text.
 const int kInstructionsPadding = 6;
 
@@ -172,9 +176,9 @@ void BookmarkBarGtk::Init(Profile* profile) {
   g_signal_connect(instructions_, "drag-data-received",
                    G_CALLBACK(&OnDragReceived), this);
 
-  gtk_widget_set_app_paintable(bookmark_hbox_, TRUE);
-  g_signal_connect(G_OBJECT(bookmark_hbox_), "expose-event",
-                   G_CALLBACK(&OnHBoxExpose), this);
+  gtk_widget_set_app_paintable(widget(), TRUE);
+  g_signal_connect(G_OBJECT(widget()), "expose-event",
+                   G_CALLBACK(&OnEventBoxExpose), this);
 
   bookmark_toolbar_.Own(gtk_toolbar_new());
   SetToolBarStyle();
@@ -208,19 +212,15 @@ void BookmarkBarGtk::Init(Profile* profile) {
   gtk_box_pack_start(GTK_BOX(bookmark_hbox_), other_bookmarks_button_,
                      FALSE, FALSE, 0);
 
-  gtk_widget_set_size_request(event_box_.get(), -1, 0);
+  gtk_widget_set_size_request(event_box_.get(), -1, kBookmarkBarMinimumHeight);
 
   slide_animation_.reset(new SlideAnimation(this));
 
   ViewIDUtil::SetID(widget(), VIEW_ID_BOOKMARK_BAR);
 }
 
-void BookmarkBarGtk::AddBookmarkbarToBox(GtkWidget* box) {
-  gtk_box_pack_start(GTK_BOX(box), widget(), FALSE, FALSE, 0);
-}
-
 void BookmarkBarGtk::Show(bool animate) {
-  gtk_widget_show_all(widget());
+  gtk_widget_show_all(bookmark_hbox_);
   if (animate) {
     slide_animation_->Show();
   } else {
@@ -242,7 +242,7 @@ void BookmarkBarGtk::Hide(bool animate) {
   if (slide_animation_->IsShowing() && animate) {
     slide_animation_->Hide();
   } else {
-    gtk_widget_hide(widget());
+    gtk_widget_hide(bookmark_hbox_);
     slide_animation_->Reset(0);
     AnimationProgressed(slide_animation_.get());
   }
@@ -398,16 +398,17 @@ bool BookmarkBarGtk::IsAlwaysShown() {
 void BookmarkBarGtk::AnimationProgressed(const Animation* animation) {
   DCHECK_EQ(animation, slide_animation_.get());
 
-  gtk_widget_set_size_request(event_box_.get(), -1,
-                              animation->GetCurrentValue() *
-                              kBookmarkBarHeight);
+  gint height = animation->GetCurrentValue() *
+      (kBookmarkBarHeight - kBookmarkBarMinimumHeight) +
+      kBookmarkBarMinimumHeight;
+  gtk_widget_set_size_request(event_box_.get(), -1, height);
 }
 
 void BookmarkBarGtk::AnimationEnded(const Animation* animation) {
   DCHECK_EQ(animation, slide_animation_.get());
 
   if (!slide_animation_->IsShowing())
-    gtk_widget_hide(widget());
+    gtk_widget_hide(bookmark_hbox_);
 }
 
 void BookmarkBarGtk::Observe(NotificationType type,
@@ -810,9 +811,9 @@ void BookmarkBarGtk::OnDragReceived(GtkWidget* widget,
 }
 
 // static
-gboolean BookmarkBarGtk::OnHBoxExpose(GtkWidget* widget,
-                                      GdkEventExpose* event,
-                                      BookmarkBarGtk* bar) {
+gboolean BookmarkBarGtk::OnEventBoxExpose(GtkWidget* widget,
+                                          GdkEventExpose* event,
+                                          BookmarkBarGtk* bar) {
   // Paint the background theme image.
   cairo_t* cr = gdk_cairo_create(GDK_DRAWABLE(widget->window));
   cairo_rectangle(cr, event->area.x, event->area.y,
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.h b/chrome/browser/gtk/bookmark_bar_gtk.h
index f779a52..0f44bf6 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/gtk/bookmark_bar_gtk.h
@@ -54,9 +54,6 @@ class BookmarkBarGtk : public AnimationDelegate,
   // Create the contents of the bookmark bar.
   void Init(Profile* profile);
 
-  // Adds this GTK toolbar into a sizing box.
-  void AddBookmarkbarToBox(GtkWidget* box);
-
   // Whether the current page is the New Tag Page (which requires different
   // rendering).
   bool OnNewTabPage();
@@ -185,9 +182,9 @@ class BookmarkBarGtk : public AnimationDelegate,
                              guint target_type, guint time,
                              BookmarkBarGtk* bar);
 
-  // GtkHBox callbacks.
-  static gboolean OnHBoxExpose(GtkWidget* widget, GdkEventExpose* event,
-                               BookmarkBarGtk* window);
+  // GtkEventBox callbacks.
+  static gboolean OnEventBoxExpose(GtkWidget* widget, GdkEventExpose* event,
+                                   BookmarkBarGtk* window);
 
   // GtkVSeparator callbacks.
   static gboolean OnSeparatorExpose(GtkWidget* widget, GdkEventExpose* event,
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index a667a32..808e5c2 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -45,11 +45,14 @@
 
 namespace {
 
-// Height of the toolbar in pixels.
-const int kToolbarHeight = 37;
+// Height of the toolbar in pixels (not counting padding).
+const int kToolbarHeight = 29;
+
+// Padding within the toolbar above the buttons and location bar.
+const int kTopPadding = 4;
 
 // Height of the toolbar in pixels when we only show the location bar.
-const int kToolbarHeightLocationBarOnly = kToolbarHeight - 10;
+const int kToolbarHeightLocationBarOnly = kToolbarHeight - 2;
 
 // Interior spacing between toolbar widgets.
 const int kToolbarWidgetSpacing = 4;
@@ -147,16 +150,18 @@ void BrowserToolbarGtk::Init(Profile* profile,
     gtk_event_box_set_visible_window(GTK_EVENT_BOX(event_box_), FALSE);
 
   toolbar_ = gtk_hbox_new(FALSE, kToolbarWidgetSpacing);
-  gtk_container_add(GTK_CONTAINER(event_box_), toolbar_);
-  gtk_container_set_border_width(GTK_CONTAINER(toolbar_),
-      ShouldOnlyShowLocation() ? 0 : 4);
+  GtkWidget* alignment = gtk_alignment_new(0.0, 0.0, 1.0, 1.0);
+  gtk_alignment_set_padding(GTK_ALIGNMENT(alignment),
+      ShouldOnlyShowLocation() ? 0 : kTopPadding, 0, 0, 0);
+  g_signal_connect(alignment, "expose-event",
+                   G_CALLBACK(&OnAlignmentExpose), this);
+  gtk_container_add(GTK_CONTAINER(event_box_), alignment);
+  gtk_container_add(GTK_CONTAINER(alignment), toolbar_);
   // Force the height of the toolbar so we get the right amount of padding
   // above and below the location bar. -1 for width means "let GTK do its
   // normal sizing".
   gtk_widget_set_size_request(toolbar_, -1, ShouldOnlyShowLocation() ?
       kToolbarHeightLocationBarOnly : kToolbarHeight);
-  g_signal_connect(toolbar_, "expose-event",
-                   G_CALLBACK(&OnToolbarExpose), this);
 
   // A GtkAccelGroup is not InitiallyUnowned, meaning we get a real reference
   // count starting at one.  We don't want the lifetime to be managed by the
@@ -259,6 +264,7 @@ void BrowserToolbarGtk::Init(Profile* profile,
 
   if (ShouldOnlyShowLocation()) {
     gtk_widget_show(event_box_);
+    gtk_widget_show(alignment);
     gtk_widget_show(toolbar_);
     gtk_widget_show_all(location_hbox);
     gtk_widget_hide(star_->widget());
@@ -289,10 +295,6 @@ void BrowserToolbarGtk::SetViewIDs() {
   ViewIDUtil::SetID(app_menu_button_.get(), VIEW_ID_APP_MENU);
 }
 
-void BrowserToolbarGtk::AddToolbarToBox(GtkWidget* box) {
-  gtk_box_pack_start(GTK_BOX(box), event_box_, FALSE, FALSE, 0);
-}
-
 void BrowserToolbarGtk::Show() {
   gtk_widget_show(toolbar_);
 }
@@ -528,9 +530,9 @@ void BrowserToolbarGtk::ChangeActiveMenu(GtkWidget* active_menu,
 }
 
 // static
-gboolean BrowserToolbarGtk::OnToolbarExpose(GtkWidget* widget,
-                                            GdkEventExpose* e,
-                                            BrowserToolbarGtk* toolbar) {
+gboolean BrowserToolbarGtk::OnAlignmentExpose(GtkWidget* widget,
+                                              GdkEventExpose* e,
+                                              BrowserToolbarGtk* toolbar) {
   cairo_t* cr = gdk_cairo_create(GDK_DRAWABLE(widget->window));
   cairo_rectangle(cr, e->area.x, e->area.y, e->area.width, e->area.height);
   cairo_clip(cr);
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.h b/chrome/browser/gtk/browser_toolbar_gtk.h
index ebf6528..35e0f85 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_toolbar_gtk.h
@@ -46,9 +46,6 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   // Set the various widgets' ViewIDs.
   void SetViewIDs();
 
-  // Adds this GTK toolbar into a sizing box.
-  void AddToolbarToBox(GtkWidget* box);
-
   void Show();
   void Hide();
 
@@ -119,8 +116,9 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
   void ChangeActiveMenu(GtkWidget* active_menu, guint timestamp);
 
   // Gtk callback for the "expose-event" signal.
-  static gboolean OnToolbarExpose(GtkWidget* widget, GdkEventExpose* e,
-                                  BrowserToolbarGtk* toolbar);
+  // The alignment contains the toolbar.
+  static gboolean OnAlignmentExpose(GtkWidget* widget, GdkEventExpose* e,
+                                    BrowserToolbarGtk* toolbar);
   static gboolean OnLocationHboxExpose(GtkWidget* omnibox_hbox,
                                        GdkEventExpose* e,
                                        BrowserToolbarGtk* toolbar);
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 190cc74..e2fce5d 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -1051,7 +1051,7 @@ void BrowserWindowGtk::UserChangedTheme() {
 
 int BrowserWindowGtk::GetExtraRenderViewHeight() const {
   int sum = infobar_container_->TotalHeightOfAnimatingBars();
-  if (bookmark_bar_->IsAnimating())
+  if (IsBookmarkBarSupported() && bookmark_bar_->IsAnimating())
     sum += bookmark_bar_->GetHeight();
   if (download_shelf_.get() && download_shelf_->IsClosing())
     sum += download_shelf_->GetHeight();
@@ -1194,8 +1194,7 @@ void BrowserWindowGtk::MaybeShowBookmarkBar(TabContents* contents,
 
   bool show_bar = false;
 
-  if (browser_->SupportsWindowFeature(Browser::FEATURE_BOOKMARKBAR)
-      && contents) {
+  if (IsBookmarkBarSupported() && contents) {
     bookmark_bar_->SetProfile(contents->profile());
     bookmark_bar_->SetPageNavigator(contents);
     show_bar = true;
@@ -1278,7 +1277,8 @@ void BrowserWindowGtk::OnStateChanged(GdkWindowState state,
       UpdateCustomFrame();
       toolbar_->Hide();
       tabstrip_->Hide();
-      bookmark_bar_->Hide(false);
+      if (IsBookmarkBarSupported())
+        bookmark_bar_->Hide(false);
       if (extension_shelf_.get())
         extension_shelf_->Hide();
     } else {
@@ -1512,16 +1512,21 @@ void BrowserWindowGtk::InitWidgets() {
 
   toolbar_.reset(new BrowserToolbarGtk(browser_.get(), this));
   toolbar_->Init(browser_->profile(), window_);
-  toolbar_->AddToolbarToBox(content_vbox_);
+  gtk_box_pack_start(GTK_BOX(content_vbox_), toolbar_->widget(),
+                     FALSE, FALSE, 0);
 #if defined(OS_CHROMEOS)
   if (browser_->type() == Browser::TYPE_NORMAL && has_compact_nav_bar) {
     gtk_widget_hide(toolbar_->widget());
   }
 #endif
 
-  bookmark_bar_.reset(new BookmarkBarGtk(browser_->profile(), browser_.get(),
-                                         this));
-  bookmark_bar_->AddBookmarkbarToBox(content_vbox_);
+  if (IsBookmarkBarSupported()) {
+    bookmark_bar_.reset(new BookmarkBarGtk(browser_->profile(), browser_.get(),
+                                           this));
+    gtk_box_pack_start(GTK_BOX(content_vbox_), bookmark_bar_->widget(),
+                       FALSE, FALSE, 0);
+    gtk_widget_show(bookmark_bar_->widget());
+  }
 
   if (IsExtensionShelfSupported()) {
     extension_shelf_.reset(new ExtensionShelfGtk(browser()->profile(),
@@ -1985,27 +1990,26 @@ void BrowserWindowGtk::HideUnsupportedWindowFeatures() {
   if (!IsToolbarSupported())
     toolbar_->Hide();
 
-  if (!IsBookmarkBarSupported())
-    bookmark_bar_->Hide(false);
-
   if (!IsExtensionShelfSupported() && extension_shelf_.get())
     extension_shelf_->Hide();
+
+  // If the bookmark bar is unsupported, then we never create it.
 }
 
-bool BrowserWindowGtk::IsTabStripSupported() {
+bool BrowserWindowGtk::IsTabStripSupported() const {
   return browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP);
 }
 
-bool BrowserWindowGtk::IsToolbarSupported() {
+bool BrowserWindowGtk::IsToolbarSupported() const {
   return browser_->SupportsWindowFeature(Browser::FEATURE_TOOLBAR) ||
          browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR);
 }
 
-bool BrowserWindowGtk::IsBookmarkBarSupported() {
+bool BrowserWindowGtk::IsBookmarkBarSupported() const {
   return browser_->SupportsWindowFeature(Browser::FEATURE_BOOKMARKBAR);
 }
 
-bool BrowserWindowGtk::IsExtensionShelfSupported() {
+bool BrowserWindowGtk::IsExtensionShelfSupported() const {
   return browser_->SupportsWindowFeature(Browser::FEATURE_EXTENSIONSHELF);
 }
 
diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h
index 5125c1a..c97c2f6 100644
--- a/chrome/browser/gtk/browser_window_gtk.h
+++ b/chrome/browser/gtk/browser_window_gtk.h
@@ -283,10 +283,10 @@ class BrowserWindowGtk : public BrowserWindow,
 
   // Helper functions that query |browser_| concerning support for UI features
   // in this window. (For example, a popup window might not support a tabstrip).
-  bool IsTabStripSupported();
-  bool IsToolbarSupported();
-  bool IsBookmarkBarSupported();
-  bool IsExtensionShelfSupported();
+  bool IsTabStripSupported() const;
+  bool IsToolbarSupported() const;
+  bool IsBookmarkBarSupported() const;
+  bool IsExtensionShelfSupported() const;
 
   // Checks to see if the mouse pointer at |x|, |y| is over the border of the
   // custom frame (a spot that should trigger a window resize). Returns true if
@@ -316,7 +316,8 @@ class BrowserWindowGtk : public BrowserWindow,
   // The object that manages all of the widgets in the toolbar.
   scoped_ptr<BrowserToolbarGtk> toolbar_;
 
-  // The object that manages the bookmark bar.
+  // The object that manages the bookmark bar. This will be NULL if the
+  // bookmark bar is not supported.
   scoped_ptr<BookmarkBarGtk> bookmark_bar_;
 
   // The object that manages the extension shelf.