Programmatically render the omnibox border

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 b0ef71c4d045b0090f81800a40932018fa3d68c5..8a19e7b6a3b3cf920414d42bf36e6b6562211046 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -187,7 +187,7 @@ void LocationBarView::Init() {
         IMAGE_GRID(IDR_OMNIBOX_POPUP_BORDER_AND_SHADOW);
     border_painter_.reset(
         views::Painter::CreateImageGridPainter(kOmniboxPopupBorderImages));
-  } else {
+  } else if (!ui::MaterialDesignController::IsModeMaterial()) {
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
     const gfx::Insets omnibox_border_insets(14, 9, 14, 9);
     border_painter_.reset(views::Painter::CreateImagePainter(
@@ -528,11 +528,13 @@ void LocationBarView::GetAccessibleState(ui::AXViewState* state) {
 
 gfx::Size LocationBarView::GetPreferredSize() const {
   // Compute minimum height.
-  gfx::Size min_size(border_painter_->GetMinimumSize());
+  gfx::Size min_size;
   // For non-material the size of the asset determines the size of the
   // LocationBarView.
   if (ui::MaterialDesignController::IsModeMaterial())
     min_size.set_height(GetLayoutConstant(LOCATION_BAR_HEIGHT));
+  else
+    min_size = border_painter_->GetMinimumSize();
 
   if (!IsInitialized())
     return min_size;

[tdanderson, 2015/09/28 15:12:14]

With your changes it looks like we can hit this and be returning a size with a
width of 0. Is that going to be problematic?

[jonross, 2015/09/28 15:21:48]

Nope. We actually refuse to layout when not initialized. That is there so we
provide a Size to calls done before initialization.

@@ -1273,8 +1275,31 @@ void LocationBarView::PaintChildren(const ui::PaintContext& context) {
   gfx::Rect border_rect(GetContentsBounds());
   if (is_popup_mode_ && (GetHorizontalEdgeThickness() == 0))
     border_rect.Inset(-kPopupEdgeThickness, 0);
-  views::Painter::PaintPainterAt(recorder.canvas(), border_painter_.get(),
-                                 border_rect);
+
+  if (ui::MaterialDesignController::IsModeMaterial()) {

[jonross, 2015/09/28 14:55:00]

If this pattern begins to be used a lot we may want to consider creating a
border/painter to encompass it.

For example the Find popup uses assets for its bubble border still.

+    recorder.canvas()->Save();
+    SkScalar scale_factor = 1.0f / recorder.canvas()->image_scale();
+    recorder.canvas()->sk_canvas()->scale(scale_factor, scale_factor);
+
+    SkPaint paint;
+    paint.setStyle(SkPaint::Style::kStroke_Style);
+    paint.setColor(SkColorSetARGBInline(0x40, 0x0, 0x0, 0x0));

[tdanderson, 2015/09/28 15:12:14]

Is this color, the corner radius value, etc, likely to be referenced anywhere
else? If so, consider pulling the constants out into a common location.

[jonross, 2015/09/28 15:21:48]

Currently they are not. However if this becomes common I would want this
rendering pulled out into a common border/painter class.

+    paint.setStrokeWidth(0);
+    paint.setAntiAlias(true);
+
+    const float kOffset = .5f;

[tdanderson, 2015/09/28 15:12:14]

nit: 0.5f

[jonross, 2015/09/28 15:21:47]

Done.

+    gfx::RectF border_rect_f(border_rect);
+    gfx::InsetsF insets(kOffset, kOffset, kOffset, kOffset);
+    border_rect_f.Inset(insets);
+
+    const SkScalar kCornerRadius = SkDoubleToScalar(2.5f);
+    recorder.canvas()->sk_canvas()->drawRoundRect(
+        gfx::RectFToSkRect(border_rect_f), kCornerRadius, kCornerRadius, paint);
+    recorder.canvas()->Restore();
+  } else {
+    views::Painter::PaintPainterAt(recorder.canvas(), border_painter_.get(),

[tdanderson, 2015/09/28 15:12:14]

perhaps include a comment above |border_painter_| in the .h to state that it is
not used in MD mode.

[jonross, 2015/09/28 15:21:48]

Done.

+                                   border_rect);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////