Commit ff8a3185 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Chromium LUCI CQ

Fix crash in RequestAssistantStructureForActiveBrowserWindow

This crash has been happening quite frequently since this summer
(see crbug.com/1128031).

The crash is at line 41:
  assistant_extra->url = web_contents->GetLastCommittedURL();
Given that we just created |assistant_extra| the line above, it must be
that |web_contents| is invalid.

|web_contents| is created at line 94:
  content::WebContents* web_contents =
      browser->tab_strip_model()->GetActiveWebContents();

It is surprising that this can be nullptr because we check at line 52
to ensure the browser is active (so you'd assume it has active web
content).
But irregardless, I added a null check after line 94.

Bug: 1128031
Test: N/A
Change-Id: Iaabdedf5bb133d608db75a16e96907cc18a0d960
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601276
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839458}
parent 8e639a98
......@@ -44,6 +44,19 @@ ax::mojom::AssistantExtraPtr CreateAssistantExtra(
return assistant_extra;
}
gfx::Rect GetWindowBoundsInPixels(const Browser* browser) {
gfx::Rect bounds = browser->window()->GetBounds();
gfx::Point top_left = bounds.origin();
gfx::Point bottom_right = bounds.bottom_right();
const aura::Window* window = browser->window()->GetNativeWindow();
const auto* window_tree_host = window->GetRootWindow()->GetHost();
// TODO: Revisit once multi-monitor support is planned.
window_tree_host->ConvertDIPToScreenInPixels(&top_left);
window_tree_host->ConvertDIPToScreenInPixels(&bottom_right);
return gfx::Rect(top_left.x(), top_left.y(), bottom_right.x() - top_left.x(),
bottom_right.y() - top_left.y());
}
} // namespace
void RequestAssistantStructureForActiveBrowserWindow(
......@@ -77,26 +90,22 @@ void RequestAssistantStructureForActiveBrowserWindow(
return;
}
content::WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents();
if (!web_contents) {
std::move(callback).Run(nullptr, nullptr);
return;
}
// We follow same convention as Clank and thus the contents are all in
// pixels. The bounds of the window need to be converted to pixel in order
// to be consistent with rest of the view hierarchy.
gfx::Rect bounds = browser->window()->GetBounds();
gfx::Point top_left = bounds.origin();
gfx::Point bottom_right = bounds.bottom_right();
auto* window_tree_host = window->GetRootWindow()->GetHost();
// TODO: Revisit once multi-monitor support is planned.
window_tree_host->ConvertDIPToScreenInPixels(&top_left);
window_tree_host->ConvertDIPToScreenInPixels(&bottom_right);
gfx::Rect window_bounds = GetWindowBoundsInPixels(browser);
content::WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents();
web_contents->RequestAXTreeSnapshot(
base::BindOnce(
&CreateAssistantStructureAndRunCallback, std::move(callback),
CreateAssistantExtra(web_contents,
gfx::Rect(top_left.x(), top_left.y(),
bottom_right.x() - top_left.x(),
bottom_right.y() - top_left.y()))),
base::BindOnce(&CreateAssistantStructureAndRunCallback,
std::move(callback),
CreateAssistantExtra(web_contents, window_bounds)),
ui::kAXModeComplete);
}
......
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