Adjusts content bubble animation with respect to icon size

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"
@@ skipping 87 matching lines @@
 }
 
 bool IconLabelBubbleView::ShouldShowBackground() const {
   return should_show_background_;
 }
 
 double IconLabelBubbleView::WidthMultiplier() const {
   return 1.0;
 }
 
+bool IconLabelBubbleView::IsShrinking() const {
+  return false;
+}
+
 int IconLabelBubbleView::GetImageAndPaddingWidth() const {
   const int image_width = image_->GetPreferredSize().width();
   return image_width
              ? image_width + GetLayoutConstant(ICON_LABEL_VIEW_INTERNAL_PADDING)
              : 0;
 }
 
 gfx::Size IconLabelBubbleView::GetPreferredSize() const {
   // Height will be ignored by the LocationBarView.
   return GetSizeForLabelWidth(label_->GetPreferredSize().width());
 }
 
 void IconLabelBubbleView::Layout() {
   // In MD mode, both extension icons and Chrome-provided icons are 16px,
   // so it's not necessary to handle them differently. TODO(estade): clean
   // this up when MD is on by default.
   bool icon_has_enough_padding =
       !is_extension_icon_ || ui::MaterialDesignController::IsModeMaterial();
-  const int image_width = image_->GetPreferredSize().width();
-  image_->SetBounds(std::min((width() - image_width) / 2,
-                             GetBubbleOuterPadding(icon_has_enough_padding)),
-                    0, image_->GetPreferredSize().width(), height());
+  const int image_preferred_width = image_->GetPreferredSize().width();
+  // image is positioned at a constant offset when a label is visible or when
+  // there is enough room to fit the whole image. This is done so that the image
+  // slides in as a part of the sliding surface and doesn't shift relative to
+  // the background.
+  const int image_x = (label_->visible() || width() > image_preferred_width)
+                          ? GetBubbleOuterPadding(icon_has_enough_padding)

[Peter Kasting, 2016/03/08 22:07:06]

Can we just always do this?  Is it ever actually necessary to center the image
instead?  Maybe for extension icons pre-MD or something?

[varkha, 2016/03/10 22:25:56]

Done. Still need to make sure the image is right aligned at the end of the
animation. We don't want it to overshoot and then snap back.

[Peter Kasting, 2016/03/11 01:14:42]

How can we overshoot given the current definition of GetSizeForLabelWidth()? 
Won't that always reserve sufficient space?

[varkha, 2016/03/11 03:08:40]

I meant this as an explanation of what I have tried to do in a previous patch
set, not as a problem. A bigger trouble of course is that I have allowed this to
happen during the growth stage as well.

+                          : (width() - image_preferred_width) / 2;
+  const int image_padding =
+      label_->visible() ? GetBubbleOuterPadding(false) : 0;
+  const int image_width =
+      std::min(width() - image_x - image_padding, image_preferred_width);
+  image_->SetBounds(image_x, 0, image_width, height());
 
-  int pre_label_width = GetBubbleOuterPadding(true) + GetImageAndPaddingWidth();
-  label_->SetBounds(pre_label_width, 0,
-                    width() - pre_label_width - GetBubbleOuterPadding(false),
-                    height());
+  int label_x = GetBubbleOuterPadding(true) + GetImageAndPaddingWidth();
+  const int label_width =
+      std::max(0, width() - label_x - GetBubbleOuterPadding(false));
+
+  // Hiding the label when it no longer fits and when the leading edge of the
+  // image plus padding crosses the right edge of the bubble allows the final
+  // frames of the animation to smoothly transition from showing a bubble to
+  // showing just the image.
+  if (IsShrinking() && label_->visible() && label_width == 0 &&
+      (image_x + image_padding + image_preferred_width >= bounds().width())) {

[Peter Kasting, 2016/03/08 22:07:06]

Why is the last conditional clause here necessary?

[varkha, 2016/03/10 22:25:56]

I wanted to hide the boundary only when the image is roughly centered in the
bubble - having the correct padding on the left and right. If I omit this last
condition the boundary will disappear earlier - when the text no longer has room
left which is the point where there is icon-to-text-padding +
text-to-right-boundary-padding to the right of the icon.

[Peter Kasting, 2016/03/11 01:14:42]

That sounds like the (label_width == 0) check is unnecessary, then.  Frankly,
the label_->visible() check is too, but I can see why it might seem confusing to
remove it.  But I'd at least remove the first, along with the "it no longer fits
and" bit of the comment above.

[varkha, 2016/03/11 03:08:40]

Done.

+    label_->SetVisible(false);
+  }
+
+  label_->SetBounds(label_x, 0, label_width, height());
 }
 
 void IconLabelBubbleView::OnNativeThemeChanged(
     const ui::NativeTheme* native_theme) {
   label_->SetEnabledColor(GetTextColor());
 
   if (!ui::MaterialDesignController::IsModeMaterial())
     return;
 
   bool inverted = color_utils::IsDark(GetParentBackgroundColor());
@@ skipping 27 matching lines @@
 
 SkColor IconLabelBubbleView::GetParentBackgroundColor() const {
   return ui::MaterialDesignController::IsModeMaterial()
              ? GetNativeTheme()->GetSystemColor(
                    ui::NativeTheme::kColorId_TextfieldDefaultBackground)
              : parent_background_color_;
 }
 
 gfx::Size IconLabelBubbleView::GetSizeForLabelWidth(int width) const {
   gfx::Size size(image_->GetPreferredSize());
-  if (ShouldShowBackground()) {
-    const int non_label_width = GetBubbleOuterPadding(true) +
-                                GetImageAndPaddingWidth() +
-                                GetBubbleOuterPadding(false);
-    size = gfx::Size(WidthMultiplier() * (width + non_label_width), 0);
-    if (!ui::MaterialDesignController::IsModeMaterial())
-      size.SetToMax(background_painter_->GetMinimumSize());
+  bool shrinking = IsShrinking();
+  if (ShouldShowBackground() || shrinking) {

[Peter Kasting, 2016/03/08 22:07:06]

Add comment on why it's necessary to check for |shrinking| here (basically what
you said to me when I asked before).

[varkha, 2016/03/10 22:25:57]

Done.

+    const int leading_padding = GetBubbleOuterPadding(true);
+    const int padding_rest =

[Peter Kasting, 2016/03/08 22:07:06]

Nit: |remaining_padding| seems better ("rest" sounds like "padding at rest")

[varkha, 2016/03/10 22:25:57]

Done.

+        GetBubbleOuterPadding(false) +
+        GetLayoutConstant(ICON_LABEL_VIEW_INTERNAL_PADDING);
+    const int image_width = size.width();
+    const double multiplier = WidthMultiplier();
+    // |multiplier| grows from zero to one, stays equal to one and then shrinks
+    // to zero again. This view width should grow from zero to its maximum
+    // width, then stay at its maximum width and then shrink to such width that
+    // has enough room for the image and its leading padding.
+    const int total_width = shrinking
+        ? multiplier * (width + padding_rest) + leading_padding + image_width
+        : multiplier * (width + padding_rest + leading_padding + image_width);

[Peter Kasting, 2016/03/08 22:07:06]

This means the view will shrink more slowly (in terms of linear movement speed)
than it grows.  I don't think that's as good as the apparent behavior we had
before, where the velocity was constant and the time was different.

I think you can achieve this by computing the total width like the second arm
here in all cases, then clamping if |shrinking| is true.

[varkha, 2016/03/10 22:25:57]

Sure. That makes the animation continue near the end for another few
milliseconds without making any apparent change on screen which is why I didn't
do it this way but this doesn't seem to cause trouble.

+    size.set_width(total_width);
   }
+  return size;
+}
 
-  return size;
+int IconLabelBubbleView::GetBubbleOuterPadding(bool leading) const {
+  if (ui::MaterialDesignController::IsModeMaterial())
+    return GetBubbleOuterPaddingMd(leading);
+
+  return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) -
+         GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
+         (leading ? 0 : GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING));
 }
 
 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::GetBubbleOuterPadding(bool leading) const {
-  if (ui::MaterialDesignController::IsModeMaterial())
-    return GetBubbleOuterPaddingMd(leading);
-
-  return GetLayoutConstant(LOCATION_BAR_HORIZONTAL_PADDING) -
-         GetLayoutConstant(LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING) +
-         (leading ? 0 : GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING));
-}
-
 int IconLabelBubbleView::GetBubbleOuterPaddingMd(bool leading) const {
   // When the image is empty, leading and trailing padding are equal.
   if (image_->GetPreferredSize().IsEmpty() || !leading)
     return GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING);
 
   // Leading padding is 2dp.
   return 2;
 }
 
 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());
   }
   if (background())
     background()->Paint(canvas, this);
 }