Notifications: Add cross-platform UMA: ShowMessageCenter, ShowSettings.

Notifications: Add cross-platform UMA: ShowMessageCenter, ShowSettings.

This also cleans up some of the visibility code in Message
Center.

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

[dewittj, 2013-09-09 16:06:10]

ptal mukai

FYI This change makes me think that we should work towards removing
MessageCenterTray altogether when possible.  Too much for just this change,
though.

[Jun Mukai, 2013-09-09 17:12:35]

Please also update ash/web_notifications/web_notification_tray.* and possibly
you may want to check
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge*

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
File chrome/browser/ui/views/message_center/web_notification_tray.cc (right):

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
chrome/browser/ui/views/message_center/web_notification_tray.cc:168: return
true;
It seems that ShowMessageCenter / ShowNotifierSettings only return true at any
case. Which platform tray will return false under what condition?
Can you check it quickly?  And if all of them return true, please remove the
bool.

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
chrome/browser/ui/views/message_center/web_notification_tray.cc:187: true,  //
settings initally invisible
s/invisible/visible/ ?

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
File ui/message_center/message_center.h (right):

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center.h:35: enum Visibility {
It would be better to declare this enum in another file, so its user just
include that simple file.
It would be message_center_style.h, or _util.h, or add new _types.h file (ash
uses _style.h file, like ash/shelf/shelf_types.h)

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
File ui/message_center/message_center_tray.cc (right):

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center_tray.cc:85: }
nit: no braces for single line.

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center_tray.cc:96:
message_center_->SetVisibility(message_center::VISIBILITY_TRANSIENT);
why need to specify TRANSIENT here?

[dewittj, 2013-09-10 21:23:24]

I don't think that ash../message_center_tray needs update, it was never using
content::RecordAction.

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
File chrome/browser/ui/views/message_center/web_notification_tray.cc (right):

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
chrome/browser/ui/views/message_center/web_notification_tray.cc:168: return
true;
On 2013/09/09 17:12:35, Jun Mukai wrote:
> It seems that ShowMessageCenter / ShowNotifierSettings only return true at any
> case. Which platform tray will return false under what condition?
> Can you check it quickly?  And if all of them return true, please remove the
> bool.

ShowMessageCenter returns false on ChromeOS if !ShouldShowMessageCenter.

https://codereview.chromium.org/23636010/diff/34001/chrome/browser/ui/views/m...
chrome/browser/ui/views/message_center/web_notification_tray.cc:187: true,  //
settings initally invisible
On 2013/09/09 17:12:35, Jun Mukai wrote:
> s/invisible/visible/ ?

Done.

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
File ui/message_center/message_center.h (right):

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center.h:35: enum Visibility {
On 2013/09/09 17:12:35, Jun Mukai wrote:
> It would be better to declare this enum in another file, so its user just
> include that simple file.
> It would be message_center_style.h, or _util.h, or add new _types.h file (ash
> uses _style.h file, like ash/shelf/shelf_types.h)

Done.

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
File ui/message_center/message_center_tray.cc (right):

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center_tray.cc:85: }
On 2013/09/09 17:12:35, Jun Mukai wrote:
> nit: no braces for single line.

Done.

https://codereview.chromium.org/23636010/diff/34001/ui/message_center/message...
ui/message_center/message_center_tray.cc:96:
message_center_->SetVisibility(message_center::VISIBILITY_TRANSIENT);
On 2013/09/09 17:12:35, Jun Mukai wrote:
> why need to specify TRANSIENT here?

Done.

[Jun Mukai, 2013-09-10 21:35:41]

lgtm

[commit-bot: I haz the power, 2013-09-10 22:15:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/23636010/48001

[commit-bot: I haz the power, 2013-09-11 13:48:44]

Change committed as 222536