Commit 4b32d39c authored by Mohamed Mansour's avatar Mohamed Mansour Committed by Commit Bot

Reland 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 to the Reset to satisfy
all other cases where the Hung Pages Modal gets fired in:

 - RenderProcessExited
 - RenderWidgetHostDestroyed
 - TabDestroyed,
 - RenderViewHostChanged
 - WebContentsDestroyed

Bug: 1140454
Change-Id: I4e5b375acb351f5338faff3f30b65b38fc834818
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545050Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Mohamed Mansour <mmansour@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#829021}
parent 10e8b7bc
...@@ -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,11 @@ class HungRendererDialogView : public views::DialogDelegateView, ...@@ -155,6 +155,11 @@ 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_; }
HungPagesTableModel* table_model_for_testing() {
return hung_pages_table_model_.get();
}
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,34 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) { ...@@ -87,3 +88,34 @@ 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();
// The Hung Dialog requires window activation, window activations does not
// work reliably in browser tests, especially in Mac because of the activation
// policy for obtaining window key status is set to prohibited. Instead of
// showing the window, populate the table model instead.
dialog->table_model_for_testing()->InitForWebContents(
web_contents,
web_contents->GetMainFrame()->GetRenderViewHost()->GetWidget(),
base::DoNothing::Repeatedly());
// Makes sure the virtual accessibility views are in sync with the model when
// the dialog is created. Should consist of a single item.
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