Re-factor Ash Message Center code part 4/4

Re-factor Ash Message Center code part 4/4
Extracts WebNotificationList management code to MessageCenter class.

After this the bulk of code can be moved to Views!

Updated the diagram:
https://docs.google.com/drawings/d/1B6ZxXObmrDFzgWEHmkP_Ppt2BFbOL9kXzakOmY0sXe0/edit


BUG=150872
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163409

[stevenjb, 2012-10-19 20:30:02]

Last one (other than moving the code)!

[miket_OOO, 2012-10-19 20:45:25]

Great! LGTM.

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
File ash/system/web_notification/message_center.cc (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
ash/system/web_notification/message_center.cc:48: const string16& message,
fix ze indent

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
File ash/system/web_notification/message_center.h (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
ash/system/web_notification/message_center.h:76: // Add a new notification. |id|
is a unique identifier, used to update or
I think these should be 3d-person singular ("Adds...")

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/we...
File ash/system/web_notification/web_notification_tray.cc (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/we...
ash/system/web_notification/web_notification_tray.cc:91:
message_center_.reset(new message_center::MessageCenter(this));
Not your change, but I suppose you could move this to the initializer with an
ALLOW_THIS_IN_INITIALIZER. Unless MessageCenter depends on the object being in a
certain state.

[stevenjb, 2012-10-19 21:47:53]

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
File ash/system/web_notification/message_center.cc (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
ash/system/web_notification/message_center.cc:48: const string16& message,
On 2012/10/19 20:45:26, miket wrote:
> fix ze indent

Done.

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
File ash/system/web_notification/message_center.h (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/me...
ash/system/web_notification/message_center.h:76: // Add a new notification. |id|
is a unique identifier, used to update or
On 2012/10/19 20:45:26, miket wrote:
> I think these should be 3d-person singular ("Adds...")
Yep. Done.

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/we...
File ash/system/web_notification/web_notification_tray.cc (right):

http://codereview.chromium.org/11189099/diff/1/ash/system/web_notification/we...
ash/system/web_notification/web_notification_tray.cc:91:
message_center_.reset(new message_center::MessageCenter(this));
On 2012/10/19 20:45:26, miket wrote:
> Not your change, but I suppose you could move this to the initializer with an
> ALLOW_THIS_IN_INITIALIZER. Unless MessageCenter depends on the object being in
a
> certain state.
The same is true with button_, but I personally prefer to put non trivial
construction in the body. I don't think the style guide dictates either way.

[stevenjb, 2012-10-19 21:51:37]

ben@ - Need an OWNERS review for chrome/browser/ui/views/ash/

[Ben Goodger (Google), 2012-10-22 17:43:35]

lgtm

[commit-bot: I haz the power, 2012-10-22 19:46:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/11189099/13001

[jennyz, 2012-10-22 19:50:13]

lgtm

[commit-bot: I haz the power, 2012-10-22 22:26:34]

Change committed as 163409

[Ben Goodger (Google), 2012-10-25 17:23:55]

lgtm