Adds the contextMessage field to notifications.

Reland 220500: Adds the contextMessage field to notifications.

This is a field displayed in an alternate font that adds an extra line
of information about the notification.

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

[jianli, 2013-08-28 21:30:54]

No bug number in the description.

Please also ask rsesek to review the Cocoa part.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
ui/message_center/cocoa/notification_controller.mm:321: // Need to hide the
message if there are items.
Do we also do this on views?

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
ui/message_center/cocoa/notification_controller.mm:344: if
(notification_->context_message().length() == 0) {
nit: empty()

https://codereview.chromium.org/23462005/diff/1/ui/message_center/message_cen...
File ui/message_center/message_center_style.h (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_style.h:50: extern const int
kContextMessageFontSize;    // For everything but title.
the comment is quite confusing with the one in kMessageFontSize.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.cc:48: // Character limits: Displayed
text will be subject to the ine limits above,
ine?

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.h (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.h:74: BoundedLabel*
context_message_view_;
nit: not initialized in the constructor

[dewittj, 2013-08-29 17:40:10]

PTAL

FYI, changing message text color to match title color and context message color
to be the old message text color.  This matches newest spec more closely.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
File ui/message_center/cocoa/notification_controller.mm (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
ui/message_center/cocoa/notification_controller.mm:321: // Need to hide the
message if there are items.
On 2013/08/28 21:30:54, jianli wrote:
> Do we also do this on views?

Yes.  This isn't new, just added a comment for functionality that already
existed.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/cocoa/notif...
ui/message_center/cocoa/notification_controller.mm:344: if
(notification_->context_message().length() == 0) {
On 2013/08/28 21:30:54, jianli wrote:
> nit: empty()

Done.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/message_cen...
File ui/message_center/message_center_style.h (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/message_cen...
ui/message_center/message_center_style.h:50: extern const int
kContextMessageFontSize;    // For everything but title.
On 2013/08/28 21:30:54, jianli wrote:
> the comment is quite confusing with the one in kMessageFontSize.

Removed.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.cc:48: // Character limits: Displayed
text will be subject to the ine limits above,
On 2013/08/28 21:30:54, jianli wrote:
> ine?

Done.

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
File ui/message_center/views/notification_view.h (right):

https://codereview.chromium.org/23462005/diff/1/ui/message_center/views/notif...
ui/message_center/views/notification_view.h:74: BoundedLabel*
context_message_view_;
On 2013/08/28 21:30:54, jianli wrote:
> nit: not initialized in the constructor

yes it is.  See line 501

[dewittj, 2013-08-29 18:22:10]

miket: IDL changes

Let me know if you want a discussion on apps-dev for this, I thought it's minor
enough that it didn't warrant a full API review discussion.

[miket_OOO, 2013-08-29 18:40:49]

On 2013/08/29 18:22:10, dewittj wrote:
> miket: IDL changes
> 
> Let me know if you want a discussion on apps-dev for this, I thought it's
minor
> enough that it didn't warrant a full API review discussion.

IDL LGTM.

[jianli, 2013-08-29 20:08:00]

lgtm

[dewittj, 2013-08-29 20:35:41]

miket: fyi I removed nodoc from the IDL, lmk if that changes your l g t m.

[commit-bot: I haz the power, 2013-08-29 22:53:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/23462005/30001

[commit-bot: I haz the power, 2013-08-30 04:00:19]

Change committed as 220500

[dewittj, 2013-08-30 20:47:48]

Forgot to get rsesek on cocoa/ - but doesn't matter since it was reverted due to
message_center_unittests not running on the try bots :(

rsesek, ptal?

I had to update a test because I noticed and corrected an error in the layout
code for notification_controller.mm - line 316.  Gives us more words per line so
the "large" notification wasn't large enough any more.

[Robert Sesek, 2013-08-30 20:52:20]

cocoa/ LGTM

[commit-bot: I haz the power, 2013-08-30 20:56:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/23462005/49001

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

Change committed as 220688