Support notifications with custom content

Support notifications with custom content

- Add a NOTIFICATION_TYPE_CUSTOM;
- Add a NotificationDelegate::CreateCustomContent;
- Add a CustomNotificationView for NOTIFICATION_TYPE_CUSTOM that
  hosts the View created in NotificationDelegate::CreateCustomContent
  as its contents;

BUG=610767
TEST=CustomNotificationViewTest.*

Committed: https://crrev.com/b36c784041e2dd9cd8d953b41dcd997d5be51dae
Cr-Commit-Position: refs/heads/master@{#394897}

[xiyuan, 2016-05-13 19:46:56]

xiyuan@chromium.org changed reviewers:
+ dewittj@chromium.org

[xiyuan, 2016-05-13 19:46:57]

The 3rd one. Note the UI about close button (and maybe a title) is not
finalized. The current CL just lays it on top of the custom content at top right
corner.

[xiyuan, 2016-05-13 22:34:15]

xiyuan@chromium.org changed reviewers:
+ yoshiki@chromium.org

[xiyuan, 2016-05-13 22:34:16]

+yoshiki fyi

[yoshiki, 2016-05-16 03:31:09]

LGTM with nits.

Please add NOTIFICATION_TYPE_CUSTOM to 
NotificationConversionHelper::MapTypeToString() in
C/B/notifications/notification_conversion_helper.cc as well.

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/cus...
File ui/message_center/views/custom_notification_view.cc (right):

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/cus...
ui/message_center/views/custom_notification_view.cc:45: if
(contents_view_->background()) {
nit: no need for braces.

[xiyuan, 2016-05-16 16:15:37]

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/cus...
File ui/message_center/views/custom_notification_view.cc (right):

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/cus...
ui/message_center/views/custom_notification_view.cc:45: if
(contents_view_->background()) {
On 2016/05/16 03:31:09, yoshiki wrote:
> nit: no need for braces.

Done.

[dewittj, 2016-05-16 16:19:28]

lgtm with q

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

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/not...
ui/message_center/views/notification_view.cc:151: MessageView*
NotificationView::Create(MessageCenterController* controller,
Seems weird that this function might return a CustomNotificationView.  Would it
make sense to move this into its own file, and then make the constructor for
NotificationView public?

[xiyuan, 2016-05-16 16:31:06]

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

https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/not...
ui/message_center/views/notification_view.cc:151: MessageView*
NotificationView::Create(MessageCenterController* controller,
On 2016/05/16 16:19:28, dewittj wrote:
> Seems weird that this function might return a CustomNotificationView.  Would
it
> make sense to move this into its own file, and then make the constructor for
> NotificationView public?

Makes sense. How about bring back MessageViewFactory (that we removed in
https://chromiumcodereview.appspot.com/11926032)?

[dewittj, 2016-05-16 16:33:14]

On 2016/05/16 16:31:06, xiyuan wrote:
>
https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/not...
> File ui/message_center/views/notification_view.cc (right):
> 
>
https://codereview.chromium.org/1979583003/diff/1/ui/message_center/views/not...
> ui/message_center/views/notification_view.cc:151: MessageView*
> NotificationView::Create(MessageCenterController* controller,
> On 2016/05/16 16:19:28, dewittj wrote:
> > Seems weird that this function might return a CustomNotificationView.  Would
> it
> > make sense to move this into its own file, and then make the constructor for
> > NotificationView public?
> 
> Makes sense. How about bring back MessageViewFactory (that we removed in
> https://chromiumcodereview.appspot.com/11926032)

That's fine by me.

[xiyuan, 2016-05-16 19:59:31]

xiyuan@chromium.org changed reviewers:
+ achuith@chromium.org

[xiyuan, 2016-05-16 19:59:31]

achuith@, could I get an owner stamp for autotest_private_api.cc? Thanks.

[achuithb, 2016-05-16 20:32:49]

On 2016/05/16 19:59:31, xiyuan wrote:
> achuith@, could I get an owner stamp for autotest_private_api.cc? Thanks.

Lgtm

[xiyuan, 2016-05-19 17:21:23]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2016-05-19 17:21:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
yoshiki@chromium.org, dewittj@chromium.org, achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1979583003/#ps60001
(title: "make NotificationDelegate::CreateCustomContent chromeos only to fix mac
build")

[commit-bot: I haz the power, 2016-05-19 17:22:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979583003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979583003/60001

[commit-bot: I haz the power, 2016-05-19 17:28:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-19 17:28:21]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[xiyuan, 2016-05-19 22:24:06]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2016-05-19 22:24:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
yoshiki@chromium.org, achuith@chromium.org, dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/1979583003/#ps80001
(title: "fix mac build, attempt 2")

[commit-bot: I haz the power, 2016-05-19 22:25:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979583003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979583003/80001

[commit-bot: I haz the power, 2016-05-19 23:09:33]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-19 23:10:38]

Description was changed from

==========
Support notifications with custom content

- Add a NOTIFICATION_TYPE_CUSTOM;
- Add a NotificationDelegate::CreateCustomContent;
- Add a CustomNotificationView for NOTIFICATION_TYPE_CUSTOM that
  hosts the View created in NotificationDelegate::CreateCustomContent
  as its contents;

BUG=610767
TEST=CustomNotificationViewTest.*
==========

to

==========
Support notifications with custom content

- Add a NOTIFICATION_TYPE_CUSTOM;
- Add a NotificationDelegate::CreateCustomContent;
- Add a CustomNotificationView for NOTIFICATION_TYPE_CUSTOM that
  hosts the View created in NotificationDelegate::CreateCustomContent
  as its contents;

BUG=610767
TEST=CustomNotificationViewTest.*

Committed: https://crrev.com/b36c784041e2dd9cd8d953b41dcd997d5be51dae
Cr-Commit-Position: refs/heads/master@{#394897}
==========

[commit-bot: I haz the power, 2016-05-19 23:10:39]

Patchset 5 (id:??) landed as
https://crrev.com/b36c784041e2dd9cd8d953b41dcd997d5be51dae
Cr-Commit-Position: refs/heads/master@{#394897}