Adjust spacing in InternalPageInfoPopupView

chrome/browser/ui/views/website_settings/website_settings_popup_view.cc

Index: chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
diff --git a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
index a324f0a761a3d812f912f62f2b9daedaac416a0e..dfbe5f8c8294e16707f2f014af2285e4a090a359 100644
--- a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
+++ b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
@@ -173,6 +173,9 @@ class InternalPageInfoPopupView : public views::BubbleDialogDelegateView {
  private:
   friend class WebsiteSettingsPopupView;
 
+  // Used around icon and inside bubble border.
+  const int kSpacing = 12;

[Peter Kasting, 2017/01/06 02:27:11]

Nit: constexpr

[Evan Stade, 2017/01/06 20:20:20]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(InternalPageInfoPopupView);
 };
 
@@ -354,7 +357,6 @@ InternalPageInfoPopupView::InternalPageInfoPopupView(
   set_anchor_view_insets(gfx::Insets(
       GetLayoutConstant(LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET), 0));
 
-  const int kSpacing = 16;
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal, kSpacing,
                                         kSpacing, kSpacing));
   set_margins(gfx::Insets());
@@ -379,9 +381,9 @@ views::NonClientFrameView* InternalPageInfoPopupView::CreateNonClientFrameView(
     views::Widget* widget) {
   views::BubbleFrameView* frame = static_cast<views::BubbleFrameView*>(
       BubbleDialogDelegateView::CreateNonClientFrameView(widget));
-  // 16px padding + half of icon width comes out to 24px.
+  // Padding around icon + half of icon width.
   frame->bubble_border()->set_arrow_offset(
-      24 + frame->bubble_border()->GetBorderThickness());
+      kSpacing + 8 + frame->bubble_border()->GetBorderThickness());

[Peter Kasting, 2017/01/06 02:27:11]

Is it possible to compute this without hardcoding "8"?  For example, keep a
pointer to the icon view around and use it to get the width/2.  This would also
let you get the x coordinate from the icon so you wouldn't have to ensure the
spacing amount/calculation stays in sync here, which would mean you wouldn't
have to hoist the spacing constant.

(You could do this without an icon pointer by just assuming "first child is the
icon", but that seems fragile.)

This would obviate the need for the comment above.

[Evan Stade, 2017/01/06 20:20:20]

I don't think the contents will have been laid out by this point, so the spacing
constant would still need to be here. We could still get rid of the hardcoded 8
but it's not at zero cost, and it doesn't seem to me super likely that will ever
save us from a bug.

   return frame;
 }