[Mac][MC] Implement notification updates and relayouts.

ui/message_center/cocoa/popup_collection.mm

Index: ui/message_center/cocoa/popup_collection.mm
diff --git a/ui/message_center/cocoa/popup_collection.mm b/ui/message_center/cocoa/popup_collection.mm
index e77c3abc24af3662a615be7fdab3c0d04ffcca3d..cf286f6cc60abc32be138ddd18daa5567c7e6fbd 100644
--- a/ui/message_center/cocoa/popup_collection.mm
+++ b/ui/message_center/cocoa/popup_collection.mm
@@ -4,7 +4,8 @@
 
 #import "ui/message_center/cocoa/popup_collection.h"
 
-#include "ui/message_center/cocoa/popup_controller.h"
+#import "ui/message_center/cocoa/notification_controller.h"
+#import "ui/message_center/cocoa/popup_controller.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/message_center_constants.h"
 #include "ui/message_center/message_center_observer.h"
@@ -17,12 +18,20 @@
 // Returns YES if the notification was actually displayed.
 - (BOOL)addNotification:(const message_center::Notification*)notification;
 
+// Updates the contents of the notification with the given ID.
+- (void)updateNotification:(const std::string&)notificationID;
+
 // Removes a popup from the screen and lays out new notifications that can
 // now potentially fit on the screen.
 - (void)removeNotification:(const std::string&)notificationID;
 
 // Closes all the popups.
 - (void)removeAllNotifications;
+
+// Positions popup notifications for the |index|th notification in |popups_|.
+// This will put the proper amount of space between popup |index - 1| and
+// |index| and then any subsequent notifications.
+- (void)layoutNotificationsStartingAt:(NSUInteger)index;
 @end
 
 namespace {
@@ -58,7 +67,7 @@ class PopupCollectionObserver : public message_center::MessageCenterObserver {
 
   virtual void OnNotificationUpdated(
       const std::string& notification_id) OVERRIDE {
-    // TODO(rsesek): Refactor MCNotificationController to support updating.
+    [popup_collection_ updateNotification:notification_id];
   }
 
  private:
@@ -130,52 +139,65 @@ class PopupCollectionObserver : public message_center::MessageCenterObserver {
   return NO;
 }
 
-- (void)removeNotification:(const std::string&)notificationID {
-  // The set of notification IDs that are currently displayed.
-  std::set<std::string> onScreen;
+- (void)updateNotification:(const std::string&)notificationID {
+  // Find the controller for this notification ID.
+  NSUInteger index = [popups_ indexOfObjectPassingTest:
+      ^BOOL(id popup, NSUInteger i, BOOL* stop) {
+          return [popup notificationID] == notificationID;
+      }];
+  // The notification may not be on screen.
+  if (index == NSNotFound)
+    return;
 
-  // Find the popup for the given ID.
-  NSUInteger index = 0;
-  MCPopupController* popup = nil;
-  for (MCPopupController* candidate in popups_.get()) {
-    if ([candidate notificationID] == notificationID) {
-      popup = candidate;
-    } else {
-      onScreen.insert([candidate notificationID]);
+  // Find the corresponding model object for the ID. This may be a replaced
+  // notification, so the controller's current model object may be stale.
+  const message_center::Notification* notification = NULL;
+  const auto& modelPopups = messageCenter_->GetPopupNotifications();
+  for (auto it = modelPopups.begin(); it != modelPopups.end(); ++it) {

[sail, 2013/05/03 21:42:32]

nit, this seems like something we'd want to do on all platforms, maybe add a
MessageCenter::GetPopupNotificationWithID()?

[Robert Sesek, 2013/05/03 21:59:18]

Justin: WDYT? NotificationList has this method but it's private.

[dewittj, 2013/05/03 22:31:37]

sounds OK to me

+    if ((*it)->id() == notificationID) {
+      notification = *it;
+      break;
     }
+  }
+  if (!notification) {
+    NOTREACHED();

[sail, 2013/05/03 21:42:32]

I don't think this is a good use of NOTREACHED(). Maybe remove the if statement
and use a DCHECK instead?

[Robert Sesek, 2013/05/03 21:59:18]

Done.

+    return;
+  }
+
+  MCPopupController* popup = [popups_ objectAtIndex:index];
 
-    // If the popup has not yet been found, increase the index.
-    if (!popup)
-      ++index;
+  CGFloat oldHeight =
+      NSHeight([[[popup notificationController] view] frame]);
+  CGFloat newHeight = NSHeight(
+      [[popup notificationController] updateNotification:notification]);
+
+  // The notification has changed height. This requires updating the popup
+  // window and any popups that come below it.
+  if (oldHeight != newHeight) {
+    NSRect popupFrame = [[popup window] frame];
+    popupFrame.size.height += newHeight - oldHeight;
+    [[popup window] setFrame:popupFrame display:YES];
+
+    // Start re-layout of notifications at this index, so that it readjusts
+    // the Y origin of this notification.
+    [self layoutNotificationsStartingAt:index];
   }
+}
 
-  if (!popup)
+- (void)removeNotification:(const std::string&)notificationID {
+  NSUInteger index = [popups_ indexOfObjectPassingTest:
+      ^BOOL(id popup, NSUInteger index, BOOL* stop) {
+          return [popup notificationID] == notificationID;
+      }];
+  // The notification may not be on screen.
+  if (index == NSNotFound)
     return;
 
-  // Calculate its height and then close it.
-  CGFloat delta = NSHeight([[popup window] frame]) +
-      message_center::kMarginBetweenItems;
-  [popup close];
+  [[popups_ objectAtIndex:index] close];
   [popups_ removeObjectAtIndex:index];
 
-  // Shift all the popups up.
-  for ( ; index < [popups_ count]; ++index) {
-    NSWindow* window = [[popups_ objectAtIndex:index] window];
-    NSPoint origin = [window frame].origin;
-    origin.y += delta;
-    [window setFrameOrigin:origin];
-  }
-
-  // Display any new popups that can now fit on screen.
-  const auto& allPopups = messageCenter_->GetPopupNotifications();
-  for (auto it = allPopups.begin(); it != allPopups.end(); ++it) {
-    if (onScreen.find((*it)->id()) == onScreen.end()) {
-      // If there's no room left on screen to display notifications, stop
-      // trying.
-      if (![self addNotification:*it])
-        break;
-    }
-  }
+  if (index < [popups_ count])
+    [self layoutNotificationsStartingAt:index];
 }
 
 - (void)removeAllNotifications {
@@ -183,4 +205,65 @@ class PopupCollectionObserver : public message_center::MessageCenterObserver {
   [popups_ removeAllObjects];
 }
 
+- (void)layoutNotificationsStartingAt:(NSUInteger)index {
+  DCHECK_GE(index, 0u);

[sail, 2013/05/03 21:42:32]

since this is unsigned, won't this always be true?

[Robert Sesek, 2013/05/03 21:59:18]

Yes, |index| wasn't always unsigned in a previous git commit.

+
+  NSRect screenFrame = [self screenFrame];
+  std::set<std::string> onScreen;
+
+  // Calcluate the bottom edge of the |index - 1|th popup.
+  CGFloat maxY = 0;
+  if (index == 0)
+    maxY = NSMaxY(screenFrame);
+  else
+    maxY = NSMinY([[[popups_ objectAtIndex:index - 1] window] frame]);
+
+  // Collect the IDs of popups that are currently on screen.
+  for (NSUInteger i = 0; i < index; ++i)
+    onScreen.insert([[popups_ objectAtIndex:i] notificationID]);
+
+  // Iterate between [index, count) notifications and reposition each. If one
+  // does not fit on screen, close it and any other on-screen popups that come
+  // after it.
+  NSUInteger removeAt = NSNotFound;
+  for (NSUInteger i = index; i < [popups_ count]; ++i) {
+    MCPopupController* popup = [popups_ objectAtIndex:i];
+    NSRect frame = [[popup window] frame];
+    frame.origin.y = maxY - message_center::kMarginBetweenItems -
+                     NSHeight(frame);
+
+    // If this popup does not fit on screen, stop repositioning and close this
+    // and subsequent popups.
+    if (NSMinY(frame) < NSMinY(screenFrame)) {
+      removeAt = i;
+      break;
+    }
+
+    [[popup window] setFrame:frame display:YES];
+    onScreen.insert([popup notificationID]);
+
+    // Set the new maximum Y to be the bottom of this notification.
+    maxY = NSMinY(frame);
+  }
+
+  if (removeAt != NSNotFound) {
+    // Remove any popups that are on screen but no longer fit.
+    while ([popups_ count] >= removeAt) {
+      [[popups_ lastObject] close];
+      [popups_ removeLastObject];
+    }
+  } else {
+    // Display any new popups that can now fit on screen.
+    const auto& allPopups = messageCenter_->GetPopupNotifications();
+    for (auto it = allPopups.begin(); it != allPopups.end(); ++it) {
+      if (onScreen.find((*it)->id()) == onScreen.end()) {
+        // If there's no room left on screen to display notifications, stop
+        // trying.
+        if (![self addNotification:*it])
+          break;
+      }
+    }
+  }
+}
+
 @end