Fix Profile* lifetime issues in Chrome's AppListViewDelegate

chrome/browser/ui/app_list/app_list_service_impl.cc

 // Copyright 2013 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/ui/app_list/app_list_service_impl.h"
 
 #include <string>
 
 #include "apps/pref_names.h"
 #include "base/bind.h"
@@ skipping 266 matching lines @@
 }
 
 void AppListServiceImpl::SetProfilePath(const base::FilePath& profile_path) {
   local_state_->SetString(
       prefs::kAppListProfile,
       profile_path.BaseName().MaybeAsASCII());
 }
 
 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() {
   profile_loader_->LoadProfileInvalidatingOtherLoads(
       GetProfilePath(profile_store_->GetUserDataDir()),
       base::Bind(&AppListServiceImpl::ShowForProfile,
                  weak_factory_.GetWeakPtr()));
 }
 
 void AppListServiceImpl::AutoShowForProfile(Profile* requested_profile) {
@@ skipping 52 matching lines @@
   if (!base::MessageLoop::current())
     return;  // In a unit test.
 
   // Send app list usage stats after a delay.
   const int kSendUsageStatsDelay = 5;
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&AppListServiceImpl::SendAppListStats),
       base::TimeDelta::FromSeconds(kSendUsageStatsDelay));
 }