Skip to content

Commit

Permalink
Fixed tab crash when tab is closed from context menu (uplift to 1.62.…
Browse files Browse the repository at this point in the history
…x) (#21401)

Uplift of #21397 (squashed) to beta
  • Loading branch information
brave-builds authored Dec 19, 2023
1 parent 056a36e commit c3ba2ca
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
32 changes: 24 additions & 8 deletions browser/ui/views/tabs/brave_tab_context_menu_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>
#include <vector>

#include "base/functional/bind.h"
#include "brave/browser/ui/browser_commands.h"
#include "brave/browser/ui/tabs/brave_tab_menu_model.h"
#include "brave/browser/ui/tabs/brave_tab_prefs.h"
Expand Down Expand Up @@ -47,7 +48,9 @@ BraveTabContextMenuContents::BraveTabContextMenuContents(
TabRestoreServiceFactory::GetForProfile(browser_->profile());
menu_runner_ = std::make_unique<views::MenuRunner>(
model_.get(),
views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::CONTEXT_MENU);
views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::CONTEXT_MENU,
base::BindRepeating(&BraveTabContextMenuContents::OnMenuClosed,
weak_ptr_.GetWeakPtr()));
}

BraveTabContextMenuContents::~BraveTabContextMenuContents() = default;
Expand All @@ -64,7 +67,7 @@ void BraveTabContextMenuContents::RunMenuAt(const gfx::Point& point,
}

bool BraveTabContextMenuContents::IsCommandIdChecked(int command_id) const {
if (!controller_->GetModelIndexOf(tab_)) {
if (!IsValidContextMenu()) {
return false;
}

Expand All @@ -77,7 +80,7 @@ bool BraveTabContextMenuContents::IsCommandIdChecked(int command_id) const {

bool BraveTabContextMenuContents::IsCommandIdEnabled(int command_id) const {
// This could be called after tab is closed.
if (!controller_->GetModelIndexOf(tab_)) {
if (!IsValidContextMenu()) {
return false;
}

Expand All @@ -89,7 +92,7 @@ bool BraveTabContextMenuContents::IsCommandIdEnabled(int command_id) const {
}

bool BraveTabContextMenuContents::IsCommandIdVisible(int command_id) const {
if (!controller_->GetModelIndexOf(tab_)) {
if (!IsValidContextMenu()) {
return false;
}

Expand All @@ -103,7 +106,7 @@ bool BraveTabContextMenuContents::IsCommandIdVisible(int command_id) const {
bool BraveTabContextMenuContents::GetAcceleratorForCommandId(
int command_id,
ui::Accelerator* accelerator) const {
if (!controller_->GetModelIndexOf(tab_)) {
if (!IsValidContextMenu()) {
return false;
}

Expand All @@ -120,7 +123,7 @@ bool BraveTabContextMenuContents::GetAcceleratorForCommandId(

void BraveTabContextMenuContents::ExecuteCommand(int command_id,
int event_flags) {
if (!controller_->GetModelIndexOf(tab_)) {
if (!IsValidContextMenu()) {
return;
}

Expand All @@ -135,7 +138,7 @@ void BraveTabContextMenuContents::ExecuteCommand(int command_id,

bool BraveTabContextMenuContents::IsBraveCommandIdEnabled(
int command_id) const {
CHECK(controller_->GetModelIndexOf(tab_));
CHECK(IsValidContextMenu());

switch (command_id) {
case BraveTabMenuModel::CommandRestoreTab:
Expand Down Expand Up @@ -166,7 +169,7 @@ bool BraveTabContextMenuContents::IsBraveCommandIdEnabled(
}

void BraveTabContextMenuContents::ExecuteBraveCommand(int command_id) {
CHECK(controller_->GetModelIndexOf(tab_));
CHECK(IsValidContextMenu());

switch (command_id) {
case BraveTabMenuModel::CommandRestoreTab:
Expand Down Expand Up @@ -206,3 +209,16 @@ bool BraveTabContextMenuContents::IsBraveCommandId(int command_id) const {
return command_id > BraveTabMenuModel::CommandStart &&
command_id < BraveTabMenuModel::CommandLast;
}

bool BraveTabContextMenuContents::IsValidContextMenu() const {
if (menu_closed_) {
return false;
}

return controller_->GetModelIndexOf(tab_).has_value() &&
controller_->model()->ContainsIndex(tab_index_);
}

void BraveTabContextMenuContents::OnMenuClosed() {
menu_closed_ = true;
}
12 changes: 11 additions & 1 deletion browser/ui/views/tabs/brave_tab_context_menu_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "ui/base/models/simple_menu_model.h"
#include "ui/base/ui_base_types.h"

Expand Down Expand Up @@ -59,15 +60,24 @@ class BraveTabContextMenuContents : public ui::SimpleMenuModel::Delegate {
bool IsBraveCommandIdEnabled(int command_id) const;
void ExecuteBraveCommand(int command_id);
bool IsBraveCommandId(int command_id) const;
bool IsValidContextMenu() const;
void OnMenuClosed();

std::unique_ptr<BraveTabMenuModel> model_;
std::unique_ptr<views::MenuRunner> menu_runner_;

raw_ptr<Tab> tab_ = nullptr;
int tab_index_;
int tab_index_ = -1;

// true when menu is closed.
// If it's set to true, this instance will not be used anymore because
// new instance is created when showing context menu.
bool menu_closed_ = false;
raw_ptr<Browser> browser_ = nullptr;
raw_ptr<sessions::TabRestoreService> restore_service_ = nullptr;
raw_ptr<BraveBrowserTabStripController> controller_ = nullptr;

base::WeakPtrFactory<BraveTabContextMenuContents> weak_ptr_{this};
};

#endif // BRAVE_BROWSER_UI_VIEWS_TABS_BRAVE_TAB_CONTEXT_MENU_CONTENTS_H_

0 comments on commit c3ba2ca

Please sign in to comment.