Commit eece779c authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

[Sheriff] Revert "Fix Crash in Hung Pages view while in screen reader"

This reverts commit fc8af8e6.

Reason for revert: Crashing on MAC bots https://ci.chromium.org/p/chromium/builders/ci/Mac10.15%20Tests/5816

Original change's description:
> 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/+/2530295
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Mohamed Mansour <mmansour@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#827900}

TBR=sky@chromium.org,tapted@chromium.org,robliao@chromium.org,dfried@chromium.org,mmansour@microsoft.com

Change-Id: Ifb73fc1762d043fbba1d3a3a8b5744c169b9a88b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1140454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543191Reviewed-by: default avatarThomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827977}
parent fe730ce5
...@@ -106,10 +106,6 @@ void HungPagesTableModel::Reset() { ...@@ -106,10 +106,6 @@ 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,8 +155,6 @@ class HungRendererDialogView : public views::DialogDelegateView, ...@@ -155,8 +155,6 @@ 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,7 +21,6 @@ ...@@ -21,7 +21,6 @@
#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 {
...@@ -78,7 +77,7 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) { ...@@ -78,7 +77,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());
ASSERT_TRUE(HungRendererDialogView::GetInstance()); EXPECT_TRUE(HungRendererDialogView::GetInstance());
// Simulate the renderer becoming responsive again. // Simulate the renderer becoming responsive again.
content::WebContents* web_contents = content::WebContents* web_contents =
...@@ -88,27 +87,3 @@ IN_PROC_BROWSER_TEST_F(HungRendererDialogViewBrowserTest, InactiveWindow) { ...@@ -88,27 +87,3 @@ 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