ContentSettingImageView cleanup, phase 3.

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/utf_string_conversions.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
 #include "chrome/browser/ui/content_settings/content_setting_image_model.h"
@@ skipping 27 matching lines @@
     LocationBarView* parent,
     const gfx::Font& font,
     int font_y_offset,
     SkColor font_color)
     : parent_(parent),
       content_setting_image_model_(
           ContentSettingImageModel::CreateContentSettingImageModel(
               content_type)),
       background_painter_(new views::HorizontalPainter(background_images)),
       icon_(new views::ImageView),
-      text_label_(NULL),
+      text_label_(new views::Label),
       slide_animator_(this),
       pause_animation_(false),
       pause_animation_state_(0.0),
-      bubble_widget_(NULL),
-      font_(font),
-      text_size_(0) {
-  SetLayoutManager(new views::BoxLayout(
-      views::BoxLayout::kHorizontal, 0, 0, 0));
+      bubble_widget_(NULL) {
   icon_->SetHorizontalAlignment(views::ImageView::LEADING);
   AddChildView(icon_);
+
+  text_label_->SetVisible(false);
+  text_label_->set_border(
+      views::Border::CreateEmptyBorder(font_y_offset, 0, 0, 0));
+  text_label_->SetFont(font);
+  text_label_->SetEnabledColor(font_color);
+  text_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  text_label_->SetElideBehavior(views::Label::NO_ELIDE);
+  AddChildView(text_label_);
+
   TouchableLocationBarView::Init(this);
 
   slide_animator_.SetSlideDuration(kAnimationDurationMS);
   slide_animator_.SetTweenType(ui::Tween::LINEAR);
 }
 
 ContentSettingImageView::~ContentSettingImageView() {
   if (bubble_widget_)
     bubble_widget_->RemoveObserver(this);
 }
@@ skipping 23 matching lines @@
       TabSpecificContentSettings::FromWebContents(web_contents) : NULL;
   if (!content_settings || content_settings->IsBlockageIndicated(
       content_setting_image_model_->get_content_settings_type()))
     return;
 
   // We just ignore this blockage if we're already showing some other string to
   // the user.  If this becomes a problem, we could design some sort of queueing
   // mechanism to show one after the other, but it doesn't seem important now.
   int string_id = content_setting_image_model_->explanatory_string_id();
   if (string_id && !background_showing()) {
-    // 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(string_id));
-    text_size_ = font_.GetStringWidth(text_label_->text());
-    text_size_ += kHorizMargin;
+    text_label_->SetVisible(true);
     slide_animator_.Show();
   }
 
   content_settings->SetBlockageHasBeenIndicated(
       content_setting_image_model_->get_content_settings_type());
 }
 
 void ContentSettingImageView::AnimationEnded(const ui::Animation* animation) {
   slide_animator_.Reset();
   if (!pause_animation_) {
-    SetLayoutManager(new views::BoxLayout(
-        views::BoxLayout::kHorizontal, 0, 0, 0));
-    RemoveChildView(text_label_);  // will also delete the view.
-    text_label_ = NULL;
+    text_label_->SetVisible(false);
     parent_->Layout();
     parent_->SchedulePaint();
   }
 }
 
 void ContentSettingImageView::AnimationProgressed(
     const ui::Animation* animation) {
   if (!pause_animation_) {
     parent_->Layout();
     parent_->SchedulePaint();
   }
 }
 
 void ContentSettingImageView::AnimationCanceled(
     const ui::Animation* animation) {
   AnimationEnded(animation);
 }
 
 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);
-
-  int visible_text_size = 0;
+  gfx::Size size(icon_->GetPreferredSize());
   if (background_showing()) {
     double 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.
     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;
-    visible_text_size = size_fraction * text_size_;
+    size.Enlarge(
+        size_fraction * (text_label_->GetPreferredSize().width() +
+            GetTotalSpacingWhileAnimating()), 0);
+    size.ClampToMin(background_painter_->GetMinimumSize());
   }
+  return size;
+}
 
-  preferred_size.set_width(non_label_width + visible_text_size);
-  return preferred_size;
+void ContentSettingImageView::Layout() {

[sky, 2013/05/29 23:37:47]

Does this do the right thing with rtl?

[Peter Kasting, 2013/05/29 23:54:52]

It seems to.  I assume this is because views auto-mirror their children unless
you ask them not to?

+  const int icon_width = icon_->GetPreferredSize().width();
+  icon_->SetBounds(
+      std::min((width() - icon_width) / 2, kHorizMargin), 0,
+      icon_width, height());
+  text_label_->SetBounds(
+      icon_->bounds().right() + kIconLabelSpacing, 0,
+      std::max(width() - GetTotalSpacingWhileAnimating() - icon_width, 0),
+      text_label_->GetPreferredSize().height());
 }
 
 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()))
@@ skipping 18 matching lines @@
   bubble_widget_->RemoveObserver(this);
   bubble_widget_ = NULL;
 
   if (pause_animation_) {
     slide_animator_.Reset(pause_animation_state_);
     pause_animation_ = false;
     slide_animator_.Show();
   }
 }
 
+int ContentSettingImageView::GetTotalSpacingWhileAnimating() const {
+  return kHorizMargin * 2 + kIconLabelSpacing;

[Peter Kasting, 2013/05/29 22:00:02]

Forgot to comment on this -- this function will be more complex, and not
compile-time-constant, in the future, hence the CamelCase name.

+}
+
 void ContentSettingImageView::OnClick() {
   if (slide_animator_.is_animating()) {
     if (!pause_animation_) {
       pause_animation_ = true;
       pause_animation_state_ = slide_animator_.GetCurrentValue();
     }
     slide_animator_.Reset();
   }
 
   content::WebContents* web_contents = parent_->GetWebContents();
   if (web_contents && !bubble_widget_) {
     bubble_widget_ =
         parent_->delegate()->CreateViewsBubble(new ContentSettingBubbleContents(
             ContentSettingBubbleModel::CreateContentSettingBubbleModel(
                 parent_->delegate()->GetContentSettingBubbleModelDelegate(),
                 web_contents, parent_->profile(),
                 content_setting_image_model_->get_content_settings_type()),
             web_contents, this, views::BubbleBorder::TOP_RIGHT));
     bubble_widget_->AddObserver(this);
     bubble_widget_->Show();
   }
 }