Fix Mac App Launcher observer lifetime on AppListItemList
r295646 moved ownership of the AppListViewDelegate out of the AppListViewController which meant that the Mac AppList began removing itself as an observer from the wrong model. This happens because, in Chrome, the AppListViewDelegate changes the return from GetModel() when the profile is switched, but doesn't currently have a way of notifying the AppList that this is happening. To fix, AppListServiceMac needs to explicitly clear the delegate on the Chrome side before asking the view delegate to switch out its model (which happens via AppListServiceImpl::GetViewDelegate(Profile*). Views platforms are not affected, because they retain a raw pointer to the AppListModel* separately from AppListViewDelegate::GetModel(). This only works because the AppListModel* sticks around until Chrome shutdown, when it's torn down with the other keyed services via the ~ProfileManager() destructor. This delayed destruction means that verifying observers in a test needs a lot of plumbing in non-test code (see patchset1). Instead, a DCHECK is added in AppListItemList to ensure RemoveObserver always removes something, then test coverage is given by AppListServiceImplBrowserTest.ShowLoadedProfiles. This CL fixes part of the flakiness in the app list interactive uitests on Mac (http://crbug.com/415264), but I've discovered other sources of flakiness to fix before re-enabling those tests. It should, however, fix the recent crashes (http://crbug.com/416345). BUG=415264, 416345 TEST=AppListServiceImplBrowserTest.ShowLoadedProfiles TEST=crbug/416345: have multiple profiles and switch between them in the app launcher on mac many times. Chrome shouldn't crash. Review URL: https://codereview.chromium.org/592983002 Cr-Commit-Position: refs/heads/master@{#296152}
Showing
Please register or sign in to comment