Update Mac notification tray behavior.

Update Mac notification tray behavior.

Now there are 8 icon resources instead of 2.  We will display different
icons based on 3 binary attributes:
* unread notifications
* do not disturb mode
* pressed status (message center open/closed)
The icon is now a bell (instead of a chat bubble), and we will no longer
display unread count with the notification tray icon.

Additionally, this patch causes the icon not to be updated while the
message center is open, and causes the icon not to be removed after the
first notification comes in (to make settings more accessible.)

BUG=264798, 264971
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214932

[Nico, 2013-07-31 00:05:25]

You probably want to land the new binary files in a separate CL, else the CQ
won't be able to process the patch as far as I know.

[dewittj, 2013-07-31 00:08:04]

Neither will the trybots run.  Thanks, I'll update this one to only include
code.

[Nico, 2013-07-31 00:09:41]

This also removes the unread count from the icon, right? Should probably be
mentioned in the Cl descriptoin.

https://codereview.chromium.org/21308002/diff/18001/chrome/browser/ui/cocoa/n...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm
(right):

https://codereview.chromium.org/21308002/diff/18001/chrome/browser/ui/cocoa/n...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm:89: // Only
show the status item if there are notifications.
remove comment

https://codereview.chromium.org/21308002/diff/18001/ui/message_center/cocoa/s...
File ui/message_center/cocoa/status_item_view.mm (right):

https://codereview.chromium.org/21308002/diff/18001/ui/message_center/cocoa/s...
ui/message_center/cocoa/status_item_view.mm:120: if (highlight) {
This whole block looks like something that would look better if pulled out into
its own function. Two advantages:

1. All lines in a function should have roughly the same level of abstraction.
This function's level of abstraction is "get data, draw stuff".

2. You could use early returns to reduce nesting.

[dewittj, 2013-07-31 00:37:09]

https://codereview.chromium.org/21308002/diff/18001/chrome/browser/ui/cocoa/n...
File chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm
(right):

https://codereview.chromium.org/21308002/diff/18001/chrome/browser/ui/cocoa/n...
chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm:89: // Only
show the status item if there are notifications.
On 2013/07/31 00:09:41, Nico wrote:
> remove comment

Done.

https://codereview.chromium.org/21308002/diff/18001/ui/message_center/cocoa/s...
File ui/message_center/cocoa/status_item_view.mm (right):

https://codereview.chromium.org/21308002/diff/18001/ui/message_center/cocoa/s...
ui/message_center/cocoa/status_item_view.mm:120: if (highlight) {
On 2013/07/31 00:09:41, Nico wrote:
> This whole block looks like something that would look better if pulled out
into
> its own function. Two advantages:
> 
> 1. All lines in a function should have roughly the same level of abstraction.
> This function's level of abstraction is "get data, draw stuff".
> 
> 2. You could use early returns to reduce nesting.

Done.  I also reordered the boolean checks to match with the order of the parts
of the name so that it's easier to see the differences in the names.

[Nico, 2013-07-31 02:40:56]

"This also removes the unread count from the icon, right? Should probably be
mentioned in the Cl description." wasn't addressed.

Assuming it gets addressed, lgtm. One optional and one trivial comment below.

https://codereview.chromium.org/21308002/diff/41001/ui/message_center/cocoa/s...
File ui/message_center/cocoa/status_item_view.mm (right):

https://codereview.chromium.org/21308002/diff/41001/ui/message_center/cocoa/s...
ui/message_center/cocoa/status_item_view.mm:156: BOOL hasUnreadItems =
unreadCount_ > 0;
optional nit: What do you think about making this table driven, like so:

int kResourceIds[2][2][2] = {
  {
    {IDR_TRAY_EMPTY, IDR_TRAY_EMPTY_PRESSED},
    {IDR_TRAY_ATTENTION, IDR_TRAY_ATTENTION_PRESSED},
  },
  {
    {ID_TRAY_DO_NOT_DISTURB_EMPTY, IDR_TRAY_DO_NOT_DISTURB_EMPTY_PRESSED},
    {IDR_TRAY_DO_NOT_DISTURB_ATTENTION,
IDR_TRAY_DO_NOT_DISTURB_ATTENTION_PRESSED},
  },
};
return kResourceIds[quietMode][hasUnreadItems][highlight];

?

https://codereview.chromium.org/21308002/diff/41001/ui/resources/ui_resources...
File ui/resources/ui_resources.grd (right):

https://codereview.chromium.org/21308002/diff/41001/ui/resources/ui_resources...
ui/resources/ui_resources.grd:460: <structure type="chrome_scaled_image"
name="IDR_TRAY_DO_NOT_DISTURB_ATTENTION"
file="mac/notification_tray_do_not_disturb_attention.png"/>
Order these 8 lines alphabetically (DND_ATT should be right before
DND_ATT_PRESSED)

[dewittj, 2013-07-31 21:58:40]

The message had in fact been updated, it's just buried in the middle that we're
not showing the number anymore.

https://codereview.chromium.org/21308002/diff/41001/ui/message_center/cocoa/s...
File ui/message_center/cocoa/status_item_view.mm (right):

https://codereview.chromium.org/21308002/diff/41001/ui/message_center/cocoa/s...
ui/message_center/cocoa/status_item_view.mm:156: BOOL hasUnreadItems =
unreadCount_ > 0;
On 2013/07/31 02:40:56, Nico wrote:
> optional nit: What do you think about making this table driven, like so:
> 
> int kResourceIds[2][2][2] = {
>   {
>     {IDR_TRAY_EMPTY, IDR_TRAY_EMPTY_PRESSED},
>     {IDR_TRAY_ATTENTION, IDR_TRAY_ATTENTION_PRESSED},
>   },
>   {
>     {ID_TRAY_DO_NOT_DISTURB_EMPTY, IDR_TRAY_DO_NOT_DISTURB_EMPTY_PRESSED},
>     {IDR_TRAY_DO_NOT_DISTURB_ATTENTION,
> IDR_TRAY_DO_NOT_DISTURB_ATTENTION_PRESSED},
>   },
> };
> return kResourceIds[quietMode][hasUnreadItems][highlight];
> 
> ?

Done.

https://codereview.chromium.org/21308002/diff/41001/ui/resources/ui_resources...
File ui/resources/ui_resources.grd (right):

https://codereview.chromium.org/21308002/diff/41001/ui/resources/ui_resources...
ui/resources/ui_resources.grd:460: <structure type="chrome_scaled_image"
name="IDR_TRAY_DO_NOT_DISTURB_ATTENTION"
file="mac/notification_tray_do_not_disturb_attention.png"/>
On 2013/07/31 02:40:56, Nico wrote:
> Order these 8 lines alphabetically (DND_ATT should be right before
> DND_ATT_PRESSED)

Done.

[commit-bot: I haz the power, 2013-07-31 22:10:36]

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

[commit-bot: I haz the power, 2013-07-31 23:15:32]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-01 00:51:18]

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

[commit-bot: I haz the power, 2013-08-01 02:50:50]

Change committed as 214932