Gtk: add a separator at the right of the browser actions toolbar.

The spacing changes in browser_toolbar_gtk are necessary to get the spacing between the separator and the page menu correct. It shouldn't affect the spacing in any other circumstance.

BUG=none
TEST=visual

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

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

7 files changed, 104 insertions, 76 deletions

diff --git a/chrome/browser/gtk/bookmark_bar_gtk.cc b/chrome/browser/gtk/bookmark_bar_gtk.cc
index 1b675d6..a2e7f0e 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bar_gtk.cc
@@ -71,6 +71,9 @@ const int kBookmarkBarMinimumHeight = 4;
 // Left-padding for the instructional text.
 const int kInstructionsPadding = 6;
 
+// Padding to left and right of the "Other Bookmarks" button.
+const int kOtherBookmarksPadding = 2;
+
 // Middle color of the separator gradient.
 const double kSeparatorColor[] =
     { 194.0 / 255.0, 205.0 / 255.0, 212.0 / 212.0 };
@@ -254,18 +257,16 @@ void BookmarkBarGtk::Init(Profile* profile) {
   g_signal_connect(bookmark_toolbar_.get(), "drag-data-received",
                    G_CALLBACK(&OnDragReceivedThunk), this);
 
-  GtkWidget* vseparator = gtk_vseparator_new();
+  GtkWidget* vseparator = theme_provider_->CreateToolbarSeparator();
   gtk_box_pack_start(GTK_BOX(bookmark_hbox_), vseparator,
                      FALSE, FALSE, 0);
-  g_signal_connect(vseparator, "expose-event",
-                   G_CALLBACK(OnSeparatorExposeThunk), this);
 
   // We pack the button manually (rather than using gtk_button_set_*) so that
   // we can have finer control over its label.
   other_bookmarks_button_ = theme_provider_->BuildChromeButton();
   ConnectFolderButtonEvents(other_bookmarks_button_);
   gtk_box_pack_start(GTK_BOX(bookmark_hbox_), other_bookmarks_button_,
-                     FALSE, FALSE, 0);
+                     FALSE, FALSE, kOtherBookmarksPadding);
 
   sync_error_button_ = theme_provider_->BuildChromeButton();
   gtk_button_set_image(
@@ -1271,51 +1272,6 @@ void BookmarkBarGtk::OnParentSizeAllocate(GtkWidget* widget,
   }
 }
 
-gboolean BookmarkBarGtk::OnSeparatorExpose(GtkWidget* widget,
-                                           GdkEventExpose* event) {
-  if (theme_provider_->UseGtkTheme())
-    return FALSE;
-
-  cairo_t* cr = gdk_cairo_create(GDK_DRAWABLE(widget->window));
-  gdk_cairo_rectangle(cr, &event->area);
-  cairo_clip(cr);
-
-  GdkColor bottom_color =
-      theme_provider_->GetGdkColor(BrowserThemeProvider::COLOR_TOOLBAR);
-  double bottom_color_rgb[] = {
-      static_cast<double>(bottom_color.red / 257) / 255.0,
-      static_cast<double>(bottom_color.green / 257) / 255.0,
-      static_cast<double>(bottom_color.blue / 257) / 255.0, };
-
-  cairo_pattern_t* pattern =
-      cairo_pattern_create_linear(widget->allocation.x, widget->allocation.y,
-                                  widget->allocation.x,
-                                  widget->allocation.y +
-                                  widget->allocation.height);
-  cairo_pattern_add_color_stop_rgb(
-      pattern, 0.0,
-      kTopBorderColor[0], kTopBorderColor[1], kTopBorderColor[2]);
-  cairo_pattern_add_color_stop_rgb(
-      pattern, 0.5,
-      kSeparatorColor[0], kSeparatorColor[1], kSeparatorColor[2]);
-  cairo_pattern_add_color_stop_rgb(
-      pattern, 1.0,
-      bottom_color_rgb[0], bottom_color_rgb[1], bottom_color_rgb[2]);
-  cairo_set_source(cr, pattern);
-
-  double start_x = 0.5 + widget->allocation.x;
-  cairo_new_path(cr);
-  cairo_set_line_width(cr, 1.0);
-  cairo_move_to(cr, start_x, widget->allocation.y);
-  cairo_line_to(cr, start_x,
-                    widget->allocation.y + widget->allocation.height);
-  cairo_stroke(cr);
-  cairo_destroy(cr);
-  cairo_pattern_destroy(pattern);
-
-  return TRUE;
-}
-
 void BookmarkBarGtk::OnThrobbingWidgetDestroy(GtkWidget* widget) {
   SetThrobbingWidget(NULL);
 }
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.h b/chrome/browser/gtk/bookmark_bar_gtk.h
index 66da4f4..1a171ec 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/gtk/bookmark_bar_gtk.h
@@ -250,10 +250,6 @@ class BookmarkBarGtk : public AnimationDelegate,
                        GdkEventExpose*);
   CHROMEGTK_CALLBACK_0(BookmarkBarGtk, void, OnEventBoxDestroy);
 
-  // GtkVSeparator callbacks.
-  CHROMEGTK_CALLBACK_1(BookmarkBarGtk, gboolean, OnSeparatorExpose,
-                       GdkEventExpose*);
-
   // Callbacks on our parent widget.
   CHROMEGTK_CALLBACK_1(BookmarkBarGtk, void, OnParentSizeAllocate,
                        GtkAllocation*);
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index 6f608c4..954b5f9 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -41,6 +41,10 @@ const int kButtonSize = 29;
 // use their maximum allowed size.
 const int kButtonPadding = 3;
 
+// The padding to the left, top, and bottom of the browser actions toolbar
+// separator.
+const int kSeparatorPadding = 2;
+
 const char* kDragTarget = "application/x-chrome-browseraction";
 
 GtkTargetEntry GetDragTargetEntry() {
@@ -299,6 +303,7 @@ BrowserActionsToolbarGtk::BrowserActionsToolbarGtk(Browser* browser)
       hbox_(gtk_hbox_new(FALSE, 0)),
       button_hbox_(gtk_chrome_shrinkable_hbox_new(TRUE, FALSE, kButtonPadding)),
       overflow_button_(browser->profile()),
+      separator_(theme_provider_->CreateToolbarSeparator()),
       drag_button_(NULL),
       drop_index_(-1),
       resize_animation_(this),
@@ -332,6 +337,7 @@ BrowserActionsToolbarGtk::BrowserActionsToolbarGtk(Browser* browser)
   gtk_box_pack_start(GTK_BOX(hbox_.get()), button_hbox_, TRUE, TRUE, 0);
   gtk_box_pack_start(GTK_BOX(hbox_.get()), overflow_button_.widget(),
                      FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(hbox_.get()), separator_, FALSE, FALSE, 0);
 
   model_ = extension_service->toolbar_model();
   model_->AddObserver(this);
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.h b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
index 11b3537..5d5f342 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
@@ -149,6 +149,9 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
   OverflowButton overflow_button_;
   scoped_ptr<MenuGtk> overflow_menu_;
 
+  // The vertical separator between the overflow button and the page/app menus.
+  GtkWidget* separator_;
+
   // The button that is currently being dragged, or NULL.
   BrowserActionButton* drag_button_;
 
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index 6bdb18e..131c213 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -60,14 +60,11 @@ const int kToolbarHeight = 29;
 // Padding within the toolbar above the buttons and location bar.
 const int kTopPadding = 4;
 
-// Exterior padding on left/right of toolbar.
-const int kLeftRightPadding = 2;
-
 // Height of the toolbar in pixels when we only show the location bar.
 const int kToolbarHeightLocationBarOnly = kToolbarHeight - 2;
 
 // Interior spacing between toolbar widgets.
-const int kToolbarWidgetSpacing = 4;
+const int kToolbarWidgetSpacing = 2;
 
 }  // namespace
 
@@ -129,7 +126,7 @@ void BrowserToolbarGtk::Init(Profile* profile,
   if (!theme_provider_->UseGtkTheme())
     gtk_event_box_set_visible_window(GTK_EVENT_BOX(event_box_), FALSE);
 
-  toolbar_ = gtk_hbox_new(FALSE, kToolbarWidgetSpacing);
+  toolbar_ = gtk_hbox_new(FALSE, 0);
   alignment_ = gtk_alignment_new(0.0, 0.0, 1.0, 1.0);
   UpdateForBookmarkBarVisibility(false);
   g_signal_connect(alignment_, "expose-event",
@@ -156,7 +153,8 @@ void BrowserToolbarGtk::Init(Profile* profile,
                      FALSE, 0);
   g_signal_connect(forward_->widget(), "clicked",
                    G_CALLBACK(OnButtonClickThunk), this);
-  gtk_box_pack_start(GTK_BOX(toolbar_), back_forward_hbox_, FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(toolbar_), back_forward_hbox_, FALSE, FALSE,
+                     kToolbarWidgetSpacing);
 
   reload_.reset(BuildToolbarButton(IDR_RELOAD, IDR_RELOAD_P, IDR_RELOAD_H, 0,
                                    IDR_BUTTON_MASK,
@@ -185,7 +183,7 @@ void BrowserToolbarGtk::Init(Profile* profile,
   g_signal_connect(location_hbox, "expose-event",
                    G_CALLBACK(OnLocationHboxExposeThunk), this);
   gtk_box_pack_start(GTK_BOX(toolbar_), location_hbox, TRUE, TRUE,
-                     ShouldOnlyShowLocation() ? 1 : 0);
+      kToolbarWidgetSpacing + (ShouldOnlyShowLocation() ? 1 : 0));
 
   if (!ShouldOnlyShowLocation()) {
     actions_toolbar_.reset(new BrowserActionsToolbarGtk(browser_));
@@ -218,7 +216,8 @@ void BrowserToolbarGtk::Init(Profile* profile,
   app_menu_.reset(new MenuGtk(this, &app_menu_model_));
   gtk_box_pack_start(GTK_BOX(menus_hbox_), chrome_menu, FALSE, FALSE, 0);
 
-  gtk_box_pack_start(GTK_BOX(toolbar_), menus_hbox_, FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(toolbar_), menus_hbox_, FALSE, FALSE,
+                     kToolbarWidgetSpacing);
 
   if (ShouldOnlyShowLocation()) {
     gtk_widget_show(event_box_);
@@ -277,7 +276,7 @@ void BrowserToolbarGtk::UpdateForBookmarkBarVisibility(
   gtk_alignment_set_padding(GTK_ALIGNMENT(alignment_),
       ShouldOnlyShowLocation() ? 0 : kTopPadding,
       !show_bottom_padding || ShouldOnlyShowLocation() ? 0 : kTopPadding,
-      kLeftRightPadding, kLeftRightPadding);
+      0, 0);
 }
 
 void BrowserToolbarGtk::ShowPageMenu() {
@@ -482,7 +481,8 @@ CustomDrawButton* BrowserToolbarGtk::BuildToolbarButton(
   g_signal_connect(button->widget(), "clicked",
                    G_CALLBACK(OnButtonClickThunk), this);
 
-  gtk_box_pack_start(GTK_BOX(toolbar_), button->widget(), FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(toolbar_), button->widget(), FALSE, FALSE,
+                     kToolbarWidgetSpacing);
   return button;
 }
 
diff --git a/chrome/browser/gtk/gtk_theme_provider.cc b/chrome/browser/gtk/gtk_theme_provider.cc
index 4227724..4a229c6 100644
--- a/chrome/browser/gtk/gtk_theme_provider.cc
+++ b/chrome/browser/gtk/gtk_theme_provider.cc
@@ -55,10 +55,20 @@ const double kMinimumLuminanceDifference = 0.1;
 // calculate the border color in GTK theme mode.
 const int kBgWeight = 3;
 
+// Padding to left, top and bottom of vertical separators.
+const int kSeparatorPadding = 2;
+
 // Default color for links on the NTP when the GTK+ theme doesn't define a
 // link color. Constant taken from gtklinkbutton.c.
 const GdkColor kDefaultLinkColor = { 0, 0, 0, 0xeeee };
 
+// Middle color of the separator gradient.
+const double kMidSeparatorColor[] =
+    { 194.0 / 255.0, 205.0 / 255.0, 212.0 / 212.0 };
+// Top color of the separator gradient.
+const double kTopSeparatorColor[] =
+    { 222.0 / 255.0, 234.0 / 255.0, 248.0 / 255.0 };
+
 // Converts a GdkColor to a SkColor.
 SkColor GdkToSkColor(const GdkColor* color) {
   return SkColorSetRGB(color->red >> 8,
@@ -211,11 +221,23 @@ GtkWidget* GtkThemeProvider::BuildChromeButton() {
   gtk_chrome_button_set_use_gtk_rendering(GTK_CHROME_BUTTON(button), use_gtk_);
   chrome_buttons_.push_back(button);
 
-  g_signal_connect(button, "destroy", G_CALLBACK(OnDestroyChromeButton),
+  g_signal_connect(button, "destroy", G_CALLBACK(OnDestroyChromeButtonThunk),
                    this);
   return button;
 }
 
+GtkWidget* GtkThemeProvider::CreateToolbarSeparator() {
+  GtkWidget* separator = gtk_vseparator_new();
+  GtkWidget* alignment = gtk_alignment_new(0, 0, 1, 1);
+  gtk_alignment_set_padding(GTK_ALIGNMENT(alignment),
+      kSeparatorPadding, kSeparatorPadding, kSeparatorPadding, 0);
+  gtk_container_add(GTK_CONTAINER(alignment), separator);
+
+  g_signal_connect(separator, "expose-event",
+                   G_CALLBACK(OnSeparatorExposeThunk), this);
+  return alignment;
+}
+
 bool GtkThemeProvider::UseGtkTheme() const {
   return use_gtk_;
 }
@@ -729,11 +751,53 @@ SkBitmap* GtkThemeProvider::GenerateTabImage(int base_id) const {
       bg_tint, 0, 0, bg_tint.width(), bg_tint.height()));
 }
 
-void GtkThemeProvider::OnDestroyChromeButton(GtkWidget* button,
-                                             GtkThemeProvider* provider) {
+void GtkThemeProvider::OnDestroyChromeButton(GtkWidget* button) {
   std::vector<GtkWidget*>::iterator it =
-      find(provider->chrome_buttons_.begin(), provider->chrome_buttons_.end(),
-           button);
-  if (it != provider->chrome_buttons_.end())
-    provider->chrome_buttons_.erase(it);
+      find(chrome_buttons_.begin(), chrome_buttons_.end(), button);
+  if (it != chrome_buttons_.end())
+    chrome_buttons_.erase(it);
+}
+
+gboolean GtkThemeProvider::OnSeparatorExpose(GtkWidget* widget,
+                                             GdkEventExpose* event) {
+  if (UseGtkTheme())
+    return FALSE;
+
+  cairo_t* cr = gdk_cairo_create(GDK_DRAWABLE(widget->window));
+  gdk_cairo_rectangle(cr, &event->area);
+  cairo_clip(cr);
+
+  GdkColor bottom_color = GetGdkColor(BrowserThemeProvider::COLOR_TOOLBAR);
+  double bottom_color_rgb[] = {
+      static_cast<double>(bottom_color.red / 257) / 255.0,
+      static_cast<double>(bottom_color.green / 257) / 255.0,
+      static_cast<double>(bottom_color.blue / 257) / 255.0, };
+
+  cairo_pattern_t* pattern =
+      cairo_pattern_create_linear(widget->allocation.x, widget->allocation.y,
+                                  widget->allocation.x,
+                                  widget->allocation.y +
+                                  widget->allocation.height);
+  cairo_pattern_add_color_stop_rgb(
+      pattern, 0.0,
+      kTopSeparatorColor[0], kTopSeparatorColor[1], kTopSeparatorColor[2]);
+  cairo_pattern_add_color_stop_rgb(
+      pattern, 0.5,
+      kMidSeparatorColor[0], kMidSeparatorColor[1], kMidSeparatorColor[2]);
+  cairo_pattern_add_color_stop_rgb(
+      pattern, 1.0,
+      bottom_color_rgb[0], bottom_color_rgb[1], bottom_color_rgb[2]);
+  cairo_set_source(cr, pattern);
+
+  double start_x = 0.5 + widget->allocation.x;
+  cairo_new_path(cr);
+  cairo_set_line_width(cr, 1.0);
+  cairo_move_to(cr, start_x, widget->allocation.y);
+  cairo_line_to(cr, start_x,
+                    widget->allocation.y + widget->allocation.height);
+  cairo_stroke(cr);
+  cairo_destroy(cr);
+  cairo_pattern_destroy(pattern);
+
+  return TRUE;
 }
diff --git a/chrome/browser/gtk/gtk_theme_provider.h b/chrome/browser/gtk/gtk_theme_provider.h
index b7310ca..c5a8b5e 100644
--- a/chrome/browser/gtk/gtk_theme_provider.h
+++ b/chrome/browser/gtk/gtk_theme_provider.h
@@ -5,10 +5,12 @@
 #ifndef CHROME_BROWSER_GTK_GTK_THEME_PROVIDER_H_
 #define CHROME_BROWSER_GTK_GTK_THEME_PROVIDER_H_
 
+#include <gtk/gtk.h>
 #include <map>
 #include <string>
 #include <vector>
 
+#include "app/gtk_signal.h"
 #include "chrome/browser/browser_theme_provider.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/owned_widget_gtk.h"
@@ -17,10 +19,6 @@
 class CairoCachedSurface;
 class Profile;
 
-typedef struct _GdkDisplay GdkDisplay;
-typedef struct _GtkStyle GtkStyle;
-typedef struct _GtkWidget GtkWidget;
-
 // Specialization of BrowserThemeProvider which supplies system colors.
 class GtkThemeProvider : public BrowserThemeProvider,
                          public NotificationObserver {
@@ -57,6 +55,9 @@ class GtkThemeProvider : public BrowserThemeProvider,
   // away.
   GtkWidget* BuildChromeButton();
 
+  // FIXME
+  GtkWidget* CreateToolbarSeparator();
+
   // Whether we should use the GTK system theme.
   bool UseGtkTheme() const;
 
@@ -160,8 +161,10 @@ class GtkThemeProvider : public BrowserThemeProvider,
 
   // A notification from the GtkChromeButton GObject destructor that we should
   // remove it from our internal list.
-  static void OnDestroyChromeButton(GtkWidget* button,
-                                    GtkThemeProvider* provider);
+  CHROMEGTK_CALLBACK_0(GtkThemeProvider, void, OnDestroyChromeButton);
+
+  CHROMEGTK_CALLBACK_1(GtkThemeProvider, gboolean, OnSeparatorExpose,
+                       GdkEventExpose*);
 
   // Whether we should be using gtk rendering.
   bool use_gtk_;