Fix transparent avatar icons.

Fix transparent avatar icons.
My previous CL https://codereview.chromium.org/678553002/ did not handle this
case correctly.

BUG=412383
Committed: https://crrev.com/153eb8a7cf3e15017d6820889c246259a6325b07
Cr-Commit-Position: refs/heads/master@{#302443}

[Roger Tawa OOO till Jul 10th, 2014-10-30 02:04:05]

Patchset #1 (id:1) has been deleted

[Roger Tawa OOO till Jul 10th, 2014-10-30 17:24:58]

rogerta@chromium.org changed reviewers:
+ noms@chromium.org

[Roger Tawa OOO till Jul 10th, 2014-10-30 17:24:59]

Hi Monica,

Please take a look, this fixes a transparent icons problem with my previous CL. 
I can move the helper function out of avatar_menu_button.cc if you like. 
Thanks.

[noms (inactive), 2014-10-30 17:39:08]

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_shortcut_manager_win.cc (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:77: // in
profile_avatar_icon_util.cc
nit: needs to end in a .

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:882:
ResourceBundle::GetSharedInstance().GetNativeImageNamed(
Maybe you get pull the ResourceBundle instance out in a variable, so that this
fits in one line?

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_shortcut_manager_win.h (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.h:87: // badging.  These
icons will have correct transparency.
I would maybe change the "correct transparency" bit to explain that this will
use the normal resources bundled with chrome, and not the high-res avatars that
are downloaded.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profiles/avatar_menu_button.cc (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/avatar_menu_button.cc:99: void
AvatarMenuButton::GetAvatarImages(Profile* profile,
I wonder if this belongs in profile_avatar_icon_util.cc. Maybe called
GetAvatarImageForBadging or something like that? It's weird because
avatar_button is the *old*, pre-mirror avatar button, which in the future will
basically only used for OTR windows (so it really shouldn't care about badging
the taskbar)

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/avatar_menu_button.cc:114:
ProfileShortcutManagerWin::GetBadgingAvatarImages(
I don't think this should do different things based on whether the flag is
turned on (the taskbar badge is the same in both cases). I think
AvatarMenu::GetImageForMenuButton should always return the right thing (i.e.
always ignore the high res resource).

(here and below)

[Roger Tawa OOO till Jul 10th, 2014-10-31 19:44:39]

Thanks Monica.  Much simpler and smaller CL now :-)  Since this is candidate for
merge to M39, maybe keep the code where it is as mentioned below.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_shortcut_manager_win.cc (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:77: // in
profile_avatar_icon_util.cc
On 2014/10/30 17:39:08, Monica Dinculescu wrote:
> nit: needs to end in a .

Removed this code from CL.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:882:
ResourceBundle::GetSharedInstance().GetNativeImageNamed(
On 2014/10/30 17:39:08, Monica Dinculescu wrote:
> Maybe you get pull the ResourceBundle instance out in a variable, so that this
> fits in one line?

Removed this code from CL.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_shortcut_manager_win.h (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.h:87: // badging.  These
icons will have correct transparency.
On 2014/10/30 17:39:08, Monica Dinculescu wrote:
> I would maybe change the "correct transparency" bit to explain that this will
> use the normal resources bundled with chrome, and not the high-res avatars
that
> are downloaded.

Removed this code from CL.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profiles/avatar_menu_button.cc (right):

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/avatar_menu_button.cc:99: void
AvatarMenuButton::GetAvatarImages(Profile* profile,
On 2014/10/30 17:39:08, Monica Dinculescu wrote:
> I wonder if this belongs in profile_avatar_icon_util.cc. Maybe called
> GetAvatarImageForBadging or something like that? It's weird because
> avatar_button is the *old*, pre-mirror avatar button, which in the future will
> basically only used for OTR windows (so it really shouldn't care about badging
> the taskbar) 

The OTR windows *do* care about badging.  OTR windows are supposed to be badged
with the profile's avatar.  Maybe we can leave this function here for now, as
discussed, and then figure out a better place once we remove all support for the
old avatar menu.

https://codereview.chromium.org/686353002/diff/60001/chrome/browser/ui/views/...
chrome/browser/ui/views/profiles/avatar_menu_button.cc:114:
ProfileShortcutManagerWin::GetBadgingAvatarImages(
On 2014/10/30 17:39:08, Monica Dinculescu wrote:
> I don't think this should do different things based on whether the flag is
> turned on (the taskbar badge is the same in both cases). I think
> AvatarMenu::GetImageForMenuButton should always return the right thing (i.e.
> always ignore the high res resource).
> 
> (here and below)

Done.

[noms (inactive), 2014-10-31 20:14:29]

lgtm

[Roger Tawa OOO till Jul 10th, 2014-11-03 15:09:02]

rogerta@chromium.org changed reviewers:
+ sky@chromium.org

[Roger Tawa OOO till Jul 10th, 2014-11-03 15:09:03]

Hi Scott,

Please take a look.  Thanks.

[sky, 2014-11-03 16:38:56]

LGTM

[Roger Tawa OOO till Jul 10th, 2014-11-03 16:40:52]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2014-11-03 16:41:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/686353002/120001

[commit-bot: I haz the power, 2014-11-03 17:24:23]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2014-11-03 17:25:08]

Patchset 6 (id:??) landed as
https://crrev.com/153eb8a7cf3e15017d6820889c246259a6325b07
Cr-Commit-Position: refs/heads/master@{#302443}