Move close button to MessageView

Move close button to MessageView

So that it is shared with derived classes.

BUG=610767
Committed: https://crrev.com/045c33eb3e2647417ff8c5d8d42c67044b25f7d6
Cr-Commit-Position: refs/heads/master@{#393849}

[xiyuan, 2016-05-13 19:45:47]

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

[xiyuan, 2016-05-13 19:45:47]

The 2nd CL.

[dewittj, 2016-05-13 19:58:38]

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

[dewittj, 2016-05-13 19:58:38]

yoshiki moved this from MessageView -> NotificationView recently
(https://codereview.chromium.org/1645843003) , so adding him to ensure that your
goals are compatible.

[dewittj, 2016-05-13 20:28:35]

Pending approval from yoshiki, lgtm

https://codereview.chromium.org/1980753002/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.cc (right):

https://codereview.chromium.org/1980753002/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:30: const int kCloseIconTopPadding = 5;
nit: I think that constexpr is slightly preferred now.

[xiyuan, 2016-05-13 22:31:00]

Will wait for yoshiki...

https://codereview.chromium.org/1980753002/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.cc (right):

https://codereview.chromium.org/1980753002/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:30: const int kCloseIconTopPadding = 5;
On 2016/05/13 20:28:35, dewittj wrote:
> nit: I think that constexpr is slightly preferred now.

Done.

[yoshiki, 2016-05-16 03:11:46]

https://codereview.chromium.org/1980753002/diff/20001/ui/message_center/views...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/1980753002/diff/20001/ui/message_center/views...
ui/message_center/views/notification_view.cc:230:
CreateOrUpdateCloseButtonView(notification);
When CreateOrUpdateViews() is called from
NotificationView::UpdateWithNotification(), CreateOrUpdateCloseButtonView() is
called twice: in MessageCenterView::UpdateWithNotification() and here.

CreateOrUpdateViews() is called from NotificationView::UpdateWithNotification()
and the ctor. Can we remove this calling CreateOrUpdateCloseButtonView() from
here and move it to the ctor?
So that CreateOrUpdateCloseButtonView() is not called twice.

[xiyuan, 2016-05-16 16:11:55]

https://codereview.chromium.org/1980753002/diff/20001/ui/message_center/views...
File ui/message_center/views/notification_view.cc (right):

https://codereview.chromium.org/1980753002/diff/20001/ui/message_center/views...
ui/message_center/views/notification_view.cc:230:
CreateOrUpdateCloseButtonView(notification);
On 2016/05/16 03:11:46, yoshiki wrote:
> When CreateOrUpdateViews() is called from
> NotificationView::UpdateWithNotification(), CreateOrUpdateCloseButtonView() is
> called twice: in MessageCenterView::UpdateWithNotification() and here.
> 
> CreateOrUpdateViews() is called from
NotificationView::UpdateWithNotification()
> and the ctor. Can we remove this calling CreateOrUpdateCloseButtonView() from
> here and move it to the ctor?
> So that CreateOrUpdateCloseButtonView() is not called twice.

Done.

[yoshiki, 2016-05-16 16:36:23]

lgtm

[xiyuan, 2016-05-16 16:38:26]

Thank you for the quick review.

[xiyuan, 2016-05-16 16:38:32]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2016-05-16 16:38:33]

The patchset sent to the CQ was uploaded after l-g-t-m from dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/1980753002/#ps40001
(title: "move CreateOrUpdateCloseButtonView")

[commit-bot: I haz the power, 2016-05-16 16:39:03]

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

[commit-bot: I haz the power, 2016-05-16 16:59:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-16 17:01:32]

Description was changed from

==========
Move close button to MessageView

So that it is shared with derived classes.

BUG=610767
==========

to

==========
Move close button to MessageView

So that it is shared with derived classes.

BUG=610767

Committed: https://crrev.com/045c33eb3e2647417ff8c5d8d42c67044b25f7d6
Cr-Commit-Position: refs/heads/master@{#393849}
==========

[commit-bot: I haz the power, 2016-05-16 17:01:35]

Patchset 3 (id:??) landed as
https://crrev.com/045c33eb3e2647417ff8c5d8d42c67044b25f7d6
Cr-Commit-Position: refs/heads/master@{#393849}