Commit 6f0f67b3 authored by Adithya Srinivasan's avatar Adithya Srinivasan Committed by Commit Bot

Portals: Fix accessibility crash

Currently, after activation and adoption, both AX trees for the newly
activated page and the portal have their parent_tree_id set to each
other, which can lead to a stack overflow when trying to retrieve
the root BrowserAccessibilityManager. This CL breaks this cycle by
unsetting the parent_tree_id of the detached WebContents' AX tree.

Bug: 1033999
Change-Id: Ibd0b6f4f9af28df2795574728800438a7ec5dd12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972234Reviewed-by: default avatarLucas Gadani <lfg@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726194}
parent 4a15c894
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "content/public/browser/site_isolation_policy.h" #include "content/public/browser/site_isolation_policy.h"
#include "content/public/browser/web_contents_delegate.h" #include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "content/public/test/accessibility_notification_waiter.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h"
...@@ -1512,6 +1513,56 @@ IN_PROC_BROWSER_TEST_F(PortalBrowserTest, DidFocusIPCFromFrameInsidePortal) { ...@@ -1512,6 +1513,56 @@ IN_PROC_BROWSER_TEST_F(PortalBrowserTest, DidFocusIPCFromFrameInsidePortal) {
EXPECT_EQ(web_contents_impl->GetFocusedFrame(), main_frame); EXPECT_EQ(web_contents_impl->GetFocusedFrame(), main_frame);
} }
namespace {
void WaitForAccessibilityTree(WebContents* web_contents) {
AccessibilityNotificationWaiter waiter(web_contents, ui::kAXModeComplete,
ax::mojom::Event::kNone);
waiter.WaitForNotification();
}
} // namespace
IN_PROC_BROWSER_TEST_F(PortalBrowserTest,
RootAccessibilityManagerShouldUpdateAfterActivation) {
EXPECT_TRUE(NavigateToURL(
shell(), embedded_test_server()->GetURL("portal.test", "/title1.html")));
WebContentsImpl* web_contents_impl =
static_cast<WebContentsImpl*>(shell()->web_contents());
RenderFrameHostImpl* main_frame = web_contents_impl->GetMainFrame();
WaitForAccessibilityTree(web_contents_impl);
// Create portal.
GURL url = embedded_test_server()->GetURL("a.com", "/title1.html");
Portal* portal = CreatePortalToUrl(web_contents_impl, url);
WebContentsImpl* portal_contents = portal->GetPortalContents();
RenderFrameHostImpl* portal_frame = portal_contents->GetMainFrame();
WaitForAccessibilityTree(portal_contents);
EXPECT_EQ(portal_frame->browser_accessibility_manager()->GetRootManager(),
main_frame->browser_accessibility_manager());
// Activate portal and adopt predecessor.
EXPECT_TRUE(ExecJs(portal_frame,
"window.addEventListener('portalactivate', e => { "
" var portal = e.adoptPredecessor(); "
" document.body.appendChild(portal); "
"});"));
{
PortalActivatedObserver activated_observer(portal);
PortalCreatedObserver adoption_observer(portal_frame);
EXPECT_TRUE(ExecJs(main_frame,
"let portal = document.querySelector('portal');"
"portal.activate().then(() => { "
" document.body.removeChild(portal); "
"});"));
EXPECT_EQ(blink::mojom::PortalActivateResult::kPredecessorWasAdopted,
activated_observer.WaitForActivateResult());
adoption_observer.WaitUntilPortalCreated();
}
EXPECT_EQ(portal_frame->browser_accessibility_manager()->GetRootManager(),
portal_frame->browser_accessibility_manager());
}
class PortalOOPIFBrowserTest : public PortalBrowserTest { class PortalOOPIFBrowserTest : public PortalBrowserTest {
protected: protected:
PortalOOPIFBrowserTest() {} PortalOOPIFBrowserTest() {}
......
...@@ -1844,6 +1844,10 @@ std::unique_ptr<WebContents> WebContentsImpl::DetachFromOuterWebContents() { ...@@ -1844,6 +1844,10 @@ std::unique_ptr<WebContents> WebContentsImpl::DetachFromOuterWebContents() {
node_.SetFocusedWebContents(this); node_.SetFocusedWebContents(this);
CreateRenderWidgetHostViewForRenderManager(GetRenderViewHost()); CreateRenderWidgetHostViewForRenderManager(GetRenderViewHost());
RecursivelyRegisterFrameSinkIds(); RecursivelyRegisterFrameSinkIds();
// TODO(adithyas): |browser_plugin_embedder_ax_tree_id| should either not be
// used for portals, or it should get a different name.
GetMainFrame()->set_browser_plugin_embedder_ax_tree_id(ui::AXTreeIDUnknown());
GetMainFrame()->UpdateAXTreeData();
return web_contents; return web_contents;
} }
......
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