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

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

BUG=179904
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=198592

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

Objective-C code LGTM

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
File ui/message_center/cocoa/popup_collection.mm (right):

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:156: for (auto it =
modelPopups.begin(); it != modelPopups.end(); ++it) {
nit, this seems like something we'd want to do on all platforms, maybe add a
MessageCenter::GetPopupNotificationWithID()?

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:163: NOTREACHED();
I don't think this is a good use of NOTREACHED(). Maybe remove the if statement
and use a DCHECK instead?

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:209: DCHECK_GE(index, 0u);
since this is unsigned, won't this always be true?

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

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
File ui/message_center/cocoa/popup_collection.mm (right):

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:156: for (auto it =
modelPopups.begin(); it != modelPopups.end(); ++it) {
On 2013/05/03 21:42:32, sail wrote:
> nit, this seems like something we'd want to do on all platforms, maybe add a
> MessageCenter::GetPopupNotificationWithID()?

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

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:163: NOTREACHED();
On 2013/05/03 21:42:32, sail wrote:
> I don't think this is a good use of NOTREACHED(). Maybe remove the if
statement
> and use a DCHECK instead?

Done.

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:209: DCHECK_GE(index, 0u);
On 2013/05/03 21:42:32, sail wrote:
> since this is unsigned, won't this always be true?

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

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

lgtm

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
File ui/message_center/cocoa/popup_collection.mm (right):

https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
ui/message_center/cocoa/popup_collection.mm:156: for (auto it =
modelPopups.begin(); it != modelPopups.end(); ++it) {
On 2013/05/03 21:59:18, rsesek wrote:
> On 2013/05/03 21:42:32, sail wrote:
> > nit, this seems like something we'd want to do on all platforms, maybe add a
> > MessageCenter::GetPopupNotificationWithID()?
> 
> Justin: WDYT? NotificationList has this method but it's private.

sounds OK to me

[Robert Sesek, 2013-05-06 19:32:13]

On 2013/05/03 22:31:37, dewittj wrote:
> lgtm
> 
>
https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
> File ui/message_center/cocoa/popup_collection.mm (right):
> 
>
https://codereview.chromium.org/14598015/diff/1/ui/message_center/cocoa/popup...
> ui/message_center/cocoa/popup_collection.mm:156: for (auto it =
> modelPopups.begin(); it != modelPopups.end(); ++it) {
> On 2013/05/03 21:59:18, rsesek wrote:
> > On 2013/05/03 21:42:32, sail wrote:
> > > nit, this seems like something we'd want to do on all platforms, maybe add
a
> > > MessageCenter::GetPopupNotificationWithID()?
> > 
> > Justin: WDYT? NotificationList has this method but it's private.
> 
> sounds OK to me

It doesn't seem that other platforms need this, really. It may make more sense
to have the MessageCenterObserver send |const Notification&| instead of |const
std::string& notification_id|, but that change would be independent.

[commit-bot: I haz the power, 2013-05-06 19:32:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/14598015/7001

[commit-bot: I haz the power, 2013-05-06 23:30:06]

Retried try job too often on mac_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-05-07 01:17:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/14598015/7001

[commit-bot: I haz the power, 2013-05-07 01:59:00]

Change committed as 198592