Issue 15582004: Move NotificationDelegate into message_center.

Issue 15582004: Move NotificationDelegate into message_center. (Closed)

Created:
7 years, 7 months ago by dewittj

Modified:
7 years, 6 months ago

Reviewers:
James Cook, Jun Mukai, Dmitry Titov, commit-bot: I haz the power

CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, derat+watch_chromium.org, ben+watch_chromium.org, tfarina, yoshiki+watch_chromium.org, Aaron Boodman, rginda+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Move NotificationDelegate into message_center. This splits NotificationDelegate into two interfaces: * message_center::NotificationDelegate, and * ::NotificationDelegate. message_center::NotificationDelegate handles click events, whereas ::NotificationDelegate has a RenderViewHost that can be used to download images. This will support adding message_center::Notification objects directly, including all the binary bits of their image notifications. BUG=174164 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202591

Patch Set 1 #

Patch Set 2 : Fix for real. #

Patch Set 3 : Fix mac tests #

Patch Set 4 : Some more cosmetic fixes. #

Total comments: 2

Patch Set 5 : Fix Win browsertests. #

Total comments: 3

Patch Set 6 : Copy delegate in notification update too. #

Total comments: 4

Patch Set 7 : Address mukai@ and dimich@ comments. #

Patch Set 8 : Mac unittest fixes. #

Patch Set 9 : Fix the ever-changing Mac unit tests. #

Created: 7 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+358 lines, -293 lines)			Patch
M	ash/shell/window_type_launcher.cc	View	1 2 3 4 5	1 chunk	+10 lines, -9 lines	0 comments	Download
M	ash/system/web_notification/web_notification_tray_unittest.cc	View	1 2 3 4 5 6	1 chunk	+5 lines, -2 lines	0 comments	Download
M	chrome/browser/notifications/message_center_notification_manager.h	View	1 2 3 4 5 6	1 chunk	+0 lines, -8 lines	0 comments	Download
M	chrome/browser/notifications/message_center_notification_manager.cc	View	1 2 3 4 5 6	4 chunks	+4 lines, -34 lines	0 comments	Download
M	chrome/browser/notifications/notification.h	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/notifications/notification.cc	View	1 2 3 4 5	2 chunks	+2 lines, -4 lines	0 comments	Download
M	chrome/browser/notifications/notification_delegate.h	View	1 2 3 4 5	4 chunks	+3 lines, -28 lines	0 comments	Download
M	chrome/browser/notifications/notification_delegate.cc	View	1 2 3 4 5	1 chunk	+3 lines, -13 lines	0 comments	Download
M	chrome/browser/ui/views/message_center/web_notification_tray_win_browsertest.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	ui/message_center/cocoa/notification_controller_unittest.mm	View	1 2 3 4 5 6 7	6 chunks	+7 lines, -1 line	0 comments	Download
M	ui/message_center/cocoa/popup_collection_unittest.mm	View	1 2 3 4 5 6 7 8	8 chunks	+9 lines, -1 line	0 comments	Download
M	ui/message_center/cocoa/popup_controller_unittest.mm	View	1 2 3 4 5 6 7 8	1 chunk	+1 line, -0 lines	0 comments	Download
M	ui/message_center/cocoa/tray_view_controller_unittest.mm	View	1 2 3 4 5 6 7 8	6 chunks	+6 lines, -0 lines	0 comments	Download
M	ui/message_center/fake_message_center.h	View	1 2 3 4 5	2 chunks	+16 lines, -14 lines	0 comments	Download
M	ui/message_center/fake_message_center.cc	View	1 2 3 4 5	1 chunk	+4 lines, -4 lines	0 comments	Download
M	ui/message_center/message_center.h	View	1 2 3 4 5 6	2 chunks	+16 lines, -18 lines	0 comments	Download
M	ui/message_center/message_center.gyp	View	1 2 3 4 5	1 chunk	+2 lines, -0 lines	0 comments	Download
M	ui/message_center/message_center_impl.h	View	1 2 3 4 5	2 chunks	+16 lines, -14 lines	0 comments	Download
M	ui/message_center/message_center_impl.cc	View	1 2 3 4 5 6	6 chunks	+35 lines, -12 lines	0 comments	Download
M	ui/message_center/message_center_tray_unittest.cc	View		4 chunks	+32 lines, -32 lines	0 comments	Download
M	ui/message_center/notification.h	View		4 chunks	+8 lines, -1 line	0 comments	Download
M	ui/message_center/notification.cc	View	1 2 3 4 5	2 chunks	+18 lines, -14 lines	0 comments	Download
A +	ui/message_center/notification_delegate.h	View	1 2 3 4 5	3 chunks	+12 lines, -19 lines	0 comments	Download
A	ui/message_center/notification_delegate.cc	View	1 2 3 4 5	1 chunk	+13 lines, -0 lines	0 comments	Download
M	ui/message_center/notification_list.h	View	1 2 3 4 5	3 chunks	+9 lines, -2 lines	0 comments	Download
M	ui/message_center/notification_list.cc	View	1 2 3 4 5	4 chunks	+28 lines, -13 lines	0 comments	Download
M	ui/message_center/notification_list_unittest.cc	View	1 2 3 4 5	7 chunks	+81 lines, -36 lines	0 comments	Download
M	ui/message_center/views/message_center_view_unittest.cc	View		1 chunk	+8 lines, -8 lines	0 comments	Download
M	ui/message_center/views/message_popup_collection_unittest.cc	View		1 chunk	+8 lines, -4 lines	0 comments	Download

Messages

Total messages: 15 (0 generated)

Expand Messages | Collapse Messages

Jun Mukai

https://codereview.chromium.org/15582004/diff/15001/ui/message_center/message_center.h File ui/message_center/message_center.h (right): https://codereview.chromium.org/15582004/diff/15001/ui/message_center/message_center.h#newcode94 ui/message_center/message_center.h:94: virtual void AddNotification(NotificationType type, Can we just receive message_center::Notification* ...

7 years, 7 months ago (2013-05-22 23:08:30 UTC) #3

dewittj

I added a delegate parameter to the Update call. https://codereview.chromium.org/15582004/diff/15001/ui/message_center/message_center.h File ui/message_center/message_center.h (right): https://codereview.chromium.org/15582004/diff/15001/ui/message_center/message_center.h#newcode94 ui/message_center/message_center.h:94: ...

7 years, 7 months ago (2013-05-24 01:03:50 UTC) #4

Dmitry Titov

lgtm with a nit: https://codereview.chromium.org/15582004/diff/16027/ui/message_center/message_center_impl.cc File ui/message_center/message_center_impl.cc (right): https://codereview.chromium.org/15582004/diff/16027/ui/message_center/message_center_impl.cc#newcode257 ui/message_center/message_center_impl.cc:257: FOR_EACH_OBSERVER(MessageCenterObserver, you use different styles ...

7 years, 7 months ago (2013-05-24 01:11:57 UTC) #5

Jun Mukai

looks good, and I noticed that the code around my recent patch for HasClickedHandler could ...

7 years, 7 months ago (2013-05-24 01:29:19 UTC) #6

dewittj

mukai, dimich PTAL https://codereview.chromium.org/15582004/diff/21001/chrome/browser/notifications/message_center_notification_manager.h File chrome/browser/notifications/message_center_notification_manager.h (right): https://codereview.chromium.org/15582004/diff/21001/chrome/browser/notifications/message_center_notification_manager.h#newcode53 chrome/browser/notifications/message_center_notification_manager.h:53: virtual bool HasClickedListener(const std::string& notification_id) OVERRIDE; ...

7 years, 7 months ago (2013-05-24 18:00:59 UTC) #8