Skip to content

GitLab

T
tangled
Commit 39702f57 authored by David Bokan's avatar David Bokan Committed by Commit Bot
Browse files
Options

Fix crash in RootScrollerController 

It looks like we sometimes recompute the root scroller on an
HTMLFrameOwnerElement that's still connected to the DOM tree but doesn't
have a ContentFrame. One way this can happen is through layouts caused
from unload handlers.

This CL adds an early out in ApplyRootScrollerProperties since a
detached frame doesn't need it's properties reset.

Bug: 805317
Change-Id: I6d285b10a69f74f05604bda5f654fca96fc35238
Reviewed-on: https://chromium-review.googlesource.com/887733
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532971}
parent e4c53ce3
Hide whitespace changes
Inline Side-by-side
Showing with 98 additions and 19 deletions
third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
View file @ 39702f57
...@@ -221,26 +221,32 @@ void RootScrollerController::ApplyRootScrollerProperties(Node& node) { ...@@ -221,26 +221,32 @@ void RootScrollerController::ApplyRootScrollerProperties(Node& node) {
if (!node.IsInTreeScope()) if (!node.IsInTreeScope())
return; return;
if (node.IsFrameOwnerElement()) { if (!node.IsFrameOwnerElement())
HTMLFrameOwnerElement* frame_owner = ToHTMLFrameOwnerElement(&node); return;
DCHECK(frame_owner->ContentFrame());
HTMLFrameOwnerElement* frame_owner = ToHTMLFrameOwnerElement(&node);
if (frame_owner->ContentFrame()->IsLocalFrame()) {
LocalFrameView* frame_view = // The current effective root scroller may have lost its ContentFrame. If
ToLocalFrameView(frame_owner->OwnedEmbeddedContentView()); // that's the case, there's nothing to be done. https://crbug.com/805317 for
// an example of how we get here.
bool is_root_scroller = &EffectiveRootScroller() == &node; if (!frame_owner->ContentFrame())
return;
// If we're making the Frame the root scroller, it must have a FrameView
// by now. if (frame_owner->ContentFrame()->IsLocalFrame()) {
DCHECK(frame_view || !is_root_scroller); LocalFrameView* frame_view =
if (frame_view) { ToLocalFrameView(frame_owner->OwnedEmbeddedContentView());
frame_view->SetLayoutSizeFixedToFrameSize(!is_root_scroller);
UpdateIFrameGeometryAndLayoutSize(*frame_owner); bool is_root_scroller = &EffectiveRootScroller() == &node;
}
} else { // If we're making the Frame the root scroller, it must have a FrameView
// TODO(bokan): Make work with OOPIF. crbug.com/642378. // by now.
DCHECK(frame_view || !is_root_scroller);
if (frame_view) {
frame_view->SetLayoutSizeFixedToFrameSize(!is_root_scroller);
UpdateIFrameGeometryAndLayoutSize(*frame_owner);
} }
} else {
// TODO(bokan): Make work with OOPIF. crbug.com/642378.
} }
} }
......
third_party/WebKit/Source/core/page/scrolling/RootScrollerTest.cpp
View file @ 39702f57
...@@ -1449,6 +1449,79 @@ TEST_P(RootScrollerSimTest, ImplicitRootScrollerIframe) { ...@@ -1449,6 +1449,79 @@ TEST_P(RootScrollerSimTest, ImplicitRootScrollerIframe) {
GetDocument().GetRootScrollerController().EffectiveRootScroller()); GetDocument().GetRootScrollerController().EffectiveRootScroller());
} }
// Tests that we don't explode when a layout occurs and the effective
// rootScroller no longer has a ContentFrame(). We setup the frame tree such
// that the first iframe is the effective root scroller. The second iframe has
// an unload handler that reaches back to the common parent and causes a
// layout. This will cause us to recalculate the effective root scroller while
// the current one is valid in all ways except that it no longer has a content
// frame. This test passes if it doesn't crash. https://crbug.com/805317.
TEST_P(RootScrollerSimTest, RecomputeEffectiveWithNoContentFrame) {
WebView().Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
SimRequest first_request("https://example.com/first.html", "text/html");
SimRequest second_request("https://example.com/second.html", "text/html");
SimRequest final_request("https://newdomain.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
body, html {
width: 100%;
height: 100%;
margin: 0px;
}
iframe {
width: 100%;
height: 100%;
border: 0;
}
</style>
<iframe id="first" src="https://example.com/first.html">
</iframe>
<iframe id="second" src="https://example.com/second.html">
</iframe>
<script>
// Dirty layout on unload
window.addEventListener('unload', function() {
document.getElementById("first").style.width="0";
});
</script>
)HTML");
first_request.Complete(R"HTML(
<!DOCTYPE html>
)HTML");
second_request.Complete(R"HTML(
<!DOCTYPE html>
<body></body>
<script>
window.addEventListener('unload', function() {
// This will do a layout.
window.top.document.getElementById("first").clientWidth;
});
</script>
)HTML");
Compositor().BeginFrame();
Element* container = GetDocument().getElementById("first");
GetDocument().GetRootScrollerController().Set(container);
ASSERT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// This will unload first the root, then the first frame, then the second.
LoadURL("https://newdomain.com/test.html");
final_request.Complete(R"HTML(
<!DOCTYPE html>
)HTML");
}
class RootScrollerHitTest : public RootScrollerTest { class RootScrollerHitTest : public RootScrollerTest {
public: public:
void CheckHitTestAtBottomOfScreen() { void CheckHitTestAtBottomOfScreen() {
......
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