Notifications should resize themselves to the content within min-max bounds, rather than being all the same size.

CL hooks into RenderView callbacks to detect the size of the content, and contains some refactoring so that conceptually balloon size = content size + frame, rather than content size = balloon size - frame as it has been.

BUG=26691
TEST=included

Review URL: http://codereview.chromium.org/460131

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

7 files changed, 90 insertions, 30 deletions

diff --git a/chrome/browser/notifications/balloon.cc b/chrome/browser/notifications/balloon.cc
index d9d4c1f..da49395 100644
--- a/chrome/browser/notifications/balloon.cc
+++ b/chrome/browser/notifications/balloon.cc
@@ -10,10 +10,10 @@
 #include "chrome/browser/renderer_host/site_instance.h"
 
 Balloon::Balloon(const Notification& notification, Profile* profile,
-                 BalloonCloseListener* listener)
+                 BalloonCollection* collection)
     : profile_(profile),
       notification_(notification),
-      close_listener_(listener) {
+      collection_(collection) {
 }
 
 Balloon::~Balloon() {
@@ -25,6 +25,10 @@ void Balloon::SetPosition(const gfx::Point& upper_left, bool reposition) {
     balloon_view_->RepositionToBalloon();
 }
 
+void Balloon::SetContentPreferredSize(const gfx::Size& size) {
+  collection_->ResizeBalloon(this, size);
+}
+
 void Balloon::set_view(BalloonView* balloon_view) {
   balloon_view_.reset(balloon_view);
 }
@@ -38,8 +42,7 @@ void Balloon::Show() {
 
 void Balloon::OnClose(bool by_user) {
   notification_.Close(by_user);
-  if (close_listener_)
-    close_listener_->OnBalloonClosed(this);
+  collection_->OnBalloonClosed(this);
 }
 
 void Balloon::CloseByScript() {
diff --git a/chrome/browser/notifications/balloon.h b/chrome/browser/notifications/balloon.h
index b0f49b4..b0ead85 100644
--- a/chrome/browser/notifications/balloon.h
+++ b/chrome/browser/notifications/balloon.h
@@ -17,6 +17,7 @@
 #include "chrome/browser/notifications/notification.h"
 
 class Balloon;
+class BalloonCollection;
 class Profile;
 class SiteInstance;
 
@@ -33,23 +34,17 @@ class BalloonView {
 
   // Close the view.
   virtual void Close(bool by_user) = 0;
+
+  // The total size of the view.
+  virtual gfx::Size GetSize() const = 0;
 };
 
 // Represents a Notification on the screen.
 class Balloon {
  public:
-  class BalloonCloseListener {
-   public:
-    virtual ~BalloonCloseListener() {}
-
-    // Called when a balloon is closed.
-    virtual void OnBalloonClosed(Balloon* source) = 0;
-  };
-
-  // |listener| may be null in unit tests w/o actual UI.
   Balloon(const Notification& notification,
           Profile* profile,
-          BalloonCloseListener* listener);
+          BalloonCollection* collection);
   virtual ~Balloon();
 
   const Notification& notification() const { return notification_; }
@@ -58,13 +53,21 @@ class Balloon {
   const gfx::Point& position() const { return position_; }
   void SetPosition(const gfx::Point& upper_left, bool reposition);
 
-  const gfx::Size& size() const { return size_; }
-  void set_size(const gfx::Size& size) { size_ = size; }
+  const gfx::Size& content_size() const { return content_size_; }
+  void set_content_size(const gfx::Size& size) { content_size_ = size; }
+
+  // Request a new content size for this balloon.  This will get passed
+  // to the balloon collection for checking against available space and
+  // min/max restrictions.
+  void SetContentPreferredSize(const gfx::Size& size);
 
   // Provides a view for this balloon.  Ownership transfers
   // to this object.
   void set_view(BalloonView* balloon_view);
 
+  // Returns the viewing size for the balloon (content + frame).
+  gfx::Size GetViewSize() const { return balloon_view_->GetSize(); }
+
   // Shows the balloon.
   virtual void Show();
 
@@ -82,15 +85,15 @@ class Balloon {
   // The notification being shown in this balloon.
   Notification notification_;
 
-  // A listener to be called when the balloon closes.
-  BalloonCloseListener* close_listener_;
+  // The collection that this balloon belongs to.  Non-owned pointer.
+  BalloonCollection* collection_;
 
   // The actual UI element for the balloon.
   scoped_ptr<BalloonView> balloon_view_;
 
   // Position and size of the balloon on the screen.
   gfx::Point position_;
-  gfx::Size size_;
+  gfx::Size content_size_;
 
   DISALLOW_COPY_AND_ASSIGN(Balloon);
 };
diff --git a/chrome/browser/notifications/balloon_collection.cc b/chrome/browser/notifications/balloon_collection.cc
index 194ebdd..b6760d5 100644
--- a/chrome/browser/notifications/balloon_collection.cc
+++ b/chrome/browser/notifications/balloon_collection.cc
@@ -76,6 +76,19 @@ bool BalloonCollectionImpl::HasSpace() const {
   return current_max_size < max_allowed_size - max_balloon_size;
 }
 
+void BalloonCollectionImpl::ResizeBalloon(Balloon* balloon,
+                                          const gfx::Size& size) {
+  // restrict to the min & max sizes
+  gfx::Size real_size(
+      std::max(Layout::min_balloon_width(),
+          std::min(Layout::max_balloon_width(), size.width())),
+      std::max(Layout::min_balloon_height(),
+          std::min(Layout::max_balloon_height(), size.height())));
+
+  balloon->set_content_size(real_size);
+  PositionBalloons(true);
+}
+
 void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
   // We want to free the balloon when finished.
   scoped_ptr<Balloon> closed(source);
@@ -95,7 +108,7 @@ void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) {
 void BalloonCollectionImpl::PositionBalloons(bool reposition) {
   gfx::Point origin = layout_.GetLayoutOrigin();
   for (Balloons::iterator it = balloons_.begin(); it != balloons_.end(); ++it) {
-    gfx::Point upper_left = layout_.NextPosition((*it)->size(), &origin);
+    gfx::Point upper_left = layout_.NextPosition((*it)->GetViewSize(), &origin);
     (*it)->SetPosition(upper_left, reposition);
   }
 }
diff --git a/chrome/browser/notifications/balloon_collection.h b/chrome/browser/notifications/balloon_collection.h
index 918f993..8810b18 100644
--- a/chrome/browser/notifications/balloon_collection.h
+++ b/chrome/browser/notifications/balloon_collection.h
@@ -32,14 +32,19 @@ class BalloonCollection {
 
   // Is there room to add another notification?
   virtual bool HasSpace() const = 0;
+
+  // Request the resizing of a balloon.
+  virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size) = 0;
+
+  // Inform the collection that a balloon was closed.
+  virtual void OnBalloonClosed(Balloon* source) = 0;
 };
 
 // A balloon collection represents a set of notification balloons being
 // shown on the screen.  It positions new notifications according to
 // a layout, and monitors for balloons being closed, which it reports
 // up to its parent, the notification UI manager.
-class BalloonCollectionImpl : public BalloonCollection,
-                              public Balloon::BalloonCloseListener {
+class BalloonCollectionImpl : public BalloonCollection {
  public:
   class BalloonSpaceChangeListener {
    public:
@@ -65,8 +70,7 @@ class BalloonCollectionImpl : public BalloonCollection,
                    Profile* profile);
   virtual bool Remove(const Notification& notification);
   virtual bool HasSpace() const;
-
-  // Balloon::BalloonCloseListener interface
+  virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size);
   virtual void OnBalloonClosed(Balloon* source);
 
  protected:
@@ -117,10 +121,10 @@ class BalloonCollectionImpl : public BalloonCollection,
       VERTICALLY_FROM_BOTTOM_RIGHT
     };
 
-    // Minimum and maximum size of balloon
+    // Minimum and maximum size of balloon content.
     static const int kBalloonMinWidth = 300;
     static const int kBalloonMaxWidth = 300;
-    static const int kBalloonMinHeight = 90;
+    static const int kBalloonMinHeight = 48;
     static const int kBalloonMaxHeight = 120;
 
     static Placement placement_;
diff --git a/chrome/browser/notifications/balloon_collection_win.cc b/chrome/browser/notifications/balloon_collection_win.cc
index 45f7921..53d9808 100644
--- a/chrome/browser/notifications/balloon_collection_win.cc
+++ b/chrome/browser/notifications/balloon_collection_win.cc
@@ -34,7 +34,7 @@ Balloon* BalloonCollectionImpl::MakeBalloon(const Notification& notification,
   Balloon* balloon = new Balloon(notification, profile, this);
   balloon->set_view(new BalloonViewImpl());
   gfx::Size size(layout_.min_balloon_width(), layout_.min_balloon_height());
-  balloon->set_size(size);
+  balloon->set_content_size(size);
   return balloon;
 }
 
diff --git a/chrome/browser/notifications/desktop_notifications_unittest.cc b/chrome/browser/notifications/desktop_notifications_unittest.cc
index 8d0bcf6..32bda04 100644
--- a/chrome/browser/notifications/desktop_notifications_unittest.cc
+++ b/chrome/browser/notifications/desktop_notifications_unittest.cc
@@ -70,8 +70,8 @@ int MockBalloonCollection::UppermostVerticalPosition() {
   return min;
 }
 
-DesktopNotificationsTest::DesktopNotificationsTest() :
-    ui_thread_(ChromeThread::UI, &message_loop_) {
+DesktopNotificationsTest::DesktopNotificationsTest()
+    : ui_thread_(ChromeThread::UI, &message_loop_) {
 }
 
 DesktopNotificationsTest::~DesktopNotificationsTest() {
@@ -183,6 +183,41 @@ TEST_F(DesktopNotificationsTest, TestPositioning) {
 
   EXPECT_EQ(expected_log, log_output_);
 }
+
+TEST_F(DesktopNotificationsTest, TestVariableSize) {
+  std::string expected_log;
+  // Create some toasts.  After each but the first, make sure there
+  // is a minimum separation between the toasts.
+  int last_top = 0;
+  EXPECT_TRUE(service_->ShowDesktopNotificationText(
+      GURL("http://long.google.com"), GURL("/icon.png"),
+      ASCIIToUTF16("Really Really Really Really Really Really "
+          "Really Really Really Really Really Really "
+          "Really Really Really Really Really Really Really Long Title"),
+      ASCIIToUTF16("Text"),
+      0, 0, DesktopNotificationService::PageNotification, 0));
+  expected_log.append("notification displayed\n");
+  EXPECT_TRUE(service_->ShowDesktopNotificationText(
+      GURL("http://short.google.com"), GURL("/icon.png"),
+      ASCIIToUTF16("Short title"), ASCIIToUTF16("Text"),
+      0, 0, DesktopNotificationService::PageNotification, 1));
+  expected_log.append("notification displayed\n");
+
+  std::set<Balloon*>& balloons = balloon_collection_->balloons();
+  std::set<Balloon*>::iterator iter;
+  for (iter = balloons.begin(); iter != balloons.end(); ++iter) {
+    if ((*iter)->notification().origin_url().host() == "long.google.com") {
+      EXPECT_GE((*iter)->GetViewSize().height(),
+                balloon_collection_->MinHeight());
+      EXPECT_LE((*iter)->GetViewSize().height(),
+                balloon_collection_->MaxHeight());
+    } else {
+      EXPECT_EQ((*iter)->GetViewSize().height(),
+                balloon_collection_->MinHeight());
+    }
+  }
+  EXPECT_EQ(expected_log, log_output_);
+}
 #endif
 
 TEST_F(DesktopNotificationsTest, TestQueueing) {
diff --git a/chrome/browser/notifications/desktop_notifications_unittest.h b/chrome/browser/notifications/desktop_notifications_unittest.h
index b06aa6e..4297768 100644
--- a/chrome/browser/notifications/desktop_notifications_unittest.h
+++ b/chrome/browser/notifications/desktop_notifications_unittest.h
@@ -45,6 +45,7 @@ class MockBalloonView : public BalloonView {
   void Show(Balloon* balloon) {}
   void RepositionToBalloon() {}
   void Close(bool by_user) { balloon_->OnClose(by_user); }
+  gfx::Size GetSize() const { return balloon_->content_size(); }
 
  private:
   // Non-owned pointer.
@@ -79,8 +80,9 @@ class MockBalloonCollection : public BalloonCollectionImpl {
   // Returns the highest y-coordinate of all the balloons in the collection.
   int UppermostVerticalPosition();
 
-  // Returns the minimum height of a balloon.
+  // Returns the height bounds of a balloon.
   int MinHeight() { return Layout::min_balloon_height(); }
+  int MaxHeight() { return Layout::max_balloon_height(); }
 
  private:
   std::set<Balloon*> balloons_;