Give request-to-act badges a grey background, and increase spacing to make it fit.

chrome/browser/ui/views/location_bar/location_bar_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index 8e9f3b367ce581707ff090ca0b069806793cbecd..8af51eafd68b4ff1a7a33b71428fc251d12b35d5 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -112,7 +112,7 @@ const int kBorderRoundCornerWidth = 4;
 // Radius of the round corners inside the location bar.
 const int kBorderCornerRadius = 2;
 
-const int kDesktopItemPadding = 3;
+const int kDesktopItemPadding = 9;

[Peter Kasting, 2012/08/24 23:08:28]

Is increasing this for all items really the right thing to do?  Or should we
have different spacing for items without a background?  Or should we be drawing
faint lines between all the items, as in the mock, so the items with a
background don't look quite as different and the otherwise-big gaps aren't
weird?

[Jeffrey Yasskin, 2012/08/29 00:37:50]

Aaron asked for the extra spacing on the star in http://crbug.com/135460. I
haven't seen anyone explicitly say what should happen for the other, non-star,
icons.
The mocks only include the faint line between the star and the action box:
https://docs.google.com/a/google.com/folder/d/0B21JpVYxVLsASVVyZlJDb3d3a2M/ed....

 const int kDesktopEdgeItemPadding = kDesktopItemPadding;
 
 const int kTouchItemPadding = 8;
@@ -915,6 +915,7 @@ void LocationBarView::OnPaint(gfx::Canvas* canvas) {
                                        radius, paint);
     if (action_box_button_view_)
       PaintActionBoxBackground(canvas, bounds);
+    PaintPageActionBackgrounds(canvas);
   } else {
     canvas->FillRect(bounds, color);
   }
@@ -1221,6 +1222,24 @@ void LocationBarView::PaintActionBoxBackground(gfx::Canvas* canvas,
                    action_box_button_view_->GetBorderColor());
 }
 
+void LocationBarView::PaintPageActionBackgrounds(gfx::Canvas* canvas) {
+  for (size_t i = 0; i < page_action_views_.size(); ++i) {

[Peter Kasting, 2012/08/24 23:08:28]

Nit: Since all you do with i is to get page_action_views_[i], use an iterator.

[Jeffrey Yasskin, 2012/08/29 00:37:50]

Done.

+    gfx::Rect bounds = page_action_views_[i]->bounds();
+    int horizontal_padding = GetItemPadding() -
+        page_action_views_[i]->GetBuiltInHorizontalPadding();
+    // These offsets place the top and bottom edges on top of the darkest line
+    // in the location bar border, and the side edges on the pixels exactly
+    // between two page actions.  If the border image changes, these offsets
+    // will have to change to match.
+    bounds.Inset(-horizontal_padding / 2, -1,

[Peter Kasting, 2012/08/24 23:08:28]

Nit: Don't use -1s here.  Split the kVerticalEdgeThickness constant into two
constants, kVisibleEdgeThickness (= 1) and kVerticalPaddingOutsideEdge (= 2) and
define the former in LocationBar instead of here (see extension_action.cc for
why).  Then subtract off that visible edge thickness constant here.

[Jeffrey Yasskin, 2012/08/29 00:37:50]

In Views, the visible edge thickness is 1 on the top and 2 on the bottom. Ah,
but it would be 3 on both sides, with the darkest pixel in the center, were it
not for the drawRoundRect call at
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/ui/views/lo...,
and the fact that the top pixel of omnibox_c.png is transparent.

In the mocks, the top request-to-act border is at the top of the 3-pixel visible
location-bar border, but the bottom border is also at the top of the 2-pixel
visible location-bar border.

+                 -horizontal_padding / 2, -1);
+    ExtensionAction* action =
+        page_action_views_[i]->image_view()->page_action();
+    action->PaintBackground(canvas, bounds,
+                            SessionID::IdForTab(GetTabContents()));
+  }
+}
+
 std::string LocationBarView::GetClassName() const {
   return kViewClassName;
 }