Anchor the infobar arrow to the LocationIconView correctly.

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 24ff38073a625cbc84960b94cd24fe596b0312ff..26a15f352a31cfae7273305a994c6469ed219eb1 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -445,13 +445,12 @@ void LocationBarView::SelectAll() {
 }
 
 gfx::Point LocationBarView::GetLocationBarAnchorPoint() const {
-  // The +1 in the next line creates a 1-px gap between icon and arrow tip.
-  const int icon_padding = GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING);

[Evan Stade, 2016/01/22 23:58:37]

I've no idea why this calculation made sense as this padding is the horizontal
spacing (I suspect it only worked by coincidence).

[Peter Kasting, 2016/01/23 00:13:41]

In pre-MD, the ICON_LABEL_VIEW_TRAILING_PADDING was calculated to match the
padding in the image, so that we'd end up with equal padding on all sides of
everything.  So reading this value gave you the height of the vertical padding
below the image.

In MD this is definitely not the case, but assuming the padding is exactly 2 px
(as the new code does by using -1 directly) is probably also unsafe.

So I think there are a couple options:
(1) Just remove the -1 and point the arrow right at the image bottom.  Assume
this won't look too bad in any mode (which may not be a good assumption).
(2) Add some sort of layout constant for the image's built-in bottom padding. 
Set this to 2 on pre-MD and <whatever> on MD.  Then use the same code that's
already here, but substitute in your constant name for
ICON_LABEL_VIEW_TRAILING_PADDING.

I like option (2) better.

[Evan Stade, 2016/01/23 00:37:26]

This CL is attempting to fix a regression and is destined for a merge. Since
this works for pre-MD, can your option (2) be a TODO?

Also, wouldn't we still put the constant here (as this patch does) since it's
only referenced here? The only improvement being we may need a different one for
MD and pre MD.

[Peter Kasting, 2016/01/23 01:25:28]

If you want to merge, then don't touch the calculation here at all.  It produces
the correct result for non-MD already, so there's no benefit to replacing it
with a hardcoded -1.
When we have constants that differ for MD and non-MD I've been putting them in
layout_constants.cc even if they're only used in one location.

And given that the obvious constant would be "padding inside the image" (not
"net offset computed by adding 1 to the padding inside the image"), we'd still
have code that looks exactly like the existing code here, it would just have a
different constant name.

-  gfx::Point icon_bottom(
-      0, location_icon_view_->GetImageBounds().bottom() - icon_padding + 1);
-  gfx::Point icon_center(location_icon_view_->GetImageBounds().CenterPoint());
-  gfx::Point point(icon_center.x(), icon_bottom.y());
-  ConvertPointToTarget(location_icon_view_, this, &point);
+  const views::ImageView* image = location_icon_view_->GetImageView();
+  // The -1 in the next line creates a 1-px gap between icon and arrow tip.
+  int icon_bottom = image->GetImageBounds().bottom() - 1;
+  gfx::Point icon_center(image->GetImageBounds().CenterPoint());
+  gfx::Point point(icon_center.x(), icon_bottom);
+  ConvertPointToTarget(image, this, &point);
   return point;
 }
 
@@ -1353,7 +1352,7 @@ void LocationBarView::AnimationEnded(const gfx::Animation* animation) {
 
 void LocationBarView::OnChanged() {
   RefreshLocationIcon();
-  location_icon_view_->ShowTooltip(!GetOmniboxView()->IsEditingOrEmpty());
+  location_icon_view_->set_show_tooltip(!GetOmniboxView()->IsEditingOrEmpty());
   Layout();
   SchedulePaint();
 }