Fix Profile* lifetime issues in Chrome's AppListViewDelegate

chrome/browser/ui/app_list/app_list_service_impl.cc

Index: chrome/browser/ui/app_list/app_list_service_impl.cc
diff --git a/chrome/browser/ui/app_list/app_list_service_impl.cc b/chrome/browser/ui/app_list/app_list_service_impl.cc
index cd59dea6d955f87ba5d7014de5b362295f21d234..1d2383488f307472ba2fdb6fecc5b3818179f23b 100644
--- a/chrome/browser/ui/app_list/app_list_service_impl.cc
+++ b/chrome/browser/ui/app_list/app_list_service_impl.cc
@@ -284,17 +284,29 @@ void AppListServiceImpl::SetProfilePath(const base::FilePath& profile_path) {
 
 void AppListServiceImpl::CreateShortcut() {}
 
-// We need to watch for profile removal to keep kAppListProfile updated.
 void AppListServiceImpl::OnProfileWillBeRemoved(
     const base::FilePath& profile_path) {
-  // If the profile the app list uses just got deleted, reset it to the last
-  // used profile.
+  // We need to watch for profile removal to keep kAppListProfile updated, for
+  // the case that the deleted profile is being used by the app list.
   std::string app_list_last_profile = local_state_->GetString(
       prefs::kAppListProfile);
-  if (profile_path.BaseName().MaybeAsASCII() == app_list_last_profile) {
-    local_state_->SetString(prefs::kAppListProfile,
-        local_state_->GetString(prefs::kProfileLastUsed));
-  }
+  if (profile_path.BaseName().MaybeAsASCII() != app_list_last_profile)
+    return;
+
+  // Before ProfileInfoCache::DeleteProfileFromCache() calls this function,

[Matt Giuca, 2014/08/25 04:48:46]

// Switch the app list over to a valid profile. <followed by the existing text>.

[tapted, 2014/08/25 06:17:00]

Done.

+  // ProfileManager::ScheduleProfileForDeletion() will have checked to see if
+  // the deleted profile was also "last used", and updated that setting with
+  // something valid.
+  local_state_->SetString(prefs::kAppListProfile,
+                          local_state_->GetString(prefs::kProfileLastUsed));
+
+  // Now, the Chrome AppListViewDelegate needs to be torn down, since it has

[Matt Giuca, 2014/08/25 04:48:46]

It might be simpler to lay this out as a bullet list:

// The AppListViewDelegate needs to be torn down, because:
1. many references
2. might not be loaded yet.
// Currently, it might be owned by ....

[tapted, 2014/08/25 06:17:00]

Done.

+  // many references to the profile and can't be profile-keyed. Note also that
+  // the last used profile might not be loaded yet, since that's sometimes done

[Matt Giuca, 2014/08/25 04:48:46]

nit: "sometimes done *by*"

[tapted, 2014/08/25 06:17:00]

Done.

+  // ProfileManager asynchronously, so the app list can't just switch to that.
+  // Currently, the AppListViewDelegate is owned by the platform-specific
+  // AppListView, so just force-close the window.
+  DestroyAppList();
 }
 
 void AppListServiceImpl::Show() {