Skip to content

Commit

Permalink
Create scroll properties for main thread scrolling even w/o scrolling
Browse files Browse the repository at this point in the history
If an object with overflow scrolling does not actually scroll, we would not
create a scroll paint property node before this patch. This was wrong because
a paint property node may be needed for main thread scrolling reasons.

BUG=671419
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2553093003
Cr-Commit-Position: refs/heads/master@{#436833}
  • Loading branch information
progers authored and Commit bot committed Dec 7, 2016
1 parent 2d053e7 commit 132d936
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 25 deletions.
55 changes: 30 additions & 25 deletions third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,15 +640,34 @@ void PaintPropertyTreeBuilder::updateSvgLocalToBorderBoxTransform(
context.current.paintOffset = LayoutPoint();
}

MainThreadScrollingReasons mainScrollingReasons(const LayoutObject& object) {
MainThreadScrollingReasons reasons = 0;
if (!object.document().settings()->threadedScrollingEnabled())
reasons |= MainThreadScrollingReason::kThreadedScrollingDisabled;
// Checking for descendants in the layout tree has two downsides:
// 1) There can be more descendants in layout order than in paint order (e.g.,
// fixed position objects).
// 2) Iterating overall all background attachment fixed objects for every
// scroll node can be slow, though there will be none in the common case.
const FrameView& frameView = *object.frameView();
if (frameView.hasBackgroundAttachmentFixedDescendants(object))
reasons |= MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects;
return reasons;
}

void PaintPropertyTreeBuilder::updateScrollAndScrollTranslation(
const LayoutObject& object,
PaintPropertyTreeBuilderContext& context) {
if (object.needsPaintPropertyUpdate() || context.forceSubtreeUpdate) {
bool needsScrollProperties = false;
if (object.hasOverflowClip()) {
auto mainThreadScrollingReasons = mainScrollingReasons(object);
const LayoutBox& box = toLayoutBox(object);
const PaintLayerScrollableArea* scrollableArea = box.getScrollableArea();
const auto* scrollableArea = box.getScrollableArea();
IntSize scrollOffset = box.scrolledContentOffset();
if (!scrollOffset.isZero() || scrollableArea->scrollsOverflow()) {
if (mainThreadScrollingReasons || !scrollOffset.isZero() ||
scrollableArea->scrollsOverflow()) {
needsScrollProperties = true;
auto& properties =
object.getMutableForPainting().ensurePaintProperties();
TransformationMatrix matrix = TransformationMatrix().translate(
Expand All @@ -664,32 +683,18 @@ void PaintPropertyTreeBuilder::updateScrollAndScrollTranslation(
scrollableArea->userInputScrollable(HorizontalScrollbar);
bool userScrollableVertical =
scrollableArea->userInputScrollable(VerticalScrollbar);
MainThreadScrollingReasons reasons = 0;
if (!object.document().settings()->threadedScrollingEnabled())
reasons |= MainThreadScrollingReason::kThreadedScrollingDisabled;
// Checking for descendants in the layout tree has two downsides:
// 1) There can be more descendants in layout order than in paint
// order (e.g., fixed position objects).
// 2) Iterating overall all background attachment fixed objects for
// every scroll node can be slow, though there will be no objects
// in the common case.
const FrameView& frameView = *object.frameView();
if (frameView.hasBackgroundAttachmentFixedDescendants(object)) {
reasons |=
MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects;
}
context.forceSubtreeUpdate |= properties.updateScroll(
context.current.scroll, properties.scrollTranslation(), scrollClip,
scrollBounds, userScrollableHorizontal, userScrollableVertical,
reasons);
} else {
// Ensure pre-existing properties are cleared when there is no
// scrolling.
auto* properties = object.getMutableForPainting().paintProperties();
if (properties) {
context.forceSubtreeUpdate |= properties->clearScrollTranslation();
context.forceSubtreeUpdate |= properties->clearScroll();
}
mainThreadScrollingReasons);
}
}

if (!needsScrollProperties) {
// Ensure pre-existing properties are cleared.
if (auto* properties = object.getMutableForPainting().paintProperties()) {
context.forceSubtreeUpdate |= properties->clearScrollTranslation();
context.forceSubtreeUpdate |= properties->clearScroll();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2696,6 +2696,36 @@ TEST_P(PaintPropertyTreeBuilderTest,
->hasBackgroundAttachmentFixedDescendants());
}

TEST_P(PaintPropertyTreeBuilderTest, MainThreadScrollReasonsWithoutScrolling) {
setBodyInnerHTML(
"<style>"
" #overflow {"
" overflow: scroll;"
" width: 100px;"
" height: 100px;"
" }"
" .backgroundAttachmentFixed {"
" background-image: url('foo');"
" background-attachment: fixed;"
" width: 10px;"
" height: 10px;"
" }"
" .forceScroll {"
" height: 4000px;"
" }"
"</style>"
"<div id='overflow'>"
" <div class='backgroundAttachmentFixed'></div>"
"</div>"
"<div class='forceScroll'></div>");
Element* overflow = document().getElementById("overflow");
EXPECT_TRUE(frameScroll()->hasBackgroundAttachmentFixedDescendants());
EXPECT_TRUE(overflow->layoutObject()
->paintProperties()
->scroll()
->hasBackgroundAttachmentFixedDescendants());
}

TEST_P(PaintPropertyTreeBuilderTest,
BackgroundAttachmentFixedMainThreadScrollReasonsWithFixedScroller) {
setBodyInnerHTML(
Expand Down

0 comments on commit 132d936

Please sign in to comment.