Commit 2ae7c231 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

[CI] Make FrameIsScrollableDidChange return false when there's no change

FrameIsScrollableDidChange had a typo and always returned true. This
prevented an early-out in ScrollingCoordinator's
UpdateAfterCompositingChangeIfNeeded from being effective.

This patch should have no web-visible changes and should only improve
performance by making an early-out work.

Bug: 826883
Change-Id: Ic3bbac06b6cf7e6a1097a42ba43d1dfd35dcb0c6
Reviewed-on: https://chromium-review.googlesource.com/994546Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548101}
parent 4693a267
...@@ -4051,7 +4051,7 @@ void LocalFrameView::RemoveResizerArea(LayoutBox& resizer_box) { ...@@ -4051,7 +4051,7 @@ void LocalFrameView::RemoveResizerArea(LayoutBox& resizer_box) {
bool LocalFrameView::FrameIsScrollableDidChange() { bool LocalFrameView::FrameIsScrollableDidChange() {
DCHECK(GetFrame().IsLocalRoot()); DCHECK(GetFrame().IsLocalRoot());
return GetScrollingContext()->WasScrollable() == return GetScrollingContext()->WasScrollable() !=
LayoutViewportScrollableArea()->IsScrollable(); LayoutViewportScrollableArea()->IsScrollable();
} }
......
...@@ -1132,6 +1132,37 @@ TEST_P(ScrollingCoordinatorTest, StickyTriggersMainThreadScroll) { ...@@ -1132,6 +1132,37 @@ TEST_P(ScrollingCoordinatorTest, StickyTriggersMainThreadScroll) {
scroll_layer->MainThreadScrollingReasons()); scroll_layer->MainThreadScrollingReasons());
} }
// LocalFrameView::FrameIsScrollableDidChange is used as a dirty bit and is
// set to clean in ScrollingCoordinator::UpdateAfterCompositingChangeIfNeeded.
// This test ensures that the dirty bit is set and unset properly.
TEST_P(ScrollingCoordinatorTest, FrameIsScrollableDidChange) {
LoadHTML(R"HTML(
<div id='bg' style='background: red; width: 10px; height: 10px;'></div>
<div id='forcescroll' style='height: 5000px;'></div>
)HTML");
// Initially there is a change but that goes away after a compositing update.
EXPECT_TRUE(GetFrame()->View()->FrameIsScrollableDidChange());
ForceFullCompositingUpdate();
EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
// A change to background color should not change the frame's scrollability.
auto* background = GetFrame()->GetDocument()->getElementById("bg");
background->removeAttribute(HTMLNames::styleAttr);
EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
ForceFullCompositingUpdate();
// Making the frame not scroll should change the frame's scrollability.
auto* forcescroll = GetFrame()->GetDocument()->getElementById("forcescroll");
forcescroll->removeAttribute(HTMLNames::styleAttr);
GetFrame()->View()->UpdateLifecycleToLayoutClean();
EXPECT_TRUE(GetFrame()->View()->FrameIsScrollableDidChange());
ForceFullCompositingUpdate();
EXPECT_FALSE(GetFrame()->View()->FrameIsScrollableDidChange());
}
class NonCompositedMainThreadScrollingReasonTest class NonCompositedMainThreadScrollingReasonTest
: public ScrollingCoordinatorTest { : public ScrollingCoordinatorTest {
static const uint32_t kLCDTextRelatedReasons = static const uint32_t kLCDTextRelatedReasons =
......
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