Commit 9c0d689b authored by Alan Cutter's avatar Alan Cutter Committed by Chromium LUCI CQ

desktop-paws: Fix crash when middle clicking in scope links with link capturing

This CL fixes middle clicks on links inside a link capturing web app
scope from crashing. These were crashing due to the expectation that
the navigating tab was already attached to a browser window (which
is incorrect during the navigation throttle) causing a nullptr crash.

We should not be link capturing these navigations so this CL checks
for and early exits in these cases.

Bug: 1159666
Change-Id: I9dae80d84a44e8bf2fa9918ece8fee6d19af2ee2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2636009
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarMaggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844666}
parent 22a07802
......@@ -187,6 +187,11 @@ AppsNavigationThrottle::CaptureWebAppScopeNavigations(
}
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
if (!browser) {
// This is a middle click open in new tab action; do not capture.
return base::nullopt;
}
if (web_app::AppBrowserController::IsForWebApp(browser, *app_id)) {
// Already in the app window; navigation already captured.
return base::nullopt;
......
......@@ -112,7 +112,8 @@ void WebAppNavigationBrowserTest::ClickLinkWithModifiersAndWaitForURL(
const GURL& target_url,
WebAppNavigationBrowserTest::LinkTarget target,
const std::string& rel,
int modifiers) {
int modifiers,
blink::WebMouseEvent::Button button) {
auto observer = GetTestNavigationObserver(target_url);
std::string script = base::StringPrintf(
"(() => {"
......@@ -134,8 +135,7 @@ void WebAppNavigationBrowserTest::ClickLinkWithModifiersAndWaitForURL(
rel.c_str());
ASSERT_TRUE(content::ExecuteScript(web_contents, script));
content::SimulateMouseClick(web_contents, modifiers,
blink::WebMouseEvent::Button::kLeft);
content::SimulateMouseClick(web_contents, modifiers, button);
observer->Wait();
}
......
......@@ -61,7 +61,9 @@ class WebAppNavigationBrowserTest : public InProcessBrowserTest {
const GURL& target_url,
LinkTarget target,
const std::string& rel,
int modifiers);
int modifiers,
blink::WebMouseEvent::Button button =
blink::WebMouseEvent::Button::kLeft);
// Creates an <a> element, sets its href and target to |link_url| and |target|
// respectively, adds it to the DOM, and clicks on it. Returns once
......
......@@ -144,9 +144,17 @@ IN_PROC_BROWSER_TEST_F(WebAppTabStripLinkCapturingBrowserTest,
ExpectTabs(browser(), {out_of_scope_});
ExpectTabs(app_browser, {in_scope_1_, in_scope_2_, scope_});
// Middle clicking links should not be captured.
ClickLinkWithModifiersAndWaitForURL(
browser()->tab_strip_model()->GetActiveWebContents(), scope_, scope_,
LinkTarget::SELF, "", blink::WebInputEvent::Modifiers::kNoModifiers,
blink::WebMouseEvent::Button::kMiddle);
ExpectTabs(browser(), {out_of_scope_, scope_});
ExpectTabs(app_browser, {in_scope_1_, in_scope_2_, scope_});
// Out of scope should behave as usual.
Navigate(browser(), out_of_scope_);
ExpectTabs(browser(), {out_of_scope_});
ExpectTabs(browser(), {out_of_scope_, scope_});
ExpectTabs(app_browser, {in_scope_1_, in_scope_2_, scope_});
}
......
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