Adjusts content bubble animation with respect to icon size

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

Index: chrome/browser/ui/views/location_bar/content_setting_image_view.cc
diff --git a/chrome/browser/ui/views/location_bar/content_setting_image_view.cc b/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
index e88ab5c3f5717dfaad2fb524f3ac242e1e3be424..5ac7b0a7e0ec404a99d4276ff17a6fa8fa6c5818 100644
--- a/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
+++ b/chrome/browser/ui/views/location_bar/content_setting_image_view.cc
@@ -29,7 +29,7 @@ const int kStayOpenTimeMS = 3200;
 
 
 // static
-const int ContentSettingImageView::kOpenTimeMS = 150;
+const int ContentSettingImageView::kOpenTimeMS = 15000;

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

You don't want to commit this :)

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

Wasn't planning on it!

 const int ContentSettingImageView::kAnimationDurationMS =
     (kOpenTimeMS * 2) + kStayOpenTimeMS;
 
@@ -56,7 +56,9 @@ ContentSettingImageView::ContentSettingImageView(
     SetBackgroundImageGrid(kBackgroundImages);
   }
 
-  image()->SetHorizontalAlignment(views::ImageView::LEADING);
+  image()->SetHorizontalAlignment(base::i18n::IsRTL()
+                                      ? views::ImageView::TRAILING
+                                      : views::ImageView::LEADING);
   image()->set_interactive(true);
   image()->EnableCanvasFlippingForRTLUI(true);
   image()->SetAccessibilityFocusable(true);
@@ -113,7 +115,8 @@ SkColor ContentSettingImageView::GetBorderColor() const {
 }
 
 bool ContentSettingImageView::ShouldShowBackground() const {
-  return slide_animator_.is_animating() || pause_animation_;
+  return label()->visible() &&
+         (slide_animator_.is_animating() || pause_animation_);
 }
 
 double ContentSettingImageView::WidthMultiplier() const {
@@ -132,12 +135,20 @@ double ContentSettingImageView::WidthMultiplier() const {
   return size_fraction;
 }
 
+bool ContentSettingImageView::IsShrinking() const {
+  const double kOpenFraction =
+      static_cast<double>(kOpenTimeMS) / kAnimationDurationMS;
+  return (!pause_animation_ && slide_animator_.is_animating() &&
+          slide_animator_.GetCurrentValue() > (1.0 - kOpenFraction));
+}
+
 void ContentSettingImageView::AnimationEnded(const gfx::Animation* animation) {
   slide_animator_.Reset();
   if (!pause_animation_) {
     label()->SetVisible(false);
     parent_->Layout();
     parent_->SchedulePaint();
+//    SchedulePaint();

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

Remove or else uncomment and add notes as to why

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

Done.

   }
 }
 
@@ -245,6 +256,14 @@ void ContentSettingImageView::OnClick() {
     if (!pause_animation_) {
       pause_animation_ = true;
       pause_animation_state_ = slide_animator_.GetCurrentValue();
+
+      // Snap animation to target if the click happens before the image is
+      // exposed.
+      if (width() < image()->GetPreferredSize().width() +
+                        GetBubbleOuterPadding(true) +
+                        GetBubbleOuterPadding(false)) {
+        pause_animation_ = false;
+      }

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

Nit: Seems like this would make more sense hoisted, so it's clear you don't need
to set |pause_animation_state_|.  Here's a more comprehensive comment, too.

  if (slide_animator_.is_animating()) {
    // If the user clicks while we're animating, the bubble arrow will be
    // pointing to the image, and if we allow the animation to keep running, the
    // image will move away from the arrow (or we'll have to move the bubble,
    // which is even worse).  So we want to stop the animation.  We have two
    // choices: jump to the final post-animation state (no label visible), or
    // pause the animation where we are and continue running after the bubble
    // closes.  The former looks more jerky, so we avoid it unless the animation
    // hasn't even fully exposed the image yet, in which case pausing with half
    // an image visible will look broken.
    const int final_width = image()->GetPreferredSize().width() +
        GetBubbleOuterPadding(true) + GetBubbleOuterPadding(false);
    if (!pause_animation_ && (width() > final_width)) {
      pause_animation_ = true;
      pause_animation_state_ = slide_animator_.GetCurrentValue();
    }
    slide_animator_.Reset();
  }

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

Done. Thanks for the wording. You also got the subtlety of width() > final_width
rather than >= correctly.

     }
     slide_animator_.Reset();
   }