Fix Profile* lifetime issues in Chrome's AppListViewDelegate

chrome/browser/ui/app_list/app_list_service_mac.mm

 // 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.
 
 #import "chrome/browser/ui/app_list/app_list_service_mac.h"
 
 #include <ApplicationServices/ApplicationServices.h>
 #import <Cocoa/Cocoa.h>
 
 #include "base/bind.h"
@@ skipping 374 matching lines @@
 
 void AppListServiceMac::CreateForProfile(Profile* requested_profile) {
   if (profile_ == requested_profile)
     return;
 
   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) {
   InvalidatePendingProfileLoads();
 
   if (requested_profile == profile_) {
     ShowWindowNearDock();
     return;
   }
 
@@ skipping 37 matching lines @@
   AppListServiceImpl::EnableAppList(initial_profile, enable_source);
   AppController* controller = [NSApp delegate];
   [controller initAppShimMenuController];
 }
 
 void AppListServiceMac::CreateShortcut() {
   CreateAppListShim(GetProfilePath(
       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];
 }
 
 AppListControllerDelegate* AppListServiceMac::GetControllerDelegate() {
   return controller_delegate_.get();
 }
 
 void AppListServiceMac::OnShimLaunch(apps::AppShimHandler::Host* host,
                                      apps::AppShimLaunchType launch_type,
@@ skipping 128 matching lines @@
 
 - (void)animationDidEnd:(NSAnimation*)animation {
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&AppListServiceMac::WindowAnimationDidEnd,
                  base::Unretained(AppListServiceMac::GetInstance())));
 }
 
 @end