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

chrome/browser/app_controller_mac.mm

 // 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.
 
 #import "chrome/browser/app_controller_mac.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 10 matching lines @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #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"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
@@ skipping 142 matching lines @@
 - (void)openUrls:(const std::vector<GURL>&)urls;
 - (void)getUrl:(NSAppleEventDescriptor*)event
      withReply:(NSAppleEventDescriptor*)reply;
 - (void)submitCloudPrintJob:(NSAppleEventDescriptor*)event;
 - (void)windowLayeringDidChange:(NSNotification*)inNotification;
 - (void)windowChangedToProfile:(Profile*)profile;
 - (void)checkForAnyKeyWindows;
 - (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_;
 
 // This method is called very early in application startup (ie, before
 // the profile is loaded or any preferences have been registered). Defer any
 // user-data initialization until -applicationDidFinishLaunching:.
 - (void)awakeFromNib {
   // We need to register the handlers early to catch events fired on launch.
   NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
@@ skipping 373 matching lines @@
   // Build up the encoding menu, the order of the items differs based on the
   // current locale (see http://crbug.com/7647 for details).
   // We need a valid g_browser_process to get the profile which is why we can't
   // call this from awakeFromNib.
   NSMenu* viewMenu = [[[NSApp mainMenu] itemWithTag:IDC_VIEW_MENU] submenu];
   NSMenuItem* encodingMenuItem = [viewMenu itemWithTag:IDC_ENCODING_MENU];
   NSMenu* encodingMenu = [encodingMenuItem submenu];
   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_];
 
   // Record the path to the (browser) app bundle; this is used by the app mode
   // shim.
   RecordLastRunAppBundlePath();
 
   // Makes "Services" menu items available.
   [self registerServicesMenuTypesTo:[notify object]];
@@ skipping 109 matching lines @@
 
 // Called to determine if we should enable the "restore tab" menu item.
 // Checks with the TabRestoreService to see if there's anything there to
 // restore and returns YES if so.
 - (BOOL)canRestoreTab {
   TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile([self lastProfile]);
   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
 // dialogs that are window modal will block the browser.
 - (BOOL)keyWindowIsModal {
   if ([NSApp modalWindow])
     return YES;
 
   Browser* browser = chrome::GetLastActiveBrowser();
   return browser &&
@@ skipping 589 matching lines @@
 
 //---------------------------------------------------------------------------
 
 namespace app_controller_mac {
 
 bool IsOpeningNewWindow() {
   return g_is_opening_new_window;
 }
 
 }  // namespace app_controller_mac