ContentSettingImageView cleanup, phase 1.

* Parent classes in alphabetical order.
* Overrides as private as possible, and in same order as parent classes.  We
  also keep the .cc file order matching the .h file.
* Remove AlwaysDrawText() entirely as it was never called.  This also means we
  can remove |force_draw_text_|.
* Inline SetImage(), SetTooltipText(), StartLabelAnimation(), PauseAnimation(),
  and UnpauseAnimation() into their lone callers.  Inline AnimationOnClick()
  into OnClick() since the two were always called together.
* Remove the scoped_ptr<> on |slide_animator_| and initialize it in the
  constructor, instead of the first time we want to animate.
* Eliminate |font_color_| since it was never used.  (We will eventually use the
  color the parent supplies us, so the constructor arg sticks around.)
* Separate out three other members that will be going away in the future.  Put
  the other ones into some sort of hopefully-logical order.
* DISALLOW_IMPLICIT_CONSTRUCTORS -> DISALLOW_COPY_AND_ASSIGN.

The changes in the .cc file are basically just deleting and reordering code,
plus "->" to "." conversions for |slide_animator_|.  I avoided doing any other
cleanup/rewriting, to keep this change "basically mechanical".

BUG=none
TEST=none
R=sky@chromium.org

Review URL: https://codereview.chromium.org/15732018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202917 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -53,25 +53,27 @@ ContentSettingImageView::ContentSettingImageView(
     int font_y_offset,
     SkColor font_color)
     : parent_(parent),
-      text_label_(NULL),
+      content_setting_image_model_(
+          ContentSettingImageModel::CreateContentSettingImageModel(
+              content_type)),
+      background_painter_(background_images),
       icon_(new views::ImageView),
-      font_(font),
-      font_color_(font_color),
+      text_label_(NULL),
+      slide_animator_(this),
       pause_animation_(false),
       pause_animation_state_(0.0),
+      bubble_widget_(NULL),
+      font_(font),
       text_size_(0),
-      visible_text_size_(0),
-      force_draw_text_(false),
-      background_painter_(background_images),
-      content_setting_image_model_(
-          ContentSettingImageModel::CreateContentSettingImageModel(
-              content_type)),
-      bubble_widget_(NULL) {
+      visible_text_size_(0) {
   SetLayoutManager(new views::BoxLayout(
       views::BoxLayout::kHorizontal, 0, 0, 0));
   icon_->SetHorizontalAlignment(views::ImageView::LEADING);
   AddChildView(icon_);
   TouchableLocationBarView::Init(this);
+
+  slide_animator_.SetSlideDuration(kMoveTimeMs);
+  slide_animator_.SetTweenType(ui::Tween::LINEAR);
 }
 
 ContentSettingImageView::~ContentSettingImageView() {
@@ -81,6 +83,10 @@ ContentSettingImageView::~ContentSettingImageView() {
   }
 }
 
+int ContentSettingImageView::GetBuiltInHorizontalPadding() const {
+  return GetBuiltInHorizontalPaddingImpl();
+}
+
 void ContentSettingImageView::Update(WebContents* web_contents) {
   if (web_contents) {
     content_setting_image_model_->UpdateFromWebContents(web_contents);
@@ -89,9 +95,10 @@ void ContentSettingImageView::Update(WebContents* web_contents) {
     SetVisible(false);
     return;
   }
-  SetImage(ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+  icon_->SetImage(ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
       content_setting_image_model_->get_icon()));
-  SetTooltipText(UTF8ToUTF16(content_setting_image_model_->get_tooltip()));
+  icon_->SetTooltipText(
+      UTF8ToUTF16(content_setting_image_model_->get_tooltip()));
   SetVisible(true);
 
   TabSpecificContentSettings* content_settings = NULL;
@@ -115,40 +122,27 @@ void ContentSettingImageView::Update(WebContents* web_contents) {
   if (!animated_string_id)
     return;
 
-  StartLabelAnimation(l10n_util::GetStringUTF16(animated_string_id),
-                      kMoveTimeMs);
-}
-
-void ContentSettingImageView::SetImage(const gfx::ImageSkia* image_skia) {
-  icon_->SetImage(image_skia);
-}
-
-void ContentSettingImageView::SetTooltipText(const string16& tooltip) {
-  icon_->SetTooltipText(tooltip);
-}
-
-gfx::Size ContentSettingImageView::GetPreferredSize() {
-  // Height will be ignored by the LocationBarView.
-  gfx::Size preferred_size(views::View::GetPreferredSize());
-  int non_label_width = preferred_size.width() -
-      (text_label_ ? text_label_->GetPreferredSize().width() : 0);
-  // When view is animated |visible_text_size_| > 0, it is 0 otherwise.
-  preferred_size.set_width(non_label_width + visible_text_size_);
-  return preferred_size;
-}
-
-void ContentSettingImageView::OnGestureEvent(ui::GestureEvent* event) {
-  if (event->type() == ui::ET_GESTURE_TAP) {
-    AnimationOnClick();
-    OnClick(parent_);
-    event->SetHandled();
-  } else if (event->type() == ui::ET_GESTURE_TAP_DOWN) {
-    event->SetHandled();
+  // Do not start animation if already in progress.
+  if (!slide_animator_.is_animating()) {
+    // Initialize animated string. It will be cleared when animation is
+    // completed.
+    if (!text_label_) {
+      text_label_ = new views::Label;
+      text_label_->SetElideBehavior(views::Label::NO_ELIDE);
+      text_label_->SetFont(font_);
+      SetLayoutManager(new views::BoxLayout(
+          views::BoxLayout::kHorizontal, kHorizMargin, 0, kIconLabelSpacing));
+      AddChildView(text_label_);
+    }
+    text_label_->SetText(l10n_util::GetStringUTF16(animated_string_id));
+    text_size_ = font_.GetStringWidth(text_label_->text());
+    text_size_ += kHorizMargin;
+    slide_animator_.Show();
   }
 }
 
 void ContentSettingImageView::AnimationEnded(const ui::Animation* animation) {
-  if (!pause_animation_ && !force_draw_text_) {
+  if (!pause_animation_) {
     SetLayoutManager(new views::BoxLayout(
         views::BoxLayout::kHorizontal, 0, 0, 0));
     RemoveChildView(text_label_);  // will also delete the view.
@@ -156,7 +150,7 @@ void ContentSettingImageView::AnimationEnded(const ui::Animation* animation) {
     parent_->Layout();
     parent_->SchedulePaint();
   }
-  slide_animator_->Reset();
+  slide_animator_.Reset();
 }
 
 void ContentSettingImageView::AnimationProgressed(
@@ -164,7 +158,7 @@ void ContentSettingImageView::AnimationProgressed(
   if (pause_animation_)
     return;
 
-  visible_text_size_ = GetTextAnimationSize(slide_animator_->GetCurrentValue(),
+  visible_text_size_ = GetTextAnimationSize(slide_animator_.GetCurrentValue(),
                                             text_size_);
 
   parent_->Layout();
@@ -176,8 +170,41 @@ void ContentSettingImageView::AnimationCanceled(
   AnimationEnded(animation);
 }
 
-int ContentSettingImageView::GetBuiltInHorizontalPadding() const {
-  return GetBuiltInHorizontalPaddingImpl();
+gfx::Size ContentSettingImageView::GetPreferredSize() {
+  // Height will be ignored by the LocationBarView.
+  gfx::Size preferred_size(views::View::GetPreferredSize());
+  int non_label_width = preferred_size.width() -
+      (text_label_ ? text_label_->GetPreferredSize().width() : 0);
+  // When view is animated |visible_text_size_| > 0, it is 0 otherwise.
+  preferred_size.set_width(non_label_width + visible_text_size_);
+  return preferred_size;
+}
+
+bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) {
+  // We want to show the bubble on mouse release; that is the standard behavior
+  // for buttons.
+  return true;
+}
+
+void ContentSettingImageView::OnMouseReleased(const ui::MouseEvent& event) {
+  if (!HitTestPoint(event.location()))
+    return;
+
+  OnClick(parent_);
+}
+
+void ContentSettingImageView::OnGestureEvent(ui::GestureEvent* event) {
+  if (event->type() == ui::ET_GESTURE_TAP) {
+    OnClick(parent_);
+    event->SetHandled();
+  } else if (event->type() == ui::ET_GESTURE_TAP_DOWN) {
+    event->SetHandled();
+  }
+}
+
+void ContentSettingImageView::OnPaintBackground(gfx::Canvas* canvas) {
+  if (slide_animator_.is_animating() || pause_animation_)
+    background_painter_.Paint(canvas, size());
 }
 
 void ContentSettingImageView::OnWidgetDestroying(views::Widget* widget) {
@@ -186,10 +213,24 @@ void ContentSettingImageView::OnWidgetDestroying(views::Widget* widget) {
     bubble_widget_ = NULL;
   }
 
-  UnpauseAnimation();
+  if (!pause_animation_)
+    return;
+
+  slide_animator_.Reset(pause_animation_state_);
+  pause_animation_ = false;
+  slide_animator_.Show();
 }
 
 void ContentSettingImageView::OnClick(LocationBarView* parent) {
+  // Stop animation.
+  if (slide_animator_.is_animating()) {
+    if (!pause_animation_) {
+      pause_animation_ = true;
+      pause_animation_state_ = slide_animator_.GetCurrentValue();
+    }
+    slide_animator_.Reset();
+  }
+
   WebContents* web_contents = parent->GetWebContents();
   if (!web_contents)
     return;
@@ -211,33 +252,6 @@ void ContentSettingImageView::OnClick(LocationBarView* parent) {
   bubble->GetWidget()->Show();
 }
 
-void ContentSettingImageView::StartLabelAnimation(string16 animated_text,
-                                                  int duration_ms) {
-  if (!slide_animator_.get()) {
-    slide_animator_.reset(new ui::SlideAnimation(this));
-    slide_animator_->SetSlideDuration(duration_ms);
-    slide_animator_->SetTweenType(ui::Tween::LINEAR);
-  }
-
-  // Do not start animation if already in progress.
-  if (!slide_animator_->is_animating()) {
-    // Initialize animated string. It will be cleared when animation is
-    // completed.
-    if (!text_label_) {
-      text_label_ = new views::Label;
-      text_label_->SetElideBehavior(views::Label::NO_ELIDE);
-      text_label_->SetFont(font_);
-      SetLayoutManager(new views::BoxLayout(
-          views::BoxLayout::kHorizontal, kHorizMargin, 0, kIconLabelSpacing));
-      AddChildView(text_label_);
-    }
-    text_label_->SetText(animated_text);
-    text_size_ = font_.GetStringWidth(animated_text);
-    text_size_ += kHorizMargin;
-    slide_animator_->Show();
-  }
-}
-
 int ContentSettingImageView::GetTextAnimationSize(double state,
                                                   int text_size) {
   if (state >= 1.0) {
@@ -251,52 +265,3 @@ int ContentSettingImageView::GetTextAnimationSize(double state,
     return text_size;
   }
 }
-
-void ContentSettingImageView::PauseAnimation() {
-  if (pause_animation_)
-    return;
-
-  pause_animation_ = true;
-  pause_animation_state_ = slide_animator_->GetCurrentValue();
-}
-
-void ContentSettingImageView::UnpauseAnimation() {
-  if (!pause_animation_)
-    return;
-
-  slide_animator_->Reset(pause_animation_state_);
-  pause_animation_ = false;
-  slide_animator_->Show();
-}
-
-void ContentSettingImageView::AlwaysDrawText() {
-  force_draw_text_ = true;
-}
-
-bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) {
-  // We want to show the bubble on mouse release; that is the standard behavior
-  // for buttons.
-  return true;
-}
-
-void ContentSettingImageView::OnMouseReleased(const ui::MouseEvent& event) {
-  if (!HitTestPoint(event.location()))
-    return;
-
-  AnimationOnClick();
-  OnClick(parent_);
-}
-
-void ContentSettingImageView::OnPaintBackground(gfx::Canvas* canvas) {
-  if (force_draw_text_ || (slide_animator_.get() &&
-      (slide_animator_->is_animating() || pause_animation_)))
-    background_painter_.Paint(canvas, size());
-}
-
-void ContentSettingImageView::AnimationOnClick() {
-  // Stop animation.
-  if (slide_animator_.get() && slide_animator_->is_animating()) {
-    PauseAnimation();
-    slide_animator_->Reset();
-  }
-}

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

@@ -5,11 +5,11 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_CONTENT_SETTING_IMAGE_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_CONTENT_SETTING_IMAGE_VIEW_H_
 
-#include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/ui/views/location_bar/touchable_location_bar_view.h"
 #include "chrome/common/content_settings_types.h"
 #include "ui/base/animation/animation_delegate.h"
+#include "ui/base/animation/slide_animation.h"
 #include "ui/gfx/font.h"
 #include "ui/views/painter.h"
 #include "ui/views/view.h"
@@ -22,10 +22,6 @@ namespace content {
 class WebContents;
 }
 
-namespace ui {
-class SlideAnimation;
-}
-
 namespace views {
 class ImageView;
 class Label;
@@ -34,9 +30,9 @@ class Label;
 // The ContentSettingImageView displays an icon and optional text label for
 // various content settings affordances in the location bar (i.e. plugin
 // blocking, geolocation).
-class ContentSettingImageView : public views::View,
+class ContentSettingImageView : public TouchableLocationBarView,
                                 public ui::AnimationDelegate,
-                                public TouchableLocationBarView,
+                                public views::View,
                                 public views::WidgetObserver {
  public:
   // |background_images| is the array of images used to draw
@@ -49,72 +45,49 @@ class ContentSettingImageView : public views::View,
                           SkColor font_color);
   virtual ~ContentSettingImageView();
 
+  // TouchableLocationBarView:
+  virtual int GetBuiltInHorizontalPadding() const OVERRIDE;
+
   // Update the decoration from the shown WebContents.
   void Update(content::WebContents* web_contents);
 
-  void SetImage(const gfx::ImageSkia* image_skia);
-  void SetTooltipText(const string16& tooltip);
-
-  // views::View overrides:
-  virtual gfx::Size GetPreferredSize() OVERRIDE;
-
-  // ui::EventHandler overrides:
-  virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE;
-
-  // ui::AnimationDelegate overrides:
+ private:
+  // ui::AnimationDelegate:
   virtual void AnimationEnded(const ui::Animation* animation) OVERRIDE;
   virtual void AnimationProgressed(const ui::Animation* animation) OVERRIDE;
   virtual void AnimationCanceled(const ui::Animation* animation) OVERRIDE;
 
-  // TouchableLocationBarView.
-  virtual int GetBuiltInHorizontalPadding() const OVERRIDE;
+  // views::View:
+  virtual gfx::Size GetPreferredSize() OVERRIDE;
+  virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
+  virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
+  virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE;
+  virtual void OnPaintBackground(gfx::Canvas* canvas) OVERRIDE;
 
-  // views::WidgetObserver override:
+  // views::WidgetObserver:
   virtual void OnWidgetDestroying(views::Widget* widget) OVERRIDE;
 
- private:
   // Invoked when the user clicks on the control.
   void OnClick(LocationBarView* parent);
 
-  // Start animating the text label if it is not already happening.
-  void StartLabelAnimation(string16 animated_text, int duration_ms);
   int GetTextAnimationSize(double state, int text_size);
 
-  void PauseAnimation();
-  void UnpauseAnimation();
-
-  // Call to always draw the text by the (optional) icon. Used to draw the web
-  // intents button.
-  void AlwaysDrawText();
-
-  // views::View overrides:
-  virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE;
-  virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE;
-  virtual void OnPaintBackground(gfx::Canvas* canvas) OVERRIDE;
-
-  // Notify the possibly-running animation that it was clicked.
-  void AnimationOnClick();
-
-  // The owning LocationBarView. Weak pointer.
-  LocationBarView* parent_;
-
-  // Child views that comprise the bubble.
-  views::Label* text_label_;
+  LocationBarView* parent_;  // Weak, owns us.
+  scoped_ptr<ContentSettingImageModel> content_setting_image_model_;
+  views::HorizontalPainter background_painter_;
   views::ImageView* icon_;
-
-  scoped_ptr<ui::SlideAnimation> slide_animator_;
-  gfx::Font font_;
-  SkColor font_color_;
+  views::Label* text_label_;
+  ui::SlideAnimation slide_animator_;
   bool pause_animation_;
   double pause_animation_state_;
+  views::Widget* bubble_widget_;
+
+  // TODO(pkasting): Eliminate these.
+  gfx::Font font_;
   int text_size_;
   int visible_text_size_;
-  bool force_draw_text_;
-  views::HorizontalPainter background_painter_;
-  scoped_ptr<ContentSettingImageModel> content_setting_image_model_;
-  views::Widget* bubble_widget_;
 
-  DISALLOW_IMPLICIT_CONSTRUCTORS(ContentSettingImageView);
+  DISALLOW_COPY_AND_ASSIGN(ContentSettingImageView);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_CONTENT_SETTING_IMAGE_VIEW_H_