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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/command_line.h"
@@ skipping 94 matching lines @@
   return browser::FindBrowserWithWebContents(contents);
 }
 
 // Height of the location bar's round corner region.
 const int kBorderRoundCornerHeight = 5;
 // Width of location bar's round corner region.
 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;
 const int kTouchEdgeItemPadding = kTouchItemPadding;
 
 // Extra padding for the height of the omnibox in search mode.
 const int kSearchEditHeightPadding = 2;
 
 }  // namespace
 
@@ skipping 782 matching lines @@
     paint.setStyle(SkPaint::kFill_Style);
     paint.setAntiAlias(true);
     // TODO(jamescook): Make the corners of the dropdown match the corners of
     // the omnibox.
     const SkScalar radius(SkIntToScalar(kBorderCornerRadius));
     bounds.Inset(kNormalHorizontalEdgeThickness, 0);
     canvas->sk_canvas()->drawRoundRect(gfx::RectToSkRect(bounds), radius,
                                        radius, paint);
     if (action_box_button_view_)
       PaintActionBoxBackground(canvas, bounds);
+    PaintPageActionBackgrounds(canvas);
   } else {
     canvas->FillRect(bounds, color);
   }
 
   if (show_focus_rect_ && HasFocus()) {
     gfx::Rect r = location_entry_view_->bounds();
     // TODO(jamescook): Is this still needed?
 #if defined(OS_WIN)
     r.Inset(-1,  -1);
 #else
@@ skipping 286 matching lines @@
   bounds.set_width(kBorderCornerRadius);
   canvas->DrawRect(bounds, paint);
 
   // Draw left border.
   gfx::Point line_end(bounds.origin());
   line_end.Offset(0, bounds.height());
   canvas->DrawLine(bounds.origin(), line_end,
                    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;
 }
 
 bool LocationBarView::SkipDefaultKeyEventProcessing(const ui::KeyEvent& event) {
 #if defined(OS_WIN)
   if (views::FocusManager::IsTabTraversalKeyEvent(event)) {
     if (location_entry_->model()->popup_model()->IsOpen()) {
       // Return true so that the edit sees the tab and moves the selection.
       return true;
@@ skipping 272 matching lines @@
 }
 
 void LocationBarView::CleanupFadeAnimation() {
   // Since we're no longer animating we don't need our layer.
   SetPaintToLayer(false);
   // Bubble labels don't need a transparent background anymore.
   ev_bubble_view_->SetLabelBackgroundColor(SK_ColorWHITE);
   selected_keyword_view_->SetLabelBackgroundColor(SK_ColorWHITE);
 }
 #endif  // USE_AURA