ProfileInfoCache: Don't delete ProfileAvatarDownloader while we're being called by it

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/files/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
@@ skipping 130 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   if (!base::PathExists(file_path))
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
 }
 
 void DeleteBitmap(const base::FilePath& image_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::DeleteFile(image_path, false);
 }
 
+// Used by SaveAvatarImageAtPath to post a task to delete the |downloader|
+// "soon". We can't just delete it directly there because
+// SaveAvatarImageAtPath is called from this very downloader.
+void DeleteDownloader(ProfileAvatarDownloader* downloader) {
+  delete downloader;
+}
+
 }  // namespace
 
 ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
                                    const base::FilePath& user_data_dir)
     : prefs_(prefs),
       user_data_dir_(user_data_dir) {
   // Populate the cache
   DictionaryPrefUpdate update(prefs_, prefs::kProfileInfoCache);
   base::DictionaryValue* cache = update.Get();
   for (base::DictionaryValue::Iterator it(*cache);
@@ skipping 735 matching lines @@
     const base::FilePath& image_path,
     const base::FilePath& profile_path) {
   cached_avatar_images_[key] = new gfx::Image(*image);
 
   scoped_ptr<ImageData> data(new ImageData);
   scoped_refptr<base::RefCountedMemory> png_data = image->As1xPNGBytes();
   data->assign(png_data->front(), png_data->front() + png_data->size());
 
   // 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_[key]) {
-    delete avatar_images_downloads_in_progress_[key];
-    avatar_images_downloads_in_progress_[key] = NULL;
+  auto downloader_iter = avatar_images_downloads_in_progress_.find(key);
+  if (downloader_iter != avatar_images_downloads_in_progress_.end()) {
+    // We mustn't delete the avatar downloader right here, since we're being
+    // called by it.
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(&DeleteDownloader,
+                                       downloader_iter->second));

[gab, 2015/02/03 18:20:16]

FYI,
BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, download_iter->second);
provides this exact functionality without having to implement your own delete
helper.

+    avatar_images_downloads_in_progress_.erase(downloader_iter);
   }
 
   if (!data->size()) {
     LOG(ERROR) << "Failed to PNG encode the image.";
   } else {
     base::Closure callback = base::Bind(&ProfileInfoCache::OnAvatarPictureSaved,
         AsWeakPtr(), key, profile_path);
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
         base::Bind(&SaveBitmap, base::Passed(&data), image_path, callback));
   }
@@ skipping 122 matching lines @@
 void ProfileInfoCache::DownloadHighResAvatar(
     size_t icon_index,
     const base::FilePath& profile_path) {
   // Downloading is only supported on desktop.
 #if defined(OS_ANDROID) || defined(OS_IOS) || defined(OS_CHROMEOS)
   return;
 #endif
   const std::string file_name =
       profiles::GetDefaultAvatarIconFileNameAtIndex(icon_index);
   // If the file is already being downloaded, don't start another download.
-  if (avatar_images_downloads_in_progress_[file_name])
+  if (avatar_images_downloads_in_progress_.count(file_name))
     return;
 
   // Start the download for this file. The cache takes ownership of the
   // |avatar_downloader|, which will be deleted when the download completes, or
   // if that never happens, when the ProfileInfoCache is destroyed.
   ProfileAvatarDownloader* avatar_downloader = new ProfileAvatarDownloader(
       icon_index,
       profile_path,
       this);
   avatar_images_downloads_in_progress_[file_name] = avatar_downloader;
@@ skipping 99 matching lines @@
   std::vector<base::FilePath>::const_iterator it;
   for (it = profiles_to_rename.begin(); it != profiles_to_rename.end(); ++it) {
     size_t profile_index = GetIndexOfProfileWithPath(*it);
     SetProfileIsUsingDefaultNameAtIndex(profile_index, true);
     // This will assign a new "Person %d" type name and re-sort the cache.
     SetNameOfProfileAtIndex(profile_index, ChooseNameForNewProfile(
         GetAvatarIconIndexOfProfileAtIndex(profile_index)));
   }
 #endif
 }