[Mac] App Menu Animated Icon

[Mac] App Menu Animated Icon

Implementation of the new app menu animated icon.
The new icon is set behind the "new-app-menu-icon" flag.
The animation is triggered when a new window/tab is created or
when the menu is opened.

BUG=704786

[spqchan, 2017-05-03 20:31:55]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 20:33:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-05-03 21:07:00]

Description was changed from

==========
[Mac] New App Menu Icon

BUG=
==========

to

==========
[Mac] App Menu Animated Icon

Implementation of the new app menu animated icon.
The new icon is set behind the "new-app-menu-icon" flag.
The animation is triggered when a new window/tab is created or
when the menu is opened.

BUG=704786
==========

[spqchan, 2017-05-03 21:07:00]

spqchan@chromium.org changed reviewers:
+ rsesek@chromium.org

[spqchan, 2017-05-03 21:08:50]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 21:09:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-05-03 21:22:57]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[spqchan, 2017-05-03 21:23:01]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-05-03 21:23:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[spqchan, 2017-05-03 21:28:20]

PTAL

This is for the new app menu icon. The one we currently have changes the icon
according to the browser's update status, which confuses a lot of users.

This is what the spec looks like:
https://drive.google.com/file/d/0B7C_rrbeaTQOUXNnNWV1SVY3b1E/view

Let me know if you have any questions!

[commit-bot: I haz the power, 2017-05-03 22:30:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-03 22:30:39]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-05-03 22:44:29]

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h (right):

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h:12: #import
"base/mac/scoped_nsobject.h"
Unused?

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h:25:
std::unique_ptr<AppToolbarButtonIcon> app_icon_;
naming: appIcon

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm (right):

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm:122: -
(void)onTabInsertedInForeground {
What do you think of this?

Rename this -maybeStartAnimation and call it directly from
BrowserWindowController, and have -willShowMenu in this class call it too. It
seems a little weird to have the button care about tabs being inserted, but I
can also see why you did it this way.

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/toolbar/app_toolbar_button_icon.mm (right):

https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/toolbar/app_toolbar_button_icon.mm:1: // Copyright 2017
The Chromium Authors. All rights reserved.
Are other platforms going to use the same spec? Should this code be shared (%
the Cocoa bits)?

[spqchan, 2017-05-04 02:29:41]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 02:29:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 02:39:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 02:39:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[spqchan, 2017-05-04 17:16:13]

Patchset #3 (id:60001) has been deleted

[spqchan, 2017-05-04 17:16:18]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 17:16:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 17:33:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 17:33:08]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[spqchan, 2017-05-04 19:02:55]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[spqchan, 2017-05-04 19:02:59]

Patchset #3 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-05-04 19:03:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 20:12:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 20:12:10]

Dry run: This issue passed the CQ dry run.

[spqchan, 2017-05-05 21:06:56]

The CQ bit was checked by spqchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 21:07:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 23:05:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 23:05:30]

Dry run: This issue passed the CQ dry run.

[spqchan, 2017-05-08 16:41:51]

On 2017/05/03 22:44:29, Robert Sesek wrote:
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h (right):
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h:12: #import
> "base/mac/scoped_nsobject.h"
> Unused?
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/toolbar/app_toolbar_button.h:25:
> std::unique_ptr<AppToolbarButtonIcon> app_icon_;
> naming: appIcon
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm (right):
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/toolbar/app_toolbar_button.mm:122: -
> (void)onTabInsertedInForeground {
> What do you think of this?
> 
> Rename this -maybeStartAnimation and call it directly from
> BrowserWindowController, and have -willShowMenu in this class call it too. It
> seems a little weird to have the button care about tabs being inserted, but I
> can also see why you did it this way.
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/toolbar/app_toolbar_button_icon.mm (right):
> 
>
https://codereview.chromium.org/2859903003/diff/40001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/toolbar/app_toolbar_button_icon.mm:1: // Copyright
2017
> The Chromium Authors. All rights reserved.
> Are other platforms going to use the same spec? Should this code be shared (%
> the Cocoa bits)?

Thanks for your help with this! Unfortunately, it looks like I'll have to get
rid of this since estade@ is writing a framework for animated icons that I could
use instead.