Move knowledge about MessageCenter out of MessageView.

Move knowledge about MessageCenter out of MessageView.

This is because I need to create a new subclass of MessageView - a synthetic placeholder for multiple notifications from the same source. Hence the new enum, MesageViewTypes, which is not used in this patch but will in second... the derived classes override abstract methods of the MesageView to provide functionality.

NotificationView currently knows about MessageCenter but in the future refactorings it can yield this to MessageCenterView.

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

[Dmitry Titov, 2013-11-18 23:33:46]

Fixed build break

[dewittj, 2013-11-19 01:44:04]

https://codereview.chromium.org/75133006/diff/80001/ui/message_center/views/m...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/75133006/diff/80001/ui/message_center/views/m...
ui/message_center/views/message_center_view.cc:893:
static_cast<NotificationView*>(message_view)->notification_id() == id) {
static_cast? Any way to get around this?

[Dmitry Titov, 2013-11-20 22:13:16]

On 2013/11/19 01:44:04, dewittj wrote:
>
https://codereview.chromium.org/75133006/diff/80001/ui/message_center/views/m...
> File ui/message_center/views/message_center_view.cc (right):
> 
>
https://codereview.chromium.org/75133006/diff/80001/ui/message_center/views/m...
> ui/message_center/views/message_center_view.cc:893:
> static_cast<NotificationView*>(message_view)->notification_id() == id) {
> static_cast? Any way to get around this?

See updated patch.

The idea is that I'll keep various subtypes of MessageView in separate
lists/sets because I usually want to do different operations with them anyways.
This also removes the assumption that a vector of MessageViews is in sync with a
set of MessageListView children. So I could use a map by notification id which
is also nice.

There is still a cast in MessageCenterView::OnNotificationRemoved. I think it's
ok because all views hosted in
MessageLustView are MessageViews. This focus code does not work btw but fixing
it is outside of the scope of this patch...

[dewittj, 2013-11-22 00:33:55]

lgtm

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.cc:910: static_cast<MessageView*>(
Can we document somewhere that this depends on all child views being
MessageViews?

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.cc:929: // TODO(dimich): add
MessageCenter::GetNotificationById(id)
GetVisibleNotificationById?

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
File ui/message_center/views/message_center_view.h (right):

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.h:94: NotificationViewsMap
notification_views_;  // Weak.
The comment here is confusing since notification_views_ is not a pointer.

[Dmitry Titov, 2013-11-22 01:13:51]

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.cc:910: static_cast<MessageView*>(
On 2013/11/22 00:33:55, dewittj wrote:
> Can we document somewhere that this depends on all child views being
> MessageViews?

I've changed the methods on MessageListView to accept only MessageView instead
of View, added couple of comments.

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.cc:929: // TODO(dimich): add
MessageCenter::GetNotificationById(id)
On 2013/11/22 00:33:55, dewittj wrote:
> GetVisibleNotificationById?

Done.

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
File ui/message_center/views/message_center_view.h (right):

https://codereview.chromium.org/75133006/diff/190001/ui/message_center/views/...
ui/message_center/views/message_center_view.h:94: NotificationViewsMap
notification_views_;  // Weak.
On 2013/11/22 00:33:55, dewittj wrote:
> The comment here is confusing since notification_views_ is not a pointer.

Done.

[commit-bot: I haz the power, 2013-11-22 01:21:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dimich@chromium.org/75133006/290001

[commit-bot: I haz the power, 2013-11-22 02:15:45]

Retried try job too often on linux_aura for step(s) app_list_unittests,
aura_unittests, browser_tests, compositor_unittests, content_browsertests,
content_unittests, interactive_ui_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-22 19:02:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dimich@chromium.org/75133006/560001

[commit-bot: I haz the power, 2013-11-22 19:52:59]

Retried try job too often on linux_chromeos_clang for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-11-22 22:49:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dimich@chromium.org/75133006/850001

[commit-bot: I haz the power, 2013-11-23 00:59:48]

Change committed as 236888