Reviewers: dtapuska
CL:
https://codereview.chromium.org/2365173002/https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.hFile third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
(right):
https://codereview.chromium.org/2365173002/diff/40001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h#newcode50third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h:50:
#include "core/page/scrolling/StickyPositionScrollingConstraints.h"
Unrelated but I noticed this is needed to include this file in new
places. It's current uses just happen to have it via other includes.
Description:
Detach PaintLayerScrollableArea from RootFrameViewport when disposed.
With document.rootScroller, a PLSA can become the layoutViewport in
RootFrameViewport. This crash was happening because the associated Node being
removed from the DOM causes deletion of the associated LayoutObject and
PaintLayer but the RootFrameViewport still has a pointer to the PLSA. The
RootScrollerController will realize the rootScroller's LayoutObject is gone
during the next layout but we can call into the dead PLSA (which could have
been garbage collected in the mean time).
This fix checks during PLSA disposal whether it's registered as the layout
viewport, and if so, resets the layout viewport to the FrameView.
BUG=649340
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Affected files (+62, -1 lines):
M third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
M third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
M third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
Index: third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
diff --git a/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp b/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
index 6dfa436619f353c1f553182fe0ce08030e4a2ecb..0ad1b19f5136105c706ca6c7ba37c934ca1a02ac 100644
--- a/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
+++ b/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.cpp
@@ -88,6 +88,16 @@ void RootScrollerController::didUpdateLayout()
recomputeEffectiveRootScroller();
}
+void RootScrollerController::didDisposePaintLayerScrollableArea(
+ PaintLayerScrollableArea& area)
+{
+ if (!m_effectiveRootScroller)
+ return;
+
+ if (&area.box() == m_effectiveRootScroller->layoutObject())
+ recomputeEffectiveRootScroller();
+}
+
void RootScrollerController::recomputeEffectiveRootScroller()
{
bool rootScrollerValid =
@@ -150,8 +160,11 @@ PaintLayer* RootScrollerController::rootScrollerPaintLayer() const
// PaintLayer (i.e. the PaintLayerCompositor's root layer). The reason the root
// scroller is the <html> layer and not #document is because the latter is a Node
// but not an Element.
- if (m_effectiveRootScroller->isSameNode(m_document->documentElement()))
+ if (m_effectiveRootScroller->isSameNode(m_document->documentElement())) {
+ if (!layer || !layer->compositor())
+ return nullptr;
return layer->compositor()->rootLayer();
+ }
return layer;
}
Index: third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
diff --git a/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h b/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
index b6bd17714142d1441bfac04689cc94c65831eca1..a293942104a35b6a0819ffe9ea77b1457220a747 100644
--- a/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
+++ b/third_party/WebKit/Source/core/page/scrolling/RootScrollerController.h
@@ -14,6 +14,7 @@ class Document;
class Element;
class GraphicsLayer;
class PaintLayer;
+class PaintLayerScrollableArea;
class ScrollableArea;
class ScrollStateCallback;
@@ -70,6 +71,10 @@ public:
// replaced by the default root scroller.
void didUpdateLayout();
+ // PaintLayerScrollableAreas need to notify this class when they're being
+ // disposed so that we can remove them as the root scroller.
+ void didDisposePaintLayerScrollableArea(PaintLayerScrollableArea&);
+
// Returns the PaintLayer associated with the currently effective root
// scroller.
PaintLayer* rootScrollerPaintLayer() const;
Index: third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
index ab70822b20994b38efc5be6a20d2a373e360672f..f572ff2bcfa033518902f60fc776a7b44a1cd743 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
@@ -52,6 +52,7 @@
#include "core/frame/FrameHost.h"
#include "core/frame/FrameView.h"
#include "core/frame/LocalFrame.h"
+#include "core/frame/RootFrameViewport.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLFrameOwnerElement.h"
#include "core/input/EventHandler.h"
@@ -67,6 +68,7 @@
#include "core/page/ChromeClient.h"
#include "core/page/FocusController.h"
#include "core/page/Page.h"
+#include "core/page/scrolling/RootScrollerController.h"
#include "core/page/scrolling/ScrollingCoordinator.h"
#include "core/paint/PaintLayerFragment.h"
#include "platform/PlatformGestureEvent.h"
@@ -156,6 +158,9 @@ void PaintLayerScrollableArea::dispose()
frameView->removeResizerArea(box());
}
+ if (RootScrollerController* controller = box().document().rootScrollerController())
+ controller->didDisposePaintLayerScrollableArea(*this);
+
m_scrollbarManager.dispose();
if (m_scrollCorner)
Index: third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
diff --git a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
index aba96399d77ea7e0b1b02e469e6a6f0eea11467d..611f278f02a8f9ee5a75b00fed1c73d22ea1209b 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
+++ b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
@@ -47,6 +47,7 @@
#include "core/CoreExport.h"
#include "core/layout/ScrollAnchor.h"
#include "core/layout/ScrollEnums.h"
+#include "core/page/scrolling/StickyPositionScrollingConstraints.h"
#include "core/paint/PaintInvalidationCapableScrollableArea.h"
#include "core/paint/PaintLayerFragment.h"
#include "platform/heap/Handle.h"
Index: third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp b/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
index 101e7151c738d80d0dbeacfd613849624cfc397c..0ae0336731114da462a5a57ceaed6946c7d5a759 100644
--- a/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
+++ b/third_party/WebKit/Source/web/tests/RootScrollerTest.cpp
@@ -4,6 +4,7 @@
#include "core/frame/FrameHost.h"
#include "core/frame/FrameView.h"
+#include "core/frame/RootFrameViewport.h"
#include "core/frame/TopControls.h"
#include "core/html/HTMLFrameOwnerElement.h"
#include "core/layout/LayoutBox.h"
@@ -950,6 +951,42 @@ TEST_F(RootScrollerTest, RemoveClippingOnCompositorLayers)
}
}
+// Tests that removing the root scroller element from the DOM resets the
+// effective root scroller without waiting for any lifecycle events.
+TEST_F(RootScrollerTest, RemoveRootScrollerFromDom)
+{
+ initialize("root-scroller-iframe.html");
+
+ {
+ HTMLFrameOwnerElement* iframe = toHTMLFrameOwnerElement(
+ mainFrame()->document()->getElementById("iframe"));
+ Element* innerContainer =
+ iframe->contentDocument()->getElementById("container");
+
+ NonThrowableExceptionState exceptionState;
+ mainFrame()->document()->setRootScroller(iframe, exceptionState);
+ iframe->contentDocument()->setRootScroller(
+ innerContainer, exceptionState);
+ mainFrameView()->updateAllLifecyclePhases();
+
+ ASSERT_EQ(iframe, mainFrame()->document()->rootScroller());
+ ASSERT_EQ(iframe, effectiveRootScroller(mainFrame()->document()));
+ ASSERT_EQ(innerContainer, iframe->contentDocument()->rootScroller());
+ ASSERT_EQ(innerContainer,
+ effectiveRootScroller(iframe->contentDocument()));
+
+ iframe->contentDocument()->body()->setInnerHTML("", exceptionState);
+
+ // If the root scroller wasn't updated by the DOM removal above, this
+ // will touch the disposed root scroller's ScrollableArea.
+ mainFrameView()->getRootFrameViewport()->serviceScrollAnimations(0);
+
+ EXPECT_EQ(iframe->contentDocument()->documentElement(),
+ effectiveRootScroller(iframe->contentDocument()));
+ }
+}
+
+
} // namespace
} // namespace blink