Commit 966219da authored by Matt Giuca's avatar Matt Giuca Committed by Commit Bot

AppMenuButton: Fixed use-after-free opening menu for the second time.

Caused by r549433, which swapped the initialization order of the AppMenu
and AppMenuModel. By constructing the AppMenuModel first, the old one
was being destroyed, being outlived by its AppMenu (holding a raw
pointer). Adds a reset(), ensuring the AppMenu is destroyed first.

Bug: 831504
No-Try: true
Change-Id: I3d155b11aa2490537fc6baa9fbbf2bdb057e8090
Reviewed-on: https://chromium-review.googlesource.com/1006739
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550022}
parent 1ca39312
......@@ -36,6 +36,9 @@ void AppMenuButton::RemoveMenuListener(views::MenuListener* listener) {
void AppMenuButton::InitMenu(std::unique_ptr<AppMenuModel> menu_model,
Browser* browser,
int run_flags) {
// |menu_| must be reset before |menu_model_| is destroyed, as per the comment
// in the class declaration.
menu_.reset();
menu_model_ = std::move(menu_model);
menu_model_->Init();
menu_ = std::make_unique<AppMenu>(browser, run_flags);
......
......@@ -56,6 +56,8 @@ class AppMenuButton : public views::MenuButton {
// App model and menu.
// Note that the menu should be destroyed before the model it uses, so the
// menu should be listed later.
// TODO(mgiuca): Simplify this model so that correctness does not depend on
// destruction order. https://crbug.com/831902
std::unique_ptr<AppMenuModel> menu_model_;
std::unique_ptr<AppMenu> menu_;
......
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