Bug 1634763 - Ensure APZ knows about inactive scrollframes that may have active scrollbars. r=tnikkel

Differential Revision: https://phabricator.services.mozilla.com/D74789
This commit is contained in:
Kartikaya Gupta 2020-05-12 02:02:51 +00:00
Родитель 7d9078bede
Коммит 6c5adadaad
3 изменённых файлов: 91 добавлений и 0 удалений

Просмотреть файл

@ -0,0 +1,69 @@
<!DOCTYPE HTML>
<html>
<head>
<title>Hit-test the scrollbar of an inactive will-change scrollframe</title>
<script type="application/javascript" src="apz_test_utils.js"></script>
<script type="application/javascript" src="apz_test_native_event_utils.js"></script>
<script src="/tests/SimpleTest/paint_listener.js"></script>
<meta name="viewport" content="width=device-width"/>
</head>
<body>
<div id="scroller" style="width: 300px; height: 100px; will-change: scroll-position; overflow-y: auto">
<div style="height: 1000px">
This div is filler to make the enclosing scroller div scrollable, but I'm
taking the opportunity to explain the test in more detail here.
The will-change property on the scroller is intended to make the
ScrollFrameHelper::IsMaybeScrollingActive() function return true,
which makes the scrollbar data contain the scroller's actual scrolltarget
(instead of NULL_SCROLL_ID). However, since there's no displayport
set on the scroller, it ends up with mWillBuildScrollableLayer set to
false and is not actually scrollable by APZ.
This means that with WR enabled, APZ never gets the metrics for the
scroller. Upon hit-testing the scrollthumb with WR enabled, it gets the
scroll id of the scroller, but doesn't have the metrics for it, resulting
in an assertion failure.
</div>
</div>
<div id="make_root_scrollable" style="height: 5000px"></div>
</body>
<script type="application/javascript">
function* test(testDriver) {
var config = getHitTestConfig();
var utils = config.utils;
var scroller = document.getElementById("scroller");
checkHitResult(hitTest(centerOf(scroller)),
APZHitResultFlags.VISIBLE |
(config.isWebRender ? APZHitResultFlags.INACTIVE_SCROLLFRAME
: APZHitResultFlags.IRREGULAR_AREA),
(config.isWebRender ? utils.getViewId(document.scrollingElement)
: utils.getViewId(scroller)),
utils.getLayersId(),
"inactive will-change scrollframe");
// The above check ensures that the scroller is *inactive*, but because of
// the will-change property on it, the scrollbars get layerized as though
// the scroller is *active*, hence we provide the scroller's viewId and
// LayerState.ACTIVE as the expected checks below.
hitTestScrollbar({
element: scroller,
directions: { vertical: true, horizontal: false },
expectedScrollId: utils.getViewId(scroller),
expectedLayersId: utils.getLayersId(),
trackLocation: ScrollbarTrackLocation.START,
expectThumb: true,
layerState: LayerState.ACTIVE,
});
subtestDone();
}
waitUntilApzStable().then(runContinuation(test));
</script>
</html>

Просмотреть файл

@ -41,6 +41,7 @@ var subtests = [
{"file": "helper_hittest_clippath.html", "prefs": prefs},
{"file": "helper_hittest_hoisted_scrollinfo.html", "prefs": prefs},
{"file": "helper_hittest_spam.html", "prefs": prefs},
{"file": "helper_hittest_scrollbar_of_inactive_scrollframe.html", "prefs": prefs},
];
if (isApzEnabled()) {

Просмотреть файл

@ -3788,6 +3788,21 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
// Make sure that APZ will dispatch events back to content so we can
// create a displayport for this frame. We'll add the item later on.
if (!mWillBuildScrollableLayer) {
// The scrollframe is inactive, but if IsMaybeScrollingActive() returns
// true, the scrollbars may have been layerized and may refer to the
// scrollId of this scrollframe. In this situation, we need to send a
// scrollinfo layer to APZ, so that it has some knowledge of this
// scrollframe, rather than potentially getting a hit-test result with a
// scrollId it knows nothing about (which can cause assertion failures in
// a debug WR-enabled configuration).
bool provideScrollInfoForAPZ = IsMaybeScrollingActive();
if (aBuilder->ShouldBuildScrollInfoItemsForHoisting()) {
// If this condition is true, then we'll be including a scrollinfo item
// anyway below, and we shouldn't do it twice (runs afoul of display
// item uniqueness checks).
provideScrollInfoForAPZ = false;
}
if (aBuilder->BuildCompositorHitTestInfo()) {
nsDisplayCompositorHitTestInfo* hitInfo =
MakeDisplayItemWithIndex<nsDisplayCompositorHitTestInfo>(
@ -3796,6 +3811,12 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
AppendInternalItemToTop(scrolledContent, hitInfo, Some(INT32_MAX));
}
}
if (provideScrollInfoForAPZ) {
nsDisplayScrollInfoLayer* scrollInfo =
MakeDisplayItem<nsDisplayScrollInfoLayer>(aBuilder, mScrolledFrame,
mOuter, info, area);
AppendInternalItemToTop(scrolledContent, scrollInfo, Nothing());
}
}
if (aBuilder->ShouldBuildScrollInfoItemsForHoisting()) {