Follow-up fixes to http://crrev.com/12638033.

ui/message_center/views/notification_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 "ui/message_center/views/notification_view.h"
 
 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "grit/ui_resources.h"
 #include "ui/base/accessibility/accessible_view_state.h"
@@ skipping 46 matching lines @@
 views::Border* MakeBorder(int top,
                           int bottom,
                           int left = kTextLeftPadding,
                           int right = kTextRightPadding,
                           SkColor color = 0x00000000) {
   return (color == 0x00000000) ?
          views::Border::CreateEmptyBorder(top, left, bottom, right) :
          views::Border::CreateSolidSidedBorder(top, left, bottom, right, color);
 }
 
-// ContainerViews is a vertical BoxLayout view that propagates its childrens'
+// ContainerView ///////////////////////////////////////////////////////////////

[dharcourt, 2013/03/13 22:47:51]

It's becoming hard to find nested classes in this file. These comments are an
attempt to solve that problem.

+
+// ContainerViews are vertical BoxLayout views that propagates their childrens'
 // ChildPreferredSizeChanged() and ChildVisibilityChanged() calls.
 class ContainerView : public views::View {
  public:
   ContainerView();
   virtual ~ContainerView();
 
  protected:
   virtual void ChildPreferredSizeChanged(View* child) OVERRIDE;
   virtual void ChildVisibilityChanged(View* child) OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ContainerView);
 };
 
 ContainerView::ContainerView() {
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
 }
 
 ContainerView::~ContainerView() {
 }
 
 void ContainerView::ChildPreferredSizeChanged(View* child) {
   PreferredSizeChanged();
 }
 
 void ContainerView::ChildVisibilityChanged(View* child) {
   PreferredSizeChanged();
 }
 
+// ItemView ////////////////////////////////////////////////////////////////////
+
 // ItemViews are responsible for drawing each list notification item's title and
 // message next to each other within a single column.
 class ItemView : public views::View {
  public:
   ItemView(const message_center::NotificationItem& item);
   virtual ~ItemView();
 
   virtual void SetVisible(bool visible) OVERRIDE;
 
  private:
@@ skipping 26 matching lines @@
 
 ItemView::~ItemView() {
 }
 
 void ItemView::SetVisible(bool visible) {
   views::View::SetVisible(visible);
   for (int i = 0; i < child_count(); ++i)
     child_at(i)->SetVisible(visible);
 }
 
+// ProportionalImageView ///////////////////////////////////////////////////////
+
 // ProportionalImageViews center their images to preserve their proportion.
-// Note that for this subclass of views::ImageView GetImageBounds() will return
-// potentially incorrect values (this can't be fixed because GetImageBounds()
-// is not virtual) and horizontal and vertical alignments will be ignored.
-class ProportionalImageView : public views::ImageView {
+class ProportionalImageView : public views::View {

[dharcourt, 2013/03/13 22:47:51]

Rather than having 3 lines of comment explaining why ProportionalImageView
shouldn't be treated as an ImageView, it's simpler to simply not have
ProportionalImageView be an ImageView subclass.

  public:
-  ProportionalImageView();
+  ProportionalImageView(const gfx::ImageSkia& image);
   virtual ~ProportionalImageView();
 
   // Overridden from views::View:
   virtual gfx::Size GetPreferredSize() OVERRIDE;
   virtual int GetHeightForWidth(int width) OVERRIDE;
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
 
  private:
   gfx::Size GetImageSizeForWidth(int width);
 
+  gfx::ImageSkia image_;
+
   DISALLOW_COPY_AND_ASSIGN(ProportionalImageView);
 };
 
-ProportionalImageView::ProportionalImageView() {
+ProportionalImageView::ProportionalImageView(const gfx::ImageSkia& image)
+    : image_(image) {
 }
 
 ProportionalImageView::~ProportionalImageView() {
 }
 
 gfx::Size ProportionalImageView::GetPreferredSize() {
-  gfx::Size size = GetImageSizeForWidth(GetImage().width());
+  gfx::Size size = GetImageSizeForWidth(image_.width());
   return gfx::Size(size.width() + GetInsets().width(),
                    size.height() + GetInsets().height());
 }
 
 int ProportionalImageView::GetHeightForWidth(int width) {
   return GetImageSizeForWidth(width).height();
 }
 
 void ProportionalImageView::OnPaint(gfx::Canvas* canvas) {
-  View::OnPaint(canvas);
+  views::View::OnPaint(canvas);
 
   gfx::Size draw_size(GetImageSizeForWidth(width()));
   if (!draw_size.IsEmpty()) {
-    int x = (width() - draw_size.width()) / 2;
-    int y = (height() - draw_size.height()) / 2;
+    gfx::Rect draw_bounds = GetLocalBounds();
+    draw_bounds.Inset(GetInsets());
+    draw_bounds.ClampToCenteredSize(draw_size);

[dharcourt, 2013/03/13 22:47:51]

Fixed these calculations to work when there are insets, using
ClampToCenteredSize() to cut down on the boilerplate computations.

 
-    gfx::Size image_size(GetImage().size());
+    gfx::Size image_size(image_.size());
     if (image_size == draw_size) {
-      canvas->DrawImageInt(GetImage(), x, y);
+      canvas->DrawImageInt(image_, draw_bounds.x(), draw_bounds.y());
     } else {
       // Resize case
       SkPaint paint;
       paint.setFilterBitmap(true);
-      canvas->DrawImageInt(GetImage(), 0, 0,
-                           image_size.width(), image_size.height(), x, y,
-                           draw_size.width(), draw_size.height(), true, paint);
+      canvas->DrawImageInt(image_, 0, 0,
+                           image_size.width(), image_size.height(),
+                           draw_bounds.x(), draw_bounds.y(),
+                           draw_size.width(), draw_size.height(),
+                           true, paint);
     }
   }
 }
 
 gfx::Size ProportionalImageView::GetImageSizeForWidth(int width) {
-  gfx::Size size = visible() ? GetImage().size() : gfx::Size();
+  gfx::Size size = visible() ? image_.size() : gfx::Size();
   if (width > 0 && !size.IsEmpty()) {
     double proportion = size.height() / (double) size.width();
     size.SetSize(width, std::max(0.5 + width * proportion, 1.0));
     if (size.height() > message_center::kNotificationMaximumImageHeight) {
       int height = message_center::kNotificationMaximumImageHeight;
       size.SetSize(std::max(0.5 + height / proportion, 1.0), height);
     }
   }
   return size;
 }
 
-// NotificationsButtons render the action buttons of notifications.
+// NotificationButton //////////////////////////////////////////////////////////
+
+// NotificationButtons render the action buttons of notifications.
 class NotificationButton : public views::CustomButton {
  public:
   NotificationButton(views::ButtonListener* listener);
   virtual ~NotificationButton();
 
   void SetIcon(const gfx::ImageSkia& icon);
   void SetTitle(const string16& title);
 
   // Overridden from views::View:
   virtual gfx::Size GetPreferredSize() OVERRIDE;
@@ skipping 55 matching lines @@
 }
 
 int NotificationButton::GetHeightForWidth(int width) {
   return kButtonHeight;
 }
 
 }  // namespace
 
 namespace message_center {
 
+// NotificationView ////////////////////////////////////////////////////////////
+
 // static
 MessageView* NotificationView::Create(const Notification& notification,
                                       NotificationChangeObserver* observer,
                                       bool expanded) {
   // For the time being, use MessageSimpleView for simple notifications unless
   // one of the use-the-new-style flags are set. This preserves the appearance
   // of notifications created by existing code that uses webkitNotifications.
   if (notification.type() == NOTIFICATION_TYPE_SIMPLE &&
       !IsRichNotificationEnabled() &&
       !CommandLine::ForCurrentProcess()->HasSwitch(
@@ skipping 73 matching lines @@
   std::vector<NotificationItem> items = notification.items();
   for (size_t i = 0; i < items.size() && i < kNotificationMaximumItems; ++i) {
     ItemView* item_view = new ItemView(items[i]);
     item_view->SetVisible(is_expanded());
     item_view->set_border(MakeBorder(0, 4));
     item_views_.push_back(item_view);
     top_view_->AddChildView(item_view);
   }
 
   // Create the notification icon view.
-  icon_view_ = new ProportionalImageView();
-  icon_view_->SetImage(notification.icon().AsImageSkia());
+  icon_view_ = new ProportionalImageView(notification.icon().AsImageSkia());
 
   // Create the bottom_view_, which collects into a vertical box all content
   // below the notification icon except for the expand button.
   bottom_view_ = new ContainerView();
   bottom_view_->set_background(
       views::Background::CreateSolidBackground(kBackgroundColor));
   
   // Create the image view if appropriate.
   image_view_ = NULL;
   if (!notification.image().IsEmpty()) {
-    image_view_ = new ProportionalImageView();
+    image_view_ = new ProportionalImageView(notification.image().AsImageSkia());
     image_view_->SetVisible(is_expanded());
-    image_view_->SetImage(notification.image().ToImageSkia());
     bottom_view_->AddChildView(image_view_);
   }
 
   // Create action buttons if appropriate.
   std::vector<ButtonInfo> buttons = notification.buttons();
   for (size_t i = 0; i < buttons.size(); ++i) {
     views::View* separator = new views::ImageView();
     separator->set_border(MakeBorder(1, 0, 0, 0, kButtonSeparatorColor));
     bottom_view_->AddChildView(separator);
     NotificationButton* button = new NotificationButton(this);
     ButtonInfo button_info = buttons[i];
     button->SetTitle(button_info.title);
     button->SetIcon(button_info.icon.AsImageSkia());
     action_buttons_.push_back(button);
     bottom_view_->AddChildView(button);
   }
 
   // Hide the expand button if appropriate.
   bool expandable = item_views_.size() || image_view_;
   expand_button()->SetVisible(expandable && !is_expanded());
 
   // Put together the different content and control views. Layering those allows
-  // for proper layout logit and it also allows the close and expand buttons to
+  // for proper layout logic and it also allows the close and expand buttons to
   // overlap the content as needed to provide large enough click and touch areas
   // (<http://crbug.com/168822> and <http://crbug.com/168856>).
   AddChildView(background_view_);
   AddChildView(top_view_);
   AddChildView(icon_view_);
   AddChildView(bottom_view_);
   AddChildView(close_button());
   AddChildView(expand_button());
 }
 
@@ skipping 65 matching lines @@
       item_views_[i]->SetVisible(true);
     if (image_view_)
       image_view_->SetVisible(true);
     expand_button()->SetVisible(false);
     PreferredSizeChanged();
     SchedulePaint();
   }
 }
 
 }  // namespace message_center