Reviewers: szager1
CL:
https://codereview.chromium.org/2437303002/Description:
Fix unscrollable scrollers after closing a <dialog>.
Inertness of a scroller can change without causing it to be laid out, if a
dialog is added or removed from Document::m_topLayerElements. Checking
inertness in PLSA::updateScrollableAreaSet led to stale compositing state.
This patch updates PLSA to ignore inertness. This means scrollers will remain
in FrameView::m_scrollableAreas even when covered by a dialog. That should be
ok, since the ::backdrop element prevents them from actually receiving input.
BUG=633520
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Affected files (+63, -6 lines):
A third_party/WebKit/LayoutTests/dialog/scrollable-after-close.html
A third_party/WebKit/LayoutTests/dialog/scrollable-after-close-expected.txt
M third_party/WebKit/Source/core/layout/LayoutObject.h
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
Index: third_party/WebKit/LayoutTests/dialog/scrollable-after-close-expected.txt
diff --git a/third_party/WebKit/LayoutTests/dialog/scrollable-after-close-expected.txt b/third_party/WebKit/LayoutTests/dialog/scrollable-after-close-expected.txt
new file mode 100644
index 0000000000000000000000000000000000000000..be0b87d9899e96f5409efbcfbb0c28043593e8a3
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/dialog/scrollable-after-close-expected.txt
@@ -0,0 +1,10 @@
+Tests that FrameView::m_scrollableAreas is updated after closing a modal dialog when the scroller and its ancestors have percentage heights. (see
crbug.com/633520)
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.numberOfScrollableAreas(document) is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Index: third_party/WebKit/LayoutTests/dialog/scrollable-after-close.html
diff --git a/third_party/WebKit/LayoutTests/dialog/scrollable-after-close.html b/third_party/WebKit/LayoutTests/dialog/scrollable-after-close.html
new file mode 100644
index 0000000000000000000000000000000000000000..567fba734960fbdd2527ed665e6b228441f7af0a
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/dialog/scrollable-after-close.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<script src="../resources/js-test.js"></script>
+<style>
+html, body { height: 100%; margin: 0; overflow: hidden; }
+#scroller {
+ overflow: scroll;
+ position: relative;
+ z-index: 0;
+ width: 300px;
+ height: 50%;
+ left: 20px;
+ top: 20px;
+ border: 1px solid black;
+}
+</style>
+<dialog></dialog>
+<div id="scroller">
+ <div style="height: 700px; width: 700px"></div>
+</div>
+<script>
+
+description("Tests that FrameView::m_scrollableAreas is updated after " +
+ "closing a modal dialog when the scroller and its ancestors " +
+ "have percentage heights. (see
crbug.com/633520)");
+
+var dialog = document.querySelector("dialog");
+dialog.showModal();
+dialog.close();
+
+// Force layout.
+document.body.offsetWidth;
+
+shouldBe("internals.numberOfScrollableAreas(document)", "1");
+
+</script>
Index: third_party/WebKit/Source/core/layout/LayoutObject.h
diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.h b/third_party/WebKit/Source/core/layout/LayoutObject.h
index c4cf35150e68a740cc9b2c61775d1e1e8db32475..f9f9b786a18b72edeb689dbc528b80b28c8b6be6 100644
--- a/third_party/WebKit/Source/core/layout/LayoutObject.h
+++ b/third_party/WebKit/Source/core/layout/LayoutObject.h
@@ -1505,9 +1505,17 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
!isInert();
}
- bool visibleToHitTesting() const {
+ enum VisibleToHitTestingBehavior {
+ VisibleToHitTestingDefault,
+ VisibleToHitTestingStyleRulesOnly
+ };
+ // Warning: VisibleToHitTestingDefault checks inertness, which can change
+ // without causing relayout of this LayoutObject.
+ bool visibleToHitTesting(
+ VisibleToHitTestingBehavior behavior = VisibleToHitTestingDefault) const {
return style()->visibility() == EVisibility::Visible &&
- style()->pointerEvents() != PE_NONE && !isInert();
+ style()->pointerEvents() != PE_NONE &&
+ (!isInert() || behavior == VisibleToHitTestingStyleRulesOnly);
}
// Map points and quads through elements, potentially via 3d transforms. You
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 57e8539ce33fbf0c3f98b9cbb7a7ff2308174a49..10f4452f94d059c8501c45d3c68cf3a7f29ddad5 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
@@ -1620,11 +1620,15 @@ void PaintLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow) {
if (!frameView)
return;
+ LayoutObject::VisibleToHitTestingBehavior behavior =
+ LayoutObject::VisibleToHitTestingStyleRulesOnly;
+
// FIXME: Does this need to be fixed later for OOPI?
- bool isVisibleToHitTest = box().visibleToHitTesting();
- if (HTMLFrameOwnerElement* owner = frame->deprecatedLocalOwner())
- isVisibleToHitTest &=
- owner->layoutObject() && owner->layoutObject()->visibleToHitTesting();
+ bool isVisibleToHitTest = box().visibleToHitTesting(behavior);
+ if (HTMLFrameOwnerElement* owner = frame->deprecatedLocalOwner()) {
+ isVisibleToHitTest &= owner->layoutObject() &&
+ owner->layoutObject()->visibleToHitTesting(behavior);
+ }
bool didScrollOverflow = m_scrollsOverflow;