Commit 1ac27c4a authored by David Bokan's avatar David Bokan Committed by Commit Bot

Fix rootScroller crash when swapping to OOPIF

This crash was occurring because a local frame is made the effective
rootScroller. Then, it is navigated to a cross origin domain and it
swaps in a remote frame. The code to handle this for rootScroller would
mark the document as needing layout - after which we'll notice the
rootScroller is no longer valid and replace it. However, before that
happens we might get a resize IPC which executes code in
RootScrollerController that assumes an iframe rootScroller is local.

The fix here is to just immediately recompute the effective
rootScroller when the iframe's FrameView is changed.

We also need to notify the RootScrollerController of updates to an
iframe's FrameView in more cases. Previously, we would early-out if the
iframe wasn't the effective root scroller. This would fail in some
situations. e.g. The iframe loses its FrameView which causes us to
recalculate and remove it as the effective root scroller. The iframe
then gets a new FrameView so we should recalculate again. This CL makes
it so we early out only if the iframe isn't the rootScroller or
implicit rootScroller.

Bug: 805298
Change-Id: Ibf5c46c9fea6016bfa376116dc1f18723fe2c2d1
Reviewed-on: https://chromium-review.googlesource.com/957791Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544344}
parent 51797648
......@@ -250,6 +250,8 @@ void HTMLFrameOwnerElement::SetEmbeddedContentView(
embedded_content_view_ = embedded_content_view;
FrameOwnerPropertiesChanged();
GetDocument().GetRootScrollerController().DidUpdateIFrameFrameView(*this);
LayoutEmbeddedContent* layout_embedded_content =
ToLayoutEmbeddedContent(GetLayoutObject());
if (!layout_embedded_content)
......@@ -269,8 +271,6 @@ void HTMLFrameOwnerElement::SetEmbeddedContentView(
embedded_content_view_->AttachToLayout();
}
GetDocument().GetRootScrollerController().DidUpdateIFrameFrameView(*this);
if (AXObjectCache* cache = GetDocument().ExistingAXObjectCache())
cache->ChildrenChanged(layout_embedded_content);
}
......
......@@ -111,14 +111,15 @@ void RootScrollerController::DidResizeFrameView() {
void RootScrollerController::DidUpdateIFrameFrameView(
HTMLFrameOwnerElement& element) {
if (&element != effective_root_scroller_.Get())
if (&element != root_scroller_.Get() && &element != implicit_root_scroller_)
return;
// Make sure we do a layout so we try to recalculate the effective root
// scroller. Ensure properties are applied even if the effective root
// scroller doesn't change.
// Reevaluate whether the iframe should be the effective root scroller (e.g.
// demote it if it became remote). Ensure properties are re-applied even if
// the effective root scroller doesn't change since the FrameView might have
// been swapped out.
needs_apply_properties_ = true;
document_->GetFrame()->View()->SetNeedsLayout();
RecomputeEffectiveRootScroller();
}
void RootScrollerController::RecomputeEffectiveRootScroller() {
......
......@@ -775,7 +775,7 @@ TEST_P(RootScrollerTest, RemoteIFrame) {
}
// Make sure that if an effective root scroller becomes a remote frame, it's
// demoted.
// immediately demoted.
TEST_P(RootScrollerTest, IFrameSwapToRemote) {
Initialize("root-scroller-iframe.html");
Element* iframe = MainFrame()->GetDocument()->getElementById("iframe");
......@@ -790,6 +790,9 @@ TEST_P(RootScrollerTest, IFrameSwapToRemote) {
// Swap in a remote frame. Make sure we revert back to the document.
{
MainWebFrame()->FirstChild()->Swap(FrameTestHelpers::CreateRemote());
EXPECT_EQ(MainFrame()->GetDocument(),
EffectiveRootScroller(MainFrame()->GetDocument()));
GetWebView()->ResizeWithBrowserControls(IntSize(400, 450), 50, 0, false);
MainFrameView()->UpdateAllLifecyclePhases();
EXPECT_EQ(MainFrame()->GetDocument(),
EffectiveRootScroller(MainFrame()->GetDocument()));
......
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