Profile picture doesn't display correctly after new profile has been created

chrome/browser/profiles/profile_info_cache.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/profiles/profile_info_cache.h"
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
@@ skipping 215 matching lines @@
   info->SetBoolean(kProfileIsEphemeral, false);
   info->SetBoolean(kIsUsingDefaultName, IsDefaultName(name));
   cache->SetWithoutPathExpansion(key, info.release());
 
   sorted_keys_.insert(FindPositionForProfile(key, name), key);
 
   FOR_EACH_OBSERVER(ProfileInfoCacheObserver,
                     observer_list_,
                     OnProfileAdded(profile_path));
 
+  if (switches::IsNewAvatarMenu())

[noms (inactive), 2014/07/11 13:47:56]

I'm sorry, I didn't see this initially because the code was hidden. Could you
move this above the OnProfileAdded notification? Basically, the notifications
should only be sent out once we've done all the work.

[Malcolm, 2014/07/11 14:18:44]

You are right, this should be done before notification is sent out. Also, I have
think about a solution to make the failed test pass, just declare the method
like:
void ProfileInfoCache::AddProfileToCache(
    const base::FilePath& profile_path,
    const base::string16& name,
    const base::string16& username,
    size_t icon_index,
    const std::string& supervised_user_id,
    bool willDownloadResAvatar = true); 

then in the ProfileInfoCacheTest.DownloadHighResAvatarTest call like below:
  GetCache()->AddProfileToCache(path_1, ASCIIToUTF16("name_1"),
                                base::string16(), 0, std::string(), false);

This will not break the current code and make the test work fine. What's your
suggestion?

On 2014/07/11 13:47:56, Monica Dinculescu wrote:

[noms (inactive), 2014/07/11 15:59:11]

Default arguments are not allowed in Chromium, unfortunately. I will take a look
at the ProfileInfoCache today and see whether this call can be moved somewhere
else, or mocked out in tests.

On 2014/07/11 14:18:44, Malcolm wrote:

+    DownloadHighResAvatar(icon_index, profile_path);
+
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 }
 
 void ProfileInfoCache::AddObserver(ProfileInfoCacheObserver* obs) {
   observer_list_.AddObserver(obs);
 }
 
@@ skipping 764 matching lines @@
                     OnProfileAvatarChanged(profile_path));
 
   // Remove the file from the list of downloads in progress. Note that this list
   // only contains the high resolution avatars, and not the Gaia profile images.
   if (!avatar_images_downloads_in_progress_[file_name])
     return;
 
   delete avatar_images_downloads_in_progress_[file_name];
   avatar_images_downloads_in_progress_[file_name] = NULL;
 }