Commit fb3b013d authored by Alex Newcomer's avatar Alex Newcomer Committed by Commit Bot

cros: Fix crash on multiprofile switch

If the profile being left had a folder, we kept a reference to that
folder in AppListModel's ScopedObserverList.

But when profiles  are swapped, we delete everything in the
AppListModel::top_level_item_list_. This means the folder is deleted.

Remove the item from the ScopedObserverList if it is a folder.

Bug: 1130901
Change-Id: I1d6ab268739f81904debcc4654e5663ddf768013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2464151
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816300}
parent fe0a5fdc
......@@ -651,6 +651,29 @@ TEST_F(AppListControllerImplTest,
EXPECT_FALSE(GetAppListView()->GetWidget()->GetNativeWindow()->IsVisible());
}
// Tests that swapping out an AppListModel (simulating a profile swap with
// multiprofile enabled) drops all references to previous folders (see
// https://crbug.com/1130901).
TEST_F(AppListControllerImplTest, SimulateProfileSwapNoCrashOnDestruct) {
// Add a folder, whose AppListItemList the AppListModel will observe.
const std::string folder_id("folder_1");
auto folder = std::make_unique<AppListFolderItem>(folder_id);
AppListModel* model = Shell::Get()->app_list_controller()->GetModel();
model->AddItem(std::move(folder));
for (int i = 0; i < 2; ++i) {
auto item = std::make_unique<AppListItem>(base::StringPrintf("app_%d", i));
model->AddItemToFolder(std::move(item), folder_id);
}
// Set a new model, simulating profile switching in multi-profile mode. This
// should cleanly drop the reference to the folder added earlier.
Shell::Get()->app_list_controller()->SetModelData(
/*profile_id=*/12, /*apps=*/{}, /*is_search_engine_google=*/false);
// Test that there is no crash on ~AppListModel() when the test finishes.
}
class AppListControllerImplTestWithNotificationBadging
: public AppListControllerImplTest {
public:
......
......@@ -302,6 +302,10 @@ void AppListModel::DeleteAllItems() {
const std::string id = item->id();
for (auto& observer : observers_)
observer.OnAppListItemWillBeDeleted(item);
if (item->GetItemType() == AppListFolderItem::kItemType) {
item_list_scoped_observer_.Remove(
static_cast<AppListFolderItem*>(item)->item_list());
}
top_level_item_list_->DeleteItemAt(0);
for (auto& observer : observers_)
observer.OnAppListItemDeleted(id);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment