Commit d5857f58 authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

Make AvatarMenu use a weak pointer for ProfileAttributesStorage

The profile_storage_ member of AvatarMenu is commented as "weak", but
the lifetime relationship is not discussed. Use an actual weak
pointer (ProfileAttributesStorage supports it already), and avoid
dereferencing profile_storage_ if it is gone.

Bug: 1008947, 1001215
Change-Id: I0e8a2fbb55465f1680c064c7cdb6193c93eb3e79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853647Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704908}
parent 7ee94745
...@@ -262,8 +262,9 @@ bool ExtensionAppShimHandler::AppState::IsMultiProfile() const { ...@@ -262,8 +262,9 @@ bool ExtensionAppShimHandler::AppState::IsMultiProfile() const {
std::unique_ptr<AvatarMenu> ExtensionAppShimHandler::Delegate::CreateAvatarMenu( std::unique_ptr<AvatarMenu> ExtensionAppShimHandler::Delegate::CreateAvatarMenu(
AvatarMenuObserver* observer) { AvatarMenuObserver* observer) {
// TODO(https://crbug.com/1008947): Fix use-after-free caused by AvatarMenu. ProfileManager* profile_manager = g_browser_process->profile_manager();
return nullptr; return std::make_unique<AvatarMenu>(
&profile_manager->GetProfileAttributesStorage(), observer, nullptr);
} }
Profile* ExtensionAppShimHandler::Delegate::ProfileForPath( Profile* ExtensionAppShimHandler::Delegate::ProfileForPath(
......
...@@ -42,7 +42,7 @@ AvatarMenu::AvatarMenu(ProfileAttributesStorage* profile_storage, ...@@ -42,7 +42,7 @@ AvatarMenu::AvatarMenu(ProfileAttributesStorage* profile_storage,
Browser* browser) Browser* browser)
: profile_list_(ProfileList::Create(profile_storage)), : profile_list_(ProfileList::Create(profile_storage)),
menu_actions_(AvatarMenuActions::Create()), menu_actions_(AvatarMenuActions::Create()),
profile_storage_(profile_storage), profile_storage_(profile_storage->AsWeakPtr()),
observer_(observer), observer_(observer),
browser_(browser) { browser_(browser) {
DCHECK(profile_storage_); DCHECK(profile_storage_);
...@@ -64,7 +64,10 @@ AvatarMenu::AvatarMenu(ProfileAttributesStorage* profile_storage, ...@@ -64,7 +64,10 @@ AvatarMenu::AvatarMenu(ProfileAttributesStorage* profile_storage,
} }
AvatarMenu::~AvatarMenu() { AvatarMenu::~AvatarMenu() {
profile_storage_->RemoveObserver(this); // Note that |profile_storage_| may be destroyed before |this|.
// https://crbug.com/1008947
if (profile_storage_)
profile_storage_->RemoveObserver(this);
} }
AvatarMenu::Item::Item(size_t menu_index, const base::FilePath& profile_path, AvatarMenu::Item::Item(size_t menu_index, const base::FilePath& profile_path,
......
...@@ -200,8 +200,8 @@ class AvatarMenu : ...@@ -200,8 +200,8 @@ class AvatarMenu :
supervised_user_observer_{this}; supervised_user_observer_{this};
#endif #endif
// The storage that provides the profile attributes. Weak. // The storage that provides the profile attributes.
ProfileAttributesStorage* profile_storage_; base::WeakPtr<ProfileAttributesStorage> profile_storage_;
// The observer of this model, which is notified of changes. Weak. // The observer of this model, which is notified of changes. Weak.
AvatarMenuObserver* observer_; AvatarMenuObserver* observer_;
......
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