Newly created profiles should have the grey silhouette as avatar.

Newly created profiles should have the grey silhouette as avatar.
The name should still be automatically generated.

Also, all the avatar icons are becoming squares (31x38 -> 38x38). New resources will be committed and necessary code changes have been made.


BUG=337986
TEST=When creating a new profile from anywhere except the settings. Also, verify avatar icons are still rendered correctly, whether GAIA, the placeholder or a cartoon icon.
page, the automatically assigned avatar should the grey
placeholder (silhouette).

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

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

[Mike Lerman, 2014-03-27 19:58:05]

Hello,

I have modified the default avatar icon when you create a new profile to be the
grey silhouette icon. I also removed the multiple references to the resource PNG
at common/login.guest which involved changing references in the UI layers.

Can you please review? Thanks!

+ noms: c/b/profiles, c/b/resources/login/*.js
+ oshima: c/app/*, build/ios
+ shess: c/b/ui/cocoa
+ msw: c/b/ui/views

[Scott Hess - ex-Googler, 2014-03-27 20:11:09]

lgtm for cocoa/

[noms (inactive), 2014-03-27 20:19:31]

https://codereview.chromium.org/212603011/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:11732: +      </message>
Hmm, I don't think the name should be "Shady". I know that with the
--new-profile-management flag we'd assign it a Person %n name by default, but I
don't know what should happen without the flag (where this is getting used).
"Guest" is not very good either.  Maybe something along the lines of how
IDS_NUMBERED_PROFILE_NAME is used?

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_info_cache.h (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_info_cache.h:136: static int
GetPlaceholderAvatarIconResourceID();
Why do you need both methods? Isn't  GetPlaceholderAvatarIconResourceID just
GetDefaultAvatarIconResourceIDAtIndex(GetPlaceholderAvatarIndex()) ?

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_window.cc:228: cache.ChooseNameForNewProfile(
make a variable that stores the result of GetPlaceholderAvatarIndex();

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profile_chooser_view.cc (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/profile_chooser_view.cc:858:
ProfileInfoCache::GetPlaceholderAvatarIconResourceID());
nit: indent by 2 more

[noms (inactive), 2014-03-27 20:39:53]

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

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:127: if
(avatar_bitmap.width() != avatar_bitmap.height()) {
nit: no brackets.
Also: as discussed offline, I don't think this works for the placeholder avatar,
as I think we should execute the code above in that case as well.

[oshima, 2014-03-27 23:00:27]

Sounds like CL is not ready yet? I'm holding this off for the time being. Ping
me when it's ready.

[Mike Lerman, 2014-04-02 17:46:25]

https://codereview.chromium.org/212603011/diff/40001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/app/generated_res...
chrome/app/generated_resources.grd:11732: +      </message>
I've changed the name in the .grd file to Shadow.
However, when the placeholder is used because a new profile was added via the
User Manager, a new name is chosen the same way the IDS_NUMBERED_PROFILE_NAME is
used. So Shadow would only appear when the icon is the one selected when the Add
New User dialog opens on the chrome://settings page.

On 2014/03/27 20:19:32, Monica Dinculescu wrote:
> Hmm, I don't think the name should be "Shady". I know that with the
> --new-profile-management flag we'd assign it a Person %n name by default, but
I
> don't know what should happen without the flag (where this is getting used).
> "Guest" is not very good either.  Maybe something along the lines of how
> IDS_NUMBERED_PROFILE_NAME is used?

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_info_cache.h (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_info_cache.h:136: static int
GetPlaceholderAvatarIconResourceID();
Yes, but this removes the need to have those two nested calls in the various
places I need to call them together (which happens multiple times). Should I
remove the helper method?

On 2014/03/27 20:19:32, Monica Dinculescu wrote:
> Why do you need both methods? Isn't  GetPlaceholderAvatarIconResourceID just
> GetDefaultAvatarIconResourceIDAtIndex(GetPlaceholderAvatarIndex()) ?

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

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_shortcut_manager_win.cc:127: if
(avatar_bitmap.width() != avatar_bitmap.height()) {
Nit - Done.

Offline discussion - the problem we found is caused elsewhere
(ProfileListDesktop::RebuildMenu). I will check a solution in with this CL.

On 2014/03/27 20:39:53, Monica Dinculescu wrote:
> nit: no brackets.
> Also: as discussed offline, I don't think this works for the placeholder
avatar,
> as I think we should execute the code above in that case as well.

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_window.cc (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profile_window.cc:228: cache.ChooseNameForNewProfile(
On 2014/03/27 20:19:32, Monica Dinculescu wrote:
> make a variable that stores the result of GetPlaceholderAvatarIndex();

Done.

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/profile_chooser_view.cc (right):

https://codereview.chromium.org/212603011/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/profile_chooser_view.cc:858:
ProfileInfoCache::GetPlaceholderAvatarIconResourceID());
On 2014/03/27 20:19:32, Monica Dinculescu wrote:
> nit: indent by 2 more

Done.

[Mike Lerman, 2014-04-04 20:24:33]

For reviewers,

Please note that the .png files will be landed separately.

Oshima, MSW, this is now ready for your review.

noms - c/b/ui/webui/signin/ also
+jhawkins - c/b/resources/options/
+pkasting - c/b/ui/views/frame/

Thanks,
Mike

[Peter Kasting, 2014-04-04 20:27:15]

LGTM

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/taskbar_decorator_win.cc (right):

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/taskbar_decorator_win.cc:29: const size_t
kOverlayIconSize = 16;
Nit: Leave this declared in the narrowest possible scope, i.e. inside the
conditional inside the function.

[msw, 2014-04-04 20:52:48]

c/b/ui/views lgtm with a nit

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:585: int
BrowserView::GetGuestIconResourceID() const {
nit: remove this function, inline the contents at its one call site.

[noms (inactive), 2014-04-04 20:54:52]

lgtm for profiles/signin, with the same nit as msw :)

[noms (inactive), 2014-04-04 20:54:53]

lgtm for profiles/signin, with the same nit as msw :)

[Mike Lerman, 2014-04-04 21:09:07]

Nits Done.

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:585: int
BrowserView::GetGuestIconResourceID() const {
On 2014/04/04 20:52:49, msw wrote:
> nit: remove this function, inline the contents at its one call site.

Done.

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/taskbar_decorator_win.cc (right):

https://codereview.chromium.org/212603011/diff/160001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/taskbar_decorator_win.cc:29: const size_t
kOverlayIconSize = 16;
On 2014/04/04 20:27:15, Peter Kasting wrote:
> Nit: Leave this declared in the narrowest possible scope, i.e. inside the
> conditional inside the function.

Done.

[James Hawkins, 2014-04-04 21:52:06]

lgtm

[oshima, 2014-04-09 17:25:13]

c/a/theme lgtm. (sorry for delay)

[Mike Lerman, 2014-04-10 14:58:51]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-10 14:59:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/220001

[commit-bot: I haz the power, 2014-04-10 14:59:14]

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

[commit-bot: I haz the power, 2014-04-10 14:59:15]

Failed to apply patch for
chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
  |diff --git a/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
b/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
  |index
b973b0fbc4494caa3c87d7fb94ad75e56c7bc688..19389ee554340ecf5047cea1afde69b2bf1e04b5
100644
  |--- a/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
  |+++ b/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:       chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
Index: chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
diff --git a/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
b/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
index
b973b0fbc4494caa3c87d7fb94ad75e56c7bc688..19389ee554340ecf5047cea1afde69b2bf1e04b5
100644
--- a/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
+++ b/chrome/browser/ui/cocoa/browser/avatar_icon_controller.mm
@@ -99,8 +99,8 @@ const CGFloat kAvatarLabelRightSpacing = 2;
     Profile* profile = browser_->profile();
 
     if (profile->IsOffTheRecord() || profile->IsGuestSession()) {
-      const int icon_id = profile->IsGuestSession() ? IDR_LOGIN_GUEST :
-                                                      IDR_OTR_ICON;
+      const int icon_id = profile->IsGuestSession() ?
+          ProfileInfoCache::GetPlaceholderAvatarIconResourceID() :
IDR_OTR_ICON;
       NSImage* icon = ResourceBundle::GetSharedInstance().GetNativeImageNamed(
           icon_id).ToNSImage();
       [self setImage:[self compositeImageWithShadow:icon]];

[Mike Lerman, 2014-04-11 13:14:29]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-11 13:18:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/240001

[commit-bot: I haz the power, 2014-04-11 14:13:52]

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

[commit-bot: I haz the power, 2014-04-11 14:13:52]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_compile_dbg

[Mike Lerman, 2014-04-11 14:34:19]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-11 14:34:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/260001

[commit-bot: I haz the power, 2014-04-11 16:51:31]

Change committed as 263262

[viettrungluu, 2014-04-11 18:08:19]

This adds static initializers....

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:220: const char*
kNoHighResAvatar = "NothingToDownload";
const char kNoHighResAvatar[]

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:224: const char*
kDefaultAvatarIconResourceFileNames[] = {
const char* const

[viettrungluu, 2014-04-11 18:12:22]

On 2014/04/11 18:08:19, viettrungluu wrote:
> This adds static initializers....
> 
>
https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
> File chrome/browser/profiles/profile_avatar_icon_util.cc (right):
> 
>
https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
> chrome/browser/profiles/profile_avatar_icon_util.cc:220: const char*
> kNoHighResAvatar = "NothingToDownload";
> const char kNoHighResAvatar[]
> 
>
https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
> chrome/browser/profiles/profile_avatar_icon_util.cc:224: const char*
> kDefaultAvatarIconResourceFileNames[] = {
> const char* const

(Reverted r263284.)

[Mike Lerman, 2014-04-14 18:51:29]

+viettrungluu, I've removed the static initializers. Can you review?

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:220: const char*
kNoHighResAvatar = "NothingToDownload";
On 2014/04/11 18:08:20, viettrungluu wrote:
> const char kNoHighResAvatar[]

Done.

https://codereview.chromium.org/212603011/diff/260001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:224: const char*
kDefaultAvatarIconResourceFileNames[] = {
On 2014/04/11 18:08:20, viettrungluu wrote:
> const char* const

Done.

[viettrungluu, 2014-04-14 20:34:46]

Rubberstamp LGTM, but here's my opinion below....

(You may want to ask your other reviewers what they think.)

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:296: static const int
kDefaultAvatarIconResources[] = {
I wonder if you shouldn't instead declare this as ...
kDefaultAvatarIconResources[kDefaultAvatarIconsCount], which would at least
prevent people from accidentally appending something to this list without
updating the count.

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:333: static const char*
kDefaultAvatarIconResourceFileNames[] = {
1) Note that this is one of the (few) times where something not being as const
as it should be actually hurts you -- since the pointers aren't const (i.e., "*
const" instead of "*"), code probably has to get executed the first time this
function is run (to initialize an array of pointers).

(This also has negative consequences on thread-safety, though I'm guessing this
doesn't matter here.)

2) What I said above about [kDefaultAvatarIconsCount] also applies here.

3) But, possibly, having parallel arrays isn't the best pattern in the world. It
might be better to do something like (at a global scope):

struct ResourceIcon {
  int id;
  const char* filename;
};
const ResourceIcon kDefaultAvatarIcons[] = {
  { IDR_PROFILE_AVATAR_0, "avatar_generic.png" },
  ...
};
const size_t kDefaultAvatarIconsCount = arraysize(kDefaultAvatarIcons);

(I don't think there arraysize() generates a static initializer; arguably, with
only a single array, you could just use arraysize(...) everywhere instead in any
case.)

[Mike Lerman, 2014-04-15 15:15:29]

trungl, I made many of the suggestions you proposed. Thanks again!

noms, can you have a review from the profiles side?

I'll send it into CQ after. Thanks!

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:296: static const int
kDefaultAvatarIconResources[] = {
On 2014/04/14 20:34:47, viettrungluu wrote:
> I wonder if you shouldn't instead declare this as ...
> kDefaultAvatarIconResources[kDefaultAvatarIconsCount], which would at least
> prevent people from accidentally appending something to this list without
> updating the count.

Done.

https://codereview.chromium.org/212603011/diff/280001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:333: static const char*
kDefaultAvatarIconResourceFileNames[] = {
The single array (instead of the parallel ones) is a good suggestion. Thanks!

On 2014/04/14 20:34:47, viettrungluu wrote:
> 1) Note that this is one of the (few) times where something not being as const
> as it should be actually hurts you -- since the pointers aren't const (i.e.,
"*
> const" instead of "*"), code probably has to get executed the first time this
> function is run (to initialize an array of pointers).
> 
> (This also has negative consequences on thread-safety, though I'm guessing
this
> doesn't matter here.)
> 
> 2) What I said above about [kDefaultAvatarIconsCount] also applies here.
> 
> 3) But, possibly, having parallel arrays isn't the best pattern in the world.
It
> might be better to do something like (at a global scope):
> 
> struct ResourceIcon {
>   int id;
>   const char* filename;
> };
> const ResourceIcon kDefaultAvatarIcons[] = {
>   { IDR_PROFILE_AVATAR_0, "avatar_generic.png" },
>   ...
> };
> const size_t kDefaultAvatarIconsCount = arraysize(kDefaultAvatarIcons);
> 
> (I don't think there arraysize() generates a static initializer; arguably,
with
> only a single array, you could just use arraysize(...) everywhere instead in
any
> case.)

[noms (inactive), 2014-04-15 15:26:47]

profiles lgtm with nits

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:216: // GAIA images get a
border.
Nit: I think this fits in one line.

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

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
chrome/browser/profiles/profile_shortcut_manager_win.cc:116: //
DLOG_ASSERT(avatar_bitmap.width() == avatar_bitmap.height());
I think this should be, if anything, a DCHECK, not a DLOG_ASSERT.

[Mike Lerman, 2014-04-15 17:56:45]

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
File chrome/browser/profiles/profile_avatar_icon_util.cc (right):

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
chrome/browser/profiles/profile_avatar_icon_util.cc:216: // GAIA images get a
border.
On 2014/04/15 15:26:48, Monica Dinculescu wrote:
> Nit: I think this fits in one line.

Done.

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

https://codereview.chromium.org/212603011/diff/300001/chrome/browser/profiles...
chrome/browser/profiles/profile_shortcut_manager_win.cc:116: //
DLOG_ASSERT(avatar_bitmap.width() == avatar_bitmap.height());
On 2014/04/15 15:26:48, Monica Dinculescu wrote:
> I think this should be, if anything, a DCHECK, not a DLOG_ASSERT.

Done.

[Mike Lerman, 2014-04-15 17:56:53]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-15 17:57:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/320001

[commit-bot: I haz the power, 2014-04-15 20:11:53]

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

[commit-bot: I haz the power, 2014-04-15 20:11:55]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[Mike Lerman, 2014-04-16 16:00:18]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-16 16:01:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/360001

[commit-bot: I haz the power, 2014-04-16 16:36:09]

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

[commit-bot: I haz the power, 2014-04-16 16:36:09]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[viettrungluu, 2014-04-17 17:27:57]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:30:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/360001

[commit-bot: I haz the power, 2014-04-17 17:31:19]

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

[commit-bot: I haz the power, 2014-04-17 17:31:19]

Failed to apply patch for chrome/browser/profiles/profile_window.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/profiles/profile_window.cc
  Hunk #1 FAILED at 8.
  Hunk #2 succeeded at 223 (offset 3 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/profiles/profile_window.cc.rej

Patch:       chrome/browser/profiles/profile_window.cc
Index: chrome/browser/profiles/profile_window.cc
diff --git a/chrome/browser/profiles/profile_window.cc
b/chrome/browser/profiles/profile_window.cc
index
f6ed167e2bff00de546b70efa87ad85b2a933baa..309c1c6e738a590b6075b75dbe84abafbd8d9a02
100644
--- a/chrome/browser/profiles/profile_window.cc
+++ b/chrome/browser/profiles/profile_window.cc
@@ -8,8 +8,10 @@
 #include "base/files/file_path.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -220,9 +222,14 @@ void SwitchToGuestProfile(chrome::HostDesktopType
desktop_type,
 void CreateAndSwitchToNewProfile(chrome::HostDesktopType desktop_type,
                                  ProfileSwitchingDoneCallback callback,
                                  ProfileMetrics::ProfileAdd metric) {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  int placeholder_avatar_index = profiles::GetPlaceholderAvatarIndex();
   ProfileManager::CreateMultiProfileAsync(
-      base::string16(),
-      base::string16(),
+      cache.ChooseNameForNewProfile(placeholder_avatar_index),
+      base::UTF8ToUTF16(profiles::GetDefaultAvatarIconUrl(
+          placeholder_avatar_index)),
       base::Bind(&OpenBrowserWindowForProfile,
                  callback,
                  true,

[Mike Lerman, 2014-04-23 00:32:20]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-23 00:33:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/380001

[commit-bot: I haz the power, 2014-04-23 00:33:39]

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

[commit-bot: I haz the power, 2014-04-23 00:33:39]

Failed to apply patch for chrome/browser/profiles/profile_avatar_icon_util.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/profiles/profile_avatar_icon_util.cc
  Hunk #2 FAILED at 191.
  Hunk #3 succeeded at 291 (offset -11 lines).
  Hunk #4 succeeded at 303 (offset -11 lines).
  Hunk #5 succeeded at 325 (offset -11 lines).
  1 out of 5 hunks FAILED -- saving rejects to file
chrome/browser/profiles/profile_avatar_icon_util.cc.rej

Patch:       chrome/browser/profiles/profile_avatar_icon_util.cc
Index: chrome/browser/profiles/profile_avatar_icon_util.cc
diff --git a/chrome/browser/profiles/profile_avatar_icon_util.cc
b/chrome/browser/profiles/profile_avatar_icon_util.cc
index
30c0661c8eb32883ca27a1e4ce5c0b060651a606..37e78ca73de94f13fcb36f0de375460395c82d2d
100644
--- a/chrome/browser/profiles/profile_avatar_icon_util.cc
+++ b/chrome/browser/profiles/profile_avatar_icon_util.cc
@@ -176,8 +176,13 @@ void AvatarImageSource::Draw(gfx::Canvas* canvas) {
 
 namespace profiles {
 
+struct IconResourceInfo {
+  int resource_id;
+  const char* filename;
+};
+
 const int kAvatarIconWidth = 38;
-const int kAvatarIconHeight = 31;
+const int kAvatarIconHeight = 38;
 const int kAvatarIconPadding = 2;
 const SkColor kAvatarTutorialBackgroundColor = SkColorSetRGB(0x42, 0x85, 0xf4);
 const SkColor kAvatarTutorialContentTextColor = SkColorSetRGB(0xc6, 0xda,
0xfc);
@@ -186,100 +191,50 @@ const char kDefaultUrlPrefix[] =
"chrome://theme/IDR_PROFILE_AVATAR_";
 const char kGAIAPictureFileName[] = "Google Profile Picture.png";
 const char kHighResAvatarFolderName[] = "Avatars";
 
-const int kDefaultAvatarIconResources[] = {
-  IDR_PROFILE_AVATAR_0,
-  IDR_PROFILE_AVATAR_1,
-  IDR_PROFILE_AVATAR_2,
-  IDR_PROFILE_AVATAR_3,
-  IDR_PROFILE_AVATAR_4,
-  IDR_PROFILE_AVATAR_5,
-  IDR_PROFILE_AVATAR_6,
-  IDR_PROFILE_AVATAR_7,
-  IDR_PROFILE_AVATAR_8,
-  IDR_PROFILE_AVATAR_9,
-  IDR_PROFILE_AVATAR_10,
-  IDR_PROFILE_AVATAR_11,
-  IDR_PROFILE_AVATAR_12,
-  IDR_PROFILE_AVATAR_13,
-  IDR_PROFILE_AVATAR_14,
-  IDR_PROFILE_AVATAR_15,
-  IDR_PROFILE_AVATAR_16,
-  IDR_PROFILE_AVATAR_17,
-  IDR_PROFILE_AVATAR_18,
-  IDR_PROFILE_AVATAR_19,
-  IDR_PROFILE_AVATAR_20,
-  IDR_PROFILE_AVATAR_21,
-  IDR_PROFILE_AVATAR_22,
-  IDR_PROFILE_AVATAR_23,
-  IDR_PROFILE_AVATAR_24,
-  IDR_PROFILE_AVATAR_25,
-};
-
-// File names for the high-res avatar icon resources. In the same order as
-// the avatars in kDefaultAvatarIconResources.
-const char* kDefaultAvatarIconResourceFileNames[] = {
-  "avatar_generic.png",
-  "avatar_generic_aqua.png",
-  "avatar_generic_blue.png",
-  "avatar_generic_green.png",
-  "avatar_generic_orange.png",
-  "avatar_generic_purple.png",
-  "avatar_generic_red.png",
-  "avatar_generic_yellow.png",
-  "avatar_secret_agent.png",
-  "avatar_superhero.png",
-  "avatar_volley_ball.png",
-  "avatar_businessman.png",
-  "avatar_ninja.png",
-  "avatar_alien.png",
-  "avatar_smiley.png",
-  "avatar_flower.png",
-  "avatar_pizza.png",
-  "avatar_soccer.png",
-  "avatar_burger.png",
-  "avatar_cat.png",
-  "avatar_cupcake.png",
-  "avatar_dog.png",
-  "avatar_horse.png",
-  "avatar_margarita.png",
-  "avatar_note.png",
-  "avatar_sun_cloud.png",
-};
+// This avatar does not exist on the server, the high res copy is in the build.
+const char kNoHighResAvatar[] = "NothingToDownload";
 
-const size_t kDefaultAvatarIconsCount = arraysize(kDefaultAvatarIconResources);
+// The size of the function-static kDefaultAvatarIconResources array below.
+const size_t kDefaultAvatarIconsCount = 27;
 
 // The first 8 icons are generic.
 const size_t kGenericAvatarIconsCount = 8;
 
+// The avatar used as a placeholder (grey silhouette).
+const int kPlaceholderAvatarIcon = 26;
+
 gfx::Image GetSizedAvatarIconWithBorder(const gfx::Image& image,
-                                        bool is_rectangle,
+                                        bool is_gaia_image,
                                         int width, int height) {
-  if (!is_rectangle)
+  // The image requires no border or resizing.
+  if (!is_gaia_image && image.Height() <= height)
     return image;
 
   gfx::Size size(width, height);
 
-  // Source for a centered, sized icon with a border.
+  // Source for a centered, sized icon. GAIA images get a border.
   scoped_ptr<gfx::ImageSkiaSource> source(
       new AvatarImageSource(
           *image.ToImageSkia(),
           size,
           std::min(width, height),
           AvatarImageSource::POSITION_CENTER,
-          AvatarImageSource::BORDER_NORMAL));
+          is_gaia_image ? AvatarImageSource::BORDER_NORMAL :
+              AvatarImageSource::BORDER_NONE));
 
   return gfx::Image(gfx::ImageSkia(source.release(), size));
 }
 
 gfx::Image GetAvatarIconForMenu(const gfx::Image& image,
-                                bool is_rectangle) {
+                                bool is_gaia_image) {
   return GetSizedAvatarIconWithBorder(
-      image, is_rectangle, kAvatarIconWidth, kAvatarIconHeight);
+      image, is_gaia_image, kAvatarIconWidth, kAvatarIconHeight);
 }
 
 gfx::Image GetAvatarIconForWebUI(const gfx::Image& image,
-                                 bool is_rectangle) {
-  if (!is_rectangle)
+                                 bool is_gaia_image) {
+  // The image requires no border or resizing.
+  if (!is_gaia_image && image.Height() <= kAvatarIconHeight)
     return image;
 
   gfx::Size size(kAvatarIconWidth, kAvatarIconHeight);
@@ -297,10 +252,11 @@ gfx::Image GetAvatarIconForWebUI(const gfx::Image& image,
 }
 
 gfx::Image GetAvatarIconForTitleBar(const gfx::Image& image,
-                                    bool is_rectangle,
+                                    bool is_gaia_image,
                                     int dst_width,
                                     int dst_height) {
-  if (!is_rectangle)
+  // The image requires no border or resizing.
+  if (!is_gaia_image && image.Height() <= kAvatarIconHeight)
     return image;
 
   int size = std::min(std::min(kAvatarIconWidth, kAvatarIconHeight),
@@ -308,14 +264,15 @@ gfx::Image GetAvatarIconForTitleBar(const gfx::Image&
image,
   gfx::Size dst_size(dst_width, dst_height);
 
   // Source for a sized icon drawn at the bottom center of the canvas,
-  // with an etched border.
+  // with an etched border (for GAIA images).
   scoped_ptr<gfx::ImageSkiaSource> source(
       new AvatarImageSource(
           *image.ToImageSkia(),
           dst_size,
           size,
           AvatarImageSource::POSITION_BOTTOM_CENTER,
-          AvatarImageSource::BORDER_ETCHED));
+          is_gaia_image ? AvatarImageSource::BORDER_ETCHED :
+              AvatarImageSource::BORDER_NONE));
 
   return gfx::Image(gfx::ImageSkia(source.release(), dst_size));
 }
@@ -329,13 +286,59 @@ size_t GetGenericAvatarIconCount() {
   return kGenericAvatarIconsCount;
 }
 
+int GetPlaceholderAvatarIndex() {
+  return kPlaceholderAvatarIcon;
+}
+
+int GetPlaceholderAvatarIconResourceID() {
+  return IDR_PROFILE_AVATAR_26;
+}
+
+const IconResourceInfo* GetDefaultAvatarIconResourceInfo(size_t index) {
+  static const IconResourceInfo resource_info[kDefaultAvatarIconsCount] = {
+    { IDR_PROFILE_AVATAR_0, "avatar_generic.png"},
+    { IDR_PROFILE_AVATAR_1, "avatar_generic_aqua.png"},
+    { IDR_PROFILE_AVATAR_2, "avatar_generic_blue.png"},
+    { IDR_PROFILE_AVATAR_3, "avatar_generic_green.png"},
+    { IDR_PROFILE_AVATAR_4, "avatar_generic_orange.png"},
+    { IDR_PROFILE_AVATAR_5, "avatar_generic_purple.png"},
+    { IDR_PROFILE_AVATAR_6, "avatar_generic_red.png"},
+    { IDR_PROFILE_AVATAR_7, "avatar_generic_yellow.png"},
+    { IDR_PROFILE_AVATAR_8, "avatar_secret_agent.png"},
+    { IDR_PROFILE_AVATAR_9, "avatar_superhero.png"},
+    { IDR_PROFILE_AVATAR_10, "avatar_volley_ball.png"},
+    { IDR_PROFILE_AVATAR_11, "avatar_businessman.png"},
+    { IDR_PROFILE_AVATAR_12, "avatar_ninja.png"},
+    { IDR_PROFILE_AVATAR_13, "avatar_alien.png"},
+    { IDR_PROFILE_AVATAR_14, "avatar_smiley.png"},
+    { IDR_PROFILE_AVATAR_15, "avatar_flower.png"},
+    { IDR_PROFILE_AVATAR_16, "avatar_pizza.png"},
+    { IDR_PROFILE_AVATAR_17, "avatar_soccer.png"},
+    { IDR_PROFILE_AVATAR_18, "avatar_burger.png"},
+    { IDR_PROFILE_AVATAR_19, "avatar_cat.png"},
+    { IDR_PROFILE_AVATAR_20, "avatar_cupcake.png"},
+    { IDR_PROFILE_AVATAR_21, "avatar_dog.png"},
+    { IDR_PROFILE_AVATAR_22, "avatar_horse.png"},
+    { IDR_PROFILE_AVATAR_23, "avatar_margarita.png"},
+    { IDR_PROFILE_AVATAR_24, "avatar_note.png"},
+    { IDR_PROFILE_AVATAR_25, "avatar_sun_cloud.png"},
+    { IDR_PROFILE_AVATAR_26, kNoHighResAvatar},
+  };
+  return &resource_info[index];
+}
+
 int GetDefaultAvatarIconResourceIDAtIndex(size_t index) {
   DCHECK(IsDefaultAvatarIconIndex(index));
-  return kDefaultAvatarIconResources[index];
+  return GetDefaultAvatarIconResourceInfo(index)->resource_id;
 }
 
 const char* GetDefaultAvatarIconFileNameAtIndex(size_t index) {
-  return kDefaultAvatarIconResourceFileNames[index];
+  DCHECK(index < kDefaultAvatarIconsCount);
+  return GetDefaultAvatarIconResourceInfo(index)->filename;
+}
+
+const char* GetNoHighResAvatarFileName() {
+  return kNoHighResAvatar;
 }
 
 std::string GetDefaultAvatarIconUrl(size_t index) {

[Mike Lerman, 2014-04-23 14:17:35]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-04-23 14:18:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlerman@chromium.org/212603011/410001

[commit-bot: I haz the power, 2014-04-23 15:34:40]

Change committed as 265645