Commit fc8af8e6 authored by Mohamed Mansour's avatar Mohamed Mansour Committed by Commit Bot

Fix Crash in Hung Pages view while in screen reader

There seems to be a timing issue between the AX queries for
GetText and when the model is updating. Looking at the model
it seems not all TabDestroyed calls are informing the Observers
that the model has changed.

The TableView uses Virtual AX Children that depends on the
observers to call. Moved the observer call from TabDestroyed to Reset
to handle all delegate_->TabDestroyed() calls defined below:

 - RenderProcessExited
 - RenderWidgetHostDestroyed
 - TabDestroyed,
 - RenderViewHostChanged
 - WebContentsDestroyed

Bug: 1140454
Change-Id: Ie7fd07d6540b9509b998d5cd95d1fd16f47f9018
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530295Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Mohamed Mansour <mmansour@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#827900}
parent 186022c3
...@@ -106,6 +106,10 @@ void HungPagesTableModel::Reset() { ...@@ -106,6 +106,10 @@ void HungPagesTableModel::Reset() {
widget_observer_.RemoveAll(); widget_observer_.RemoveAll();
tab_observers_.clear(); tab_observers_.clear();
render_widget_host_ = nullptr; render_widget_host_ = nullptr;
// Inform the table model observers that we cleared the model.
if (observer_)
observer_->OnModelChanged();
} }
void HungPagesTableModel::RestartHangMonitorTimeout() { void HungPagesTableModel::RestartHangMonitorTimeout() {
......
...@@ -155,6 +155,8 @@ class HungRendererDialogView : public views::DialogDelegateView, ...@@ -155,6 +155,8 @@ class HungRendererDialogView : public views::DialogDelegateView,
// Returns true if the frame is in the foreground. // Returns true if the frame is in the foreground.
static bool IsFrameActive(content::WebContents* contents); static bool IsFrameActive(content::WebContents* contents);
views::TableView* table_for_testing() { return hung_pages_table_; }
virtual void ShowForWebContents( virtual void ShowForWebContents(
content::WebContents* contents, content::WebContents* contents,
content::RenderWidgetHost* render_widget_host, content::RenderWidgetHost* render_widget_host,
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "content/public/browser/web_contents_delegate.h" #include "content/public/browser/web_contents_delegate.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "ui/views/accessibility/view_accessibility.h"
// Interactive UI tests for the hung renderer (aka page unresponsive) dialog. // Interactive UI tests for the hung renderer (aka page unresponsive) dialog.
class HungRendererDialogViewBrowserTest : public DialogBrowserTest { class HungRendererDialogViewBrowserTest : public DialogBrowserTest {
...@@ -77,7 +78,7 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) { ...@@ -77,7 +78,7 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) {
// This is what happens when HungRendererDialogView::ShowForWebContents // This is what happens when HungRendererDialogView::ShowForWebContents
// returns early if the frame or the dialog are not active. // returns early if the frame or the dialog are not active.
HungRendererDialogView::Create(browser()->window()->GetNativeWindow()); HungRendererDialogView::Create(browser()->window()->GetNativeWindow());
EXPECT_TRUE(HungRendererDialogView::GetInstance()); ASSERT_TRUE(HungRendererDialogView::GetInstance());
// Simulate the renderer becoming responsive again. // Simulate the renderer becoming responsive again.
content::WebContents* web_contents = content::WebContents* web_contents =
...@@ -87,3 +88,27 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) { ...@@ -87,3 +88,27 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) {
content::WebContentsDelegate* web_contents_delegate = browser(); content::WebContentsDelegate* web_contents_delegate = browser();
web_contents_delegate->RendererResponsive(web_contents, render_widget_host); web_contents_delegate->RendererResponsive(web_contents, render_widget_host);
} }
IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, ProcessClosed) {
HungRendererDialogView* dialog =
HungRendererDialogView::Create(browser()->window()->GetNativeWindow());
ASSERT_TRUE(dialog);
auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
dialog->ShowForWebContents(
web_contents,
web_contents->GetMainFrame()->GetRenderViewHost()->GetWidget(),
base::DoNothing::Repeatedly());
views::ViewAccessibility& view_accessibility =
dialog->table_for_testing()->GetViewAccessibility();
EXPECT_EQ(size_t{1}, view_accessibility.virtual_children().size());
// Simulate an abrupt ending to webcontents. The accessibility tree for the
// TableView depends on the Hung Render View table model to send the right
// events. By checking the virtual tree, we can guarantee the sync is correct.
dialog->EndForWebContents(
web_contents,
web_contents->GetMainFrame()->GetRenderViewHost()->GetWidget());
EXPECT_EQ(size_t{0}, view_accessibility.virtual_children().size());
}
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