Fix up views web intents location bar decoration.

chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc b/chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc
index 8b214e24084ee23279c1f960a4a3ef9e6ccae187..28d7d383558004716cb55d7bd5e8ca4f664e16b4 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_decoration_view.cc
@@ -12,7 +12,6 @@
 #include "third_party/skia/include/core/SkShader.h"
 #include "ui/base/animation/slide_animation.h"
 #include "ui/base/animation/tween.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/views/border.h"
@@ -22,9 +21,8 @@ namespace {
 // Animation parameters.
 const int kFrameRateHz = 60;
 // Margins for animated box (pixels).
-const int kTextMargin = 4;
+const int kTextMargin = 5;

[Peter Kasting, 2012/09/15 00:23:46]

Should the icon margin and text margins match?  Should these match how the
keyword hint view draws things?

[Greg Billock, 2012/09/15 04:58:20]

The text margin is between the text and other controls. I'm not sure why that'd
have to match icon-left, but I'm happy to do it. I'm mostly reacting to the
specific feedback from UI. Shall I ask Jeff about this?

[Peter Kasting, 2012/09/17 20:04:27]

Nah, don't worry about it.

 const int kIconLeftMargin = 4;
-
 }
 
 LocationBarDecorationView::LocationBarDecorationView(
@@ -46,6 +44,8 @@ LocationBarDecorationView::~LocationBarDecorationView() {
 
 gfx::Size LocationBarDecorationView::GetPreferredSize() {
   gfx::Size preferred_size(views::ImageView::GetPreferredSize());
+  if (!GetImageSize(&preferred_size))

[Peter Kasting, 2012/09/15 00:23:46]

I don't actually understand what this is doing.  It doesn't look like we ever
call SetImage() so how can GetImageSize() ever succeed?  More fundamentally, if
we aren't calling SetImage(), why does this class inherit from ImageView to
begin with?

[Greg Billock, 2012/09/15 04:58:20]

Me either. This is a failed attempt to get the vertical spacing right for the
background. Here's what I see: This method returns "21" for both the content
settings and the web intents controls. The view superclass is then returning 21
for height(), but OnPaintBackground is getting called with a height of 23 for
the WI control and 21 for the CS control.


On 2012/09/15 00:23:46, Peter Kasting wrote:

+    preferred_size.set_height(background_painter_.height());
   preferred_size.set_height(std::max(preferred_size.height(),
                                      background_painter_.height()));
   // When view is animated |visible_text_size_| > 0, it is 0 otherwise.
@@ -93,6 +93,10 @@ int LocationBarDecorationView::GetBuiltInHorizontalPadding() const {
   return GetBuiltInHorizontalPaddingImpl();
 }
 
+void LocationBarDecorationView::SetFont(const gfx::Font& font) {
+  font_ = font;
+}
+
 void LocationBarDecorationView::StartLabelAnimation(string16 animated_text,
                                                     int duration_ms) {
   if (!slide_animator_.get()) {
@@ -106,8 +110,7 @@ void LocationBarDecorationView::StartLabelAnimation(string16 animated_text,
     // Initialize animated string. It will be cleared when animation is
     // completed.
     animated_text_ = animated_text;
-    text_size_ = ui::ResourceBundle::GetSharedInstance().GetFont(
-        ui::ResourceBundle::MediumFont).GetStringWidth(animated_text);
+    text_size_ = font_.GetStringWidth(animated_text);
     text_size_ += 2 * kTextMargin + kIconLeftMargin;
     slide_animator_->Show();
   }
@@ -171,9 +174,9 @@ void LocationBarDecorationView::OnPaint(gfx::Canvas* canvas) {
     views::ImageView::OnPaint(canvas);
 
     // Paint text to the right of the icon.
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    canvas->DrawStringInt(animated_text_,
-        rb.GetFont(ui::ResourceBundle::MediumFont), SK_ColorBLACK,
+    LOG(INFO) << "ImageBnds=" << GetImageBounds().right() << " " << GetImageBounds().width();

[Peter Kasting, 2012/09/15 00:23:46]

Don't check these in.  (Multiple places)

[Greg Billock, 2012/09/15 04:58:20]

Yeah, this is leftover trying to diagnose this height thing. Any advice would be
appreciated!

On 2012/09/15 00:23:46, Peter Kasting wrote:

+    LOG(INFO) << "height=" << height();
+    canvas->DrawStringInt(animated_text_, font_, SK_ColorDKGRAY,

[Peter Kasting, 2012/09/15 00:23:46]

This should not be hardcoding a text color.  We should be using the location
bar's text color, optionally passing it to color_utils::GetReadableColor() with
the relevant background color here (which, if the images are partly transparent,
may itself have to be calculated as an alpha-blend of a particular value with
the location bar's background color).  See how location_bar_view.cc calls
GetColor().

[Greg Billock, 2012/09/15 04:58:20]

Got it. I'll change this to take a color and call GetColor(ToolbarModel::NONE,
TEXT) in the location bar view.

On 2012/09/15 00:23:46, Peter Kasting wrote:

         GetImageBounds().right() + kTextMargin, 0,
         width() - GetImageBounds().width(), height(),
         gfx::Canvas::TEXT_ALIGN_LEFT | gfx::Canvas::TEXT_VALIGN_MIDDLE);
@@ -185,6 +188,7 @@ void LocationBarDecorationView::OnPaint(gfx::Canvas* canvas) {
 void LocationBarDecorationView::OnPaintBackground(gfx::Canvas* canvas) {
   if (force_draw_text_ || (slide_animator_.get() &&
       (slide_animator_->is_animating() || pause_animation_))) {
+    LOG(INFO) << "Paint bg " << size().width() << " " << size().height();
     background_painter_.Paint(canvas, size());
   } else {
     views::ImageView::OnPaintBackground(canvas);