Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed crash from tab's context menu (uplift to 1.62.x) #21336

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions browser/ui/views/tabs/brave_browser_tab_strip_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ BraveBrowserTabStripController::~BraveBrowserTabStripController() {
context_menu_contents_->Cancel();
}

const std::optional<int> BraveBrowserTabStripController::GetModelIndexOf(
Tab* tab) {
return tabstrip_->GetModelIndexOf(tab);
}

void BraveBrowserTabStripController::ShowContextMenuForTab(
Tab* tab,
const gfx::Point& p,
Expand Down
3 changes: 3 additions & 0 deletions browser/ui/views/tabs/brave_browser_tab_strip_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define BRAVE_BROWSER_UI_VIEWS_TABS_BRAVE_BROWSER_TAB_STRIP_CONTROLLER_H_

#include <memory>
#include <optional>

#include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"

Expand All @@ -24,6 +25,8 @@ class BraveBrowserTabStripController : public BrowserTabStripController {
const BraveBrowserTabStripController&) = delete;
~BraveBrowserTabStripController() override;

const std::optional<int> GetModelIndexOf(Tab* tab);

// BrowserTabStripController overrides:
void ShowContextMenuForTab(Tab* tab,
const gfx::Point& p,
Expand Down
25 changes: 25 additions & 0 deletions browser/ui/views/tabs/brave_tab_context_menu_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ void BraveTabContextMenuContents::RunMenuAt(const gfx::Point& point,
}

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

if (command_id == BraveTabMenuModel::CommandShowVerticalTabs) {
return tabs::utils::ShouldShowVerticalTabs(browser_);
}
Expand All @@ -72,6 +76,11 @@ 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_)) {
return false;
}

if (IsBraveCommandId(command_id))
return IsBraveCommandIdEnabled(command_id);

Expand All @@ -80,6 +89,10 @@ bool BraveTabContextMenuContents::IsCommandIdEnabled(int command_id) const {
}

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

if (command_id == BraveTabMenuModel::CommandShowVerticalTabs) {
return tabs::utils::SupportsVerticalTabs(browser_);
}
Expand All @@ -90,6 +103,10 @@ bool BraveTabContextMenuContents::IsCommandIdVisible(int command_id) const {
bool BraveTabContextMenuContents::GetAcceleratorForCommandId(
int command_id,
ui::Accelerator* accelerator) const {
if (!controller_->GetModelIndexOf(tab_)) {
return false;
}

if (IsBraveCommandId(command_id))
return false;

Expand All @@ -103,6 +120,10 @@ bool BraveTabContextMenuContents::GetAcceleratorForCommandId(

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

if (IsBraveCommandId(command_id))
return ExecuteBraveCommand(command_id);

Expand All @@ -114,6 +135,8 @@ void BraveTabContextMenuContents::ExecuteCommand(int command_id,

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

switch (command_id) {
case BraveTabMenuModel::CommandRestoreTab:
return restore_service_ && (!restore_service_->IsLoaded() ||
Expand Down Expand Up @@ -143,6 +166,8 @@ bool BraveTabContextMenuContents::IsBraveCommandIdEnabled(
}

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

switch (command_id) {
case BraveTabMenuModel::CommandRestoreTab:
chrome::RestoreTab(browser_);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright (c) 2023 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_TABS_BROWSER_TAB_STRIP_CONTROLLER_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_TABS_BROWSER_TAB_STRIP_CONTROLLER_H_

#define CloseContextMenuForTesting \
CloseContextMenuForTesting_UnUsed() {} \
friend class BraveBrowserTabStripController; \
void CloseContextMenuForTesting

#include "src/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h" // IWYU pragma: export

#undef CloseContextMenuForTesting

#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_TABS_BROWSER_TAB_STRIP_CONTROLLER_H_