[Mac] AppController needs to update its "last profile" pointer when the active profile is deleted

chrome/browser/app_controller_mac.mm

Index: chrome/browser/app_controller_mac.mm
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index feb0f95f782cfa782706dee63350095316590490..7057fdbdaa8c9a01791b86ed9014447dcb811807 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -28,6 +28,7 @@
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/printing/print_dialog_cloud.h"
+#include "chrome/browser/profiles/profile_info_cache_observer.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/service/service_process_control.h"
 #include "chrome/browser/sessions/session_restore.h"
@@ -190,8 +191,54 @@ void RecordLastRunAppBundlePath() {
 - (BOOL)userWillWaitForInProgressDownloads:(int)downloadCount;
 - (BOOL)shouldQuitWithInProgressDownloads;
 - (void)executeApplication:(id)sender;
+- (void)profileWasRemoved:(const string16&)profileName;
 @end
 
+class AppControllerProfileObserver : public ProfileInfoCacheObserver {
+ public:
+  AppControllerProfileObserver(
+    ProfileManager* profile_manager, AppController* app_controller)
+        : profile_manager_(profile_manager),
+          app_controller_(app_controller) {
+    DCHECK(profile_manager_);
+    profile_manager_->GetProfileInfoCache().AddObserver(this);

[Alexei Svitkine (slow), 2013/05/08 15:25:06]

I don't think you need the DCHECK() if the following line would crash already if
it was NULL. The point of the DCHECK() is to catch errors earlier than the bad
stuff they would cause to happen.

[noms (inactive), 2013/05/08 15:56:53]

I originally had an if (profile_manager_) check, and Roger suggested I should
replace it with a DCHECK(). Do you think there should be no check at all, or no
DCHECK() but with an if () check?

On 2013/05/08 15:25:06, Alexei Svitkine wrote:

[Alexei Svitkine (slow), 2013/05/08 16:32:07]

Don't have an if check unless there's a scenario where the profile_manager_ can
actually be NULL (e.g. maybe in some tests?). If it is possible in tests, then
you need the if (or somehow make the tests ensure that it won't be NULL).

+  }
+
+  virtual ~AppControllerProfileObserver() {
+    DCHECK(profile_manager_);
+    profile_manager_->GetProfileInfoCache().RemoveObserver(this);
+  }
+
+ private:
+  ProfileManager* profile_manager_;

[Alexei Svitkine (slow), 2013/05/08 15:25:06]

These should be below the methods.

[noms (inactive), 2013/05/08 19:10:35]

Done.

+
+  AppController* app_controller_;  // Weak; owns us.
+
+  // ProfileInfoCacheObserver implementation:
+
+  void OnProfileAdded(const base::FilePath& profile_path) {

[Alexei Svitkine (slow), 2013/05/08 15:25:06]

All of these should be marked "virtual" and "OVERRIDE".

[noms (inactive), 2013/05/08 19:10:35]

Done.

+  }
+
+  void OnProfileWasRemoved(const base::FilePath& profile_path,
+                           const string16& profile_name) {
+    // When a profile is deleted we need to notify the AppController,
+    // so that it can correctly update its pointer to the last used profile.
+    [app_controller_ profileWasRemoved:profile_name];
+  }
+
+  void OnProfileWillBeRemoved(const base::FilePath& profile_path) {
+  }
+
+  void OnProfileNameChanged(const base::FilePath& profile_path,
+                            const string16& old_profile_name) {
+  }
+
+  void OnProfileAvatarChanged(const base::FilePath& profile_path) {
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(AppControllerProfileObserver);
+};
+
 @implementation AppController
 
 @synthesize startupComplete = startupComplete_;
@@ -585,6 +632,11 @@ void RecordLastRunAppBundlePath() {
   EncodingMenuControllerDelegate::BuildEncodingMenu([self lastProfile],
                                                     encodingMenu);
 
+  // Instantiate the ProfileInfoCache observer so that we can get
+  // notified when a profile is deleted.
+  profileInfoCacheObserver_.reset(new AppControllerProfileObserver(
+      g_browser_process->profile_manager(), self));
+
   // Since Chrome is localized to more languages than the OS, tell Cocoa which
   // menu is the Help so it can add the search item to it.
   [NSApp setHelpMenu:helpMenu_];
@@ -714,6 +766,19 @@ void RecordLastRunAppBundlePath() {
   return service && !service->entries().empty();
 }
 
+// Called from the AppControllerProfileObserver every time a profile is deleted.
+- (void)profileWasRemoved:(const string16&)profileName {
+  Profile* lastProfile = [self lastProfile];
+  string16 lastProfileName = UTF8ToUTF16(lastProfile->GetPrefs()->GetString(
+      prefs::kProfileName));
+
+  // If the last profile has been deleted, we must NULL out our pointer
+  // otherwise we will try to start up a browser window with a pointer
+  // to the old profile.
+  if (profileName == lastProfileName)
+    lastProfile_ = NULL;
+}
+
 // Returns true if there is a modal window (either window- or application-
 // modal) blocking the active browser. Note that tab modal dialogs (HTTP auth
 // sheets) will not count as blocking the browser. But things like open/save