Fix Profile* lifetime issues in Chrome's AppListViewDelegate

chrome/browser/ui/app_list/app_list_service_mac.mm

Index: chrome/browser/ui/app_list/app_list_service_mac.mm
diff --git a/chrome/browser/ui/app_list/app_list_service_mac.mm b/chrome/browser/ui/app_list/app_list_service_mac.mm
index 8e448e8cde0a1c4c728ce16299c30b7bcee3fd59..27f10bc8f943b31d52a37853734c35758203924a 100644
--- a/chrome/browser/ui/app_list/app_list_service_mac.mm
+++ b/chrome/browser/ui/app_list/app_list_service_mac.mm
@@ -392,9 +392,11 @@ void AppListServiceMac::CreateForProfile(Profile* requested_profile) {
   if (!window_controller_)
     window_controller_.reset([[AppListWindowController alloc] init]);
 
-  scoped_ptr<app_list::AppListViewDelegate> delegate(
-      new AppListViewDelegate(profile_, GetControllerDelegate()));
-  [[window_controller_ appListViewController] setDelegate:delegate.Pass()];
+  scoped_ptr<AppListViewDelegate> delegate(
+      new AppListViewDelegate(GetControllerDelegate()));
+  delegate->SetProfile(profile_);
+  [[window_controller_ appListViewController]
+      setDelegate:delegate.PassAs<app_list::AppListViewDelegate>()];

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

Can you explain this line? (To me: not necessarily in a comment.)

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

Yeah - we really should rename ::AppListViewDelegate to
::ChromeAppListViewDelegate, since it makes things like this extra confusing.

SetProfile doesn't (and shouldn't) exist on app_list::AppListViewDelegate --
it's only on ::AppListViewDelegate, hence the change on line 395.

Then the mac AppListViewController is using scoped_ptr to indicate an ownership
transferral, but scoped_ptr<B> can't implicitly upcast to scoped_ptr<A> (when B
inherits from A). So this has to be done with PassAs, which does the implicit
cast so that it can be passed to a scoped_ptr of its parent type.

[Matt Giuca, 2014/08/26 01:29:45]

But you reverted the use of PassAs now?

[tapted, 2014/08/26 02:43:57]

Yep - since the constructor is calling SetProfile, there's no need for a
::AppListViewDelegate instead of an app_list::AppListViewDelegate here

 }
 
 void AppListServiceMac::ShowForProfile(Profile* requested_profile) {
@@ -452,6 +454,17 @@ void AppListServiceMac::CreateShortcut() {
       g_browser_process->profile_manager()->user_data_dir()));
 }
 
+void AppListServiceMac::DestroyAppList() {
+  // Due to reference counting, Mac can't guarantee that the widget is deleted,
+  // but mac supports a visible app list with a NULL profile, so there's also no
+  // need to tear it down completely.
+  DismissAppList();
+  [[window_controller_ appListViewController]
+      setDelegate:scoped_ptr<app_list::AppListViewDelegate>()];
+
+  profile_ = NULL;
+}
+
 NSWindow* AppListServiceMac::GetAppListWindow() {
   return [window_controller_ window];
 }