Adjusts content bubble animation with respect to icon size

chrome/browser/ui/views/location_bar/content_setting_image_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/content_setting_image_view.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
 #include "chrome/browser/ui/content_settings/content_setting_image_model.h"
@@ skipping 38 matching lines @@
       pause_animation_state_(0.0),
       bubble_view_(nullptr),
       suppress_mouse_released_action_(false),
       ink_drop_delegate_(new views::ButtonInkDropDelegate(this, this)) {
   if (!ui::MaterialDesignController::IsModeMaterial()) {
     static const int kBackgroundImages[] =
         IMAGE_GRID(IDR_OMNIBOX_CONTENT_SETTING_BUBBLE);
     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);
   label()->SetElideBehavior(gfx::NO_ELIDE);
   label()->SetVisible(false);
 
   slide_animator_.SetSlideDuration(kAnimationDurationMS);
   slide_animator_.SetTweenType(gfx::Tween::LINEAR);
 }
 
@@ skipping 36 matching lines @@
 SkColor ContentSettingImageView::GetTextColor() const {
   return GetNativeTheme()->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldDefaultColor);
 }
 
 SkColor ContentSettingImageView::GetBorderColor() const {
   return gfx::kGoogleYellow700;
 }
 
 bool ContentSettingImageView::ShouldShowBackground() const {
-  return slide_animator_.is_animating() || pause_animation_;
+  return label()->visible() &&
+         (slide_animator_.is_animating() || pause_animation_);
 }
 
 double ContentSettingImageView::WidthMultiplier() const {
   double state = pause_animation_ ? pause_animation_state_
                                   : slide_animator_.GetCurrentValue();
   // The fraction of the animation we'll spend animating the string into view,
   // which is also the fraction we'll spend animating it closed; total
   // animation (slide out, show, then slide in) is 1.0.

[Peter Kasting, 2016/03/07 20:24:27]

We probably need more comments here, since now we won't spend the same amount of
time animating closed as open, even though we calculate the curves as if we
will.

[varkha, 2016/03/08 18:01:03]

Timings should be matching the original comment now.

   const double kOpenFraction =
       static_cast<double>(kOpenTimeMS) / kAnimationDurationMS;
   double size_fraction = 1.0;
   if (state < kOpenFraction)
     size_fraction = state / kOpenFraction;
   if (state > (1.0 - kOpenFraction))
     size_fraction = (1.0 - state) / kOpenFraction;
   return size_fraction;
 }
 
+void ContentSettingImageView::StopAnimation() {
+  slide_animator_.Stop();

[Peter Kasting, 2016/03/07 20:24:27]

Stop() is redundant with Reset().  You should only need the latter.

[varkha, 2016/03/08 18:01:03]

Done. also this method is no longer necessary.

+  slide_animator_.Reset();
+}
+
+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/07 20:24:27]

Given the definition of SchedulePaint(), it seems like calling it on parent_
should schedule a paint for the entire parent rect, which should eliminate the
need to call SchedulePaint() here on the child.  Either this should not be
added, or I'd like to understand technically why what I just said isn't true.

I wonder if the issue is that parent_->Layout() sees that the parent's size
hasn't changed and just quickly no-ops or something, without sizing us to our
new preferred size.

[varkha, 2016/03/08 18:01:03]

Done.

[varkha, 2016/03/08 21:56:03]

Will remove the commented line obviously before commit.

   }
 }
 
 void ContentSettingImageView::AnimationProgressed(
     const gfx::Animation* animation) {
   if (!pause_animation_) {
     parent_->Layout();
     parent_->SchedulePaint();
   }
 }
@@ skipping 87 matching lines @@
   // |widget| is a bubble that has just got shown / hidden.
   if (!visible && !label()->visible())
     ink_drop_delegate_->OnAction(views::InkDropState::DEACTIVATED);
 }
 
 void ContentSettingImageView::OnClick() {
   if (slide_animator_.is_animating()) {
     if (!pause_animation_) {
       pause_animation_ = true;
       pause_animation_state_ = slide_animator_.GetCurrentValue();
+
+      // Snap animation to target if a user clicks before the image is exposed.

[Peter Kasting, 2016/03/07 20:24:27]

Nit: a -> the

[varkha, 2016/03/08 18:01:03]

Done.

+      const double kOpenFraction =
+          static_cast<double>(kOpenTimeMS) / kAnimationDurationMS;
+      if (pause_animation_state_ < kOpenFraction &&

[Peter Kasting, 2016/03/07 20:24:27]

Is this first portion of the conditional needed?  Seems like if at the end of
the animation we animate just down to our target size, this check is redundant.

[varkha, 2016/03/08 18:01:03]

Done.

+          width() < image()->GetPreferredSize().width() +
+                        GetBubbleOuterPadding(true) +
+                        GetBubbleOuterPadding(false)) {
+        pause_animation_ = false;
+        StopAnimation();
+      }
     }
     slide_animator_.Reset();
   }
 
   content::WebContents* web_contents = parent_->GetWebContents();
   if (web_contents && !bubble_view_) {
     bubble_view_ = new ContentSettingBubbleContents(
                 content_setting_image_model_->CreateBubbleModel(
                     parent_->delegate()->GetContentSettingBubbleModelDelegate(),
                     web_contents, parent_->profile()),
                 web_contents, this, views::BubbleBorder::TOP_RIGHT);
     views::Widget* bubble_widget =
         parent_->delegate()->CreateViewsBubble(bubble_view_);
     bubble_widget->AddObserver(this);
     bubble_widget->Show();
   }
 }
 
 void ContentSettingImageView::UpdateImage() {
   SetImage(content_setting_image_model_->GetIcon(GetTextColor()).AsImageSkia());
   image()->SetTooltipText(content_setting_image_model_->get_tooltip());
 }
 
 bool ContentSettingImageView::IsBubbleShowing() const {
   return bubble_view_ != nullptr;
 }