Change IconLabelBubbleView layout to specify padding beside the separator.

chrome/browser/ui/views/location_bar/icon_label_bubble_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/icon_label_bubble_view.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/location_bar/background_with_1_px_border.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_util.h"
 #include "ui/gfx/scoped_canvas.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/painter.h"
 
 namespace {
 
-// Extra padding to place at the end of the chip when the label is showing.
-const int kExtraTrailingPadding = 7;
+// Amount of space on either side of the separator that appears after the label.
+constexpr int kSpaceBesideSeparator = 8;
+
+// We really only want 1 px for the separator, but there's no way to use that to
+// lay out, so the best we can do is to reserve a whole DIP.

[Evan Stade, 2016/06/15 23:12:58]

wouldn't the more correct thing to do be reserve 0px for scale factors >2?
Imagine 10x scale factor. You can choose between 1px too few or 9px too many.

[Peter Kasting, 2016/06/16 05:12:39]

Yeah, that would be better.  Changed, though this did require getting the scale
factor off the widget.

+constexpr int kSeparatorWidth = 1;
 
 SkColor CalculateImageColor(gfx::ImageSkia* image) {
   // We grab the color of the middle pixel of the image, which we treat as
   // the representative color of the entire image (reasonable, given the current
   // appearance of these assets).
   const SkBitmap& bitmap(image->GetRepresentation(1.0f).sk_bitmap());
   SkAutoLockPixels pixel_lock(bitmap);
   return bitmap.getColor(bitmap.width() / 2, bitmap.height() / 2);
 }
 
@@ skipping 185 matching lines @@
 }
 
 gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int label_width) const {
   gfx::Size size(image_->GetPreferredSize());
   const bool shrinking = IsShrinking();
   // Animation continues for the last few pixels even after the label is not
   // visible in order to slide the icon into its final position. Therefore it
   // is necessary to animate |total_width| even when the background is hidden
   // as long as the animation is still shrinking.
   if (ShouldShowBackground() || shrinking) {
-    const int extra_trailing_padding =
-        ui::MaterialDesignController::IsModeMaterial() ? kExtraTrailingPadding
-                                                       : 0;
     // |multiplier| grows from zero to one, stays equal to one and then shrinks
     // to zero again. The view width should correspondingly grow from zero to
     // fully showing both label and icon, stay there, then shrink to just large
     // enough to show the icon. We don't want to shrink all the way back to
     // zero, since this would mean the view would completely disappear and then
     // pop back to an icon after the animation finishes.
-    const int max_width = MinimumWidthForImageWithBackgroundShown() +
-                          GetInternalSpacing() + label_width +
-                          extra_trailing_padding;
+    const int max_width = GetImageTrailingEdge() + GetInternalSpacing() +
+                          label_width + GetPostLabelWidth();
     const int current_width = WidthMultiplier() * max_width;
     size.set_width(shrinking ? std::max(current_width, size.width())
                              : current_width);
   }
   return size;
 }
 
 int IconLabelBubbleView::MinimumWidthForImageWithBackgroundShown() const {
-  return GetOuterPadding(true) + image_->GetPreferredSize().width() +
-         GetOuterPadding(false);
+  return GetImageTrailingEdge() + GetOuterPadding(false);
 }
 
 void IconLabelBubbleView::SetLabelBackgroundColor(
     SkColor chip_background_color) {
   // The background images are painted atop |parent_background_color_|.
   // Alpha-blend |chip_background_color| with |parent_background_color_| to
   // determine the actual color the label text will sit atop.
   // Tricky bit: We alpha blend an opaque version of |chip_background_color|
   // against |parent_background_color_| using the original image grid color's
   // alpha. This is because AlphaBlend(a, b, 255) always returns |a| unchanged
   // even if |a| is a color with non-255 alpha.
   label_->SetBackgroundColor(color_utils::AlphaBlend(
       SkColorSetA(chip_background_color, 255), GetParentBackgroundColor(),
       SkColorGetA(chip_background_color)));
 }
 
 int IconLabelBubbleView::GetOuterPadding(bool leading) const {
   if (ui::MaterialDesignController::IsModeMaterial())
     return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
 
   return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) -
          GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
          (leading ? 0 : kTrailingPaddingPreMD);
 }
 
+int IconLabelBubbleView::GetImageTrailingEdge() const {
+  return GetOuterPadding(true) + image_->GetPreferredSize().width();
+}
+
 int IconLabelBubbleView::GetInternalSpacing() const {
   return image_->GetPreferredSize().IsEmpty()
              ? 0
              : GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
 }
 
+int IconLabelBubbleView::GetPostLabelWidth() const {
+  if (!ui::MaterialDesignController::IsModeMaterial())
+    return GetOuterPadding(false);
+
+  // The location bar will add LOCATION_BAR_HORIZONTAL_PADDING after us.
+  return (kSpaceBesideSeparator * 2) + kSeparatorWidth -
+         GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
+}
+
 const char* IconLabelBubbleView::GetClassName() const {
   return "IconLabelBubbleView";
 }
 
 void IconLabelBubbleView::OnPaint(gfx::Canvas* canvas) {
   if (!ShouldShowBackground())
     return;
   if (background_painter_) {
     views::Painter::PaintPainterAt(canvas, background_painter_.get(),
                                    GetContentsBounds());
   }
 
   // In MD, draw a separator and not a background.
   if (ui::MaterialDesignController::IsModeMaterial()) {
     const SkColor plain_text_color = GetNativeTheme()->GetSystemColor(
         ui::NativeTheme::kColorId_TextfieldDefaultColor);
     const SkColor separator_color = SkColorSetA(
         plain_text_color, color_utils::IsDark(plain_text_color) ? 0x59 : 0xCC);
 
-    // Amount of padding that is built into the whatever view comes after this
-    // one in the location bar (e.g. omnibox textfield).
-    int post_chip_spacing =
-        GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING);
-    // Amount of space to leave after the separator (dp).
-    const int kPostSeparatorSpacing =
         (GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) +
-         kExtraTrailingPadding + post_chip_spacing) /
-            2 -
-        post_chip_spacing;
-    // Height of the separator (dp).
+    gfx::Rect bounds(GetLocalBounds());
     const int kSeparatorHeight = 16;
-    gfx::Rect bounds(GetLocalBounds());
-    bounds.Inset(kPostSeparatorSpacing,
+    bounds.Inset(GetPostLabelWidth() - kSpaceBesideSeparator - kSeparatorWidth,
                  (bounds.height() - kSeparatorHeight) / 2);
 
-    // 1px at all scale factors.
+    // Draw the 1 px separator.
     gfx::ScopedCanvas scoped_canvas(canvas);
     const float scale = canvas->UndoDeviceScaleFactor();
+    // Keep the separator aligned on a pixel center, but center it within the
+    // DIP if the scale factor is large enough.
     const gfx::RectF pixel_aligned_bounds =
-        gfx::ScaleRect(gfx::RectF(bounds), scale) - gfx::Vector2dF(0.5f, 0);
+        gfx::ScaleRect(gfx::RectF(bounds), scale) -
+        gfx::Vector2dF(std::floor((scale - 1) / 2) + 0.5f, 0);
     canvas->DrawLine(pixel_aligned_bounds.top_right(),
                      pixel_aligned_bounds.bottom_right(), separator_color);
   }
 }