Programmatically render the omnibox border

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/i18n/rtl.h"
@@ skipping 169 matching lines @@
 void LocationBarView::Init() {
   // We need to be in a Widget, otherwise GetNativeTheme() may change and we're
   // not prepared for that.
   DCHECK(GetWidget());
 
   if (is_popup_mode_) {
     const int kOmniboxPopupBorderImages[] =
         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(
         *rb.GetImageSkiaNamed(IDR_OMNIBOX_BORDER), omnibox_border_insets));
   }
 
   location_icon_view_ = new LocationIconView(this);
   location_icon_view_->set_drag_controller(this);
   AddChildView(location_icon_view_);
 
@@ skipping 320 matching lines @@
 bool LocationBarView::HasFocus() const {
   return omnibox_view_->model()->has_focus();
 }
 
 void LocationBarView::GetAccessibleState(ui::AXViewState* state) {
   state->role = ui::AX_ROLE_GROUP;
 }
 
 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.

 
   min_size.set_height(min_size.height() * size_animation_.GetCurrentValue());
 
   const int padding = GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
 
   // Compute width of omnibox-leading content.
   const int horizontal_edge_thickness = GetHorizontalEdgeThickness();
   int leading_width = horizontal_edge_thickness;
   if (ShouldShowKeywordBubble()) {
     // The selected keyword view can collapse completely.
@@ skipping 717 matching lines @@
   // OnPaint().
   // Note: |Canvas::DrawFocusRect| paints a dashed rect with gray color.
   if (show_focus_rect_ && HasFocus())
     recorder.canvas()->DrawFocusRect(omnibox_view_->bounds());
 
   // Maximized popup windows don't draw the horizontal edges.  We implement this
   // by simply expanding the paint area outside the view by the edge thickness.
   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);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, private views::ButtonListener implementation:
 
 void LocationBarView::ButtonPressed(views::Button* sender,
                                     const ui::Event& event) {
   DCHECK_EQ(mic_search_view_, sender);
   command_updater()->ExecuteCommand(IDC_TOGGLE_SPEECH_INPUT);
 }
@@ skipping 93 matching lines @@
 
 void LocationBarView::ModelChanged(const SearchModel::State& old_state,
                                    const SearchModel::State& new_state) {
   const bool visible = !GetToolbarModel()->input_in_progress() &&
       new_state.voice_search_supported;
   if (mic_search_view_->visible() != visible) {
     mic_search_view_->SetVisible(visible);
     Layout();
   }
 }