Commit 9ccd1018 authored by John Lee's avatar John Lee Committed by Commit Bot

WebUI Tab Strip: Remove invalid tab when opening context menu

This is a temporary solution to prevent crashes that occur from when a
tab's context menu is opened from a tab that is no longer in the tab
strip. The tab should be visible in another window's tab strip already.

Bug: 1141573
Change-Id: I211bf3e3c31b20d61fd924f74471a40c26f5aa45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511332
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823238}
parent 93c3ab71
...@@ -761,7 +761,14 @@ void TabStripUIHandler::HandleShowTabContextMenu(const base::ListValue* args) { ...@@ -761,7 +761,14 @@ void TabStripUIHandler::HandleShowTabContextMenu(const base::ListValue* args) {
tab_id, browser_->profile(), true /* include_incognito */, &browser, tab_id, browser_->profile(), true /* include_incognito */, &browser,
nullptr, nullptr, &tab_index); nullptr, nullptr, &tab_index);
CHECK(got_tab); CHECK(got_tab);
CHECK_EQ(browser, browser_);
if (browser != browser_) {
// TODO(crbug.com/1141573): Investigate how a context menu is being opened
// for a tab that is no longer in the tab strip. Until then, fire a
// tab-removed event so the tab is removed from this tab strip.
FireWebUIListener("tab-removed", base::Value(tab_id));
return;
}
DCHECK(embedder_); DCHECK(embedder_);
embedder_->ShowContextMenuAtPoint( embedder_->ShowContextMenuAtPoint(
......
...@@ -60,6 +60,8 @@ class TabStripUIHandler : public content::WebUIMessageHandler, ...@@ -60,6 +60,8 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTab); FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTab);
FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTabAcrossProfiles); FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTabAcrossProfiles);
FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTabAcrossWindows); FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, MoveTabAcrossWindows);
FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest,
RemoveTabIfInvalidContextMenu);
FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, UngroupTab); FRIEND_TEST_ALL_PREFIXES(TabStripUIHandlerTest, UngroupTab);
void HandleCreateNewTab(const base::ListValue* args); void HandleCreateNewTab(const base::ListValue* args);
......
...@@ -549,3 +549,31 @@ TEST_F(TabStripUIHandlerTest, CloseTab) { ...@@ -549,3 +549,31 @@ TEST_F(TabStripUIHandlerTest, CloseTab) {
ASSERT_EQ(1, browser()->tab_strip_model()->GetTabCount()); ASSERT_EQ(1, browser()->tab_strip_model()->GetTabCount());
} }
TEST_F(TabStripUIHandlerTest, RemoveTabIfInvalidContextMenu) {
AddTab(browser(), GURL("http://foo"));
std::unique_ptr<BrowserWindow> new_window(CreateBrowserWindow());
std::unique_ptr<Browser> new_browser =
CreateBrowser(profile(), browser()->type(), false, new_window.get());
AddTab(new_browser.get(), GURL("http://bar"));
web_ui()->ClearTrackedCalls();
base::ListValue args;
args.AppendInteger(extensions::ExtensionTabUtil::GetTabId(
new_browser->tab_strip_model()->GetWebContentsAt(0)));
args.AppendDouble(50);
args.AppendDouble(100);
handler()->HandleShowTabContextMenu(&args);
const content::TestWebUI::CallData& call_data = *web_ui()->call_data().back();
EXPECT_EQ("cr.webUIListenerCallback", call_data.function_name());
EXPECT_EQ("tab-removed", call_data.arg1()->GetString());
EXPECT_EQ(extensions::ExtensionTabUtil::GetTabId(
new_browser->tab_strip_model()->GetWebContentsAt(0)),
call_data.arg2()->GetInt());
// Close all tabs before destructing.
new_browser.get()->tab_strip_model()->CloseAllTabs();
}
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