Commit bd0fcfc5 authored by skobes's avatar skobes Committed by Commit bot

Call ScrollableArea::ShowOverlayScrollbars for explicit scrolls only.

We now call ShowOverlayScrollbars for user and programmatic scrolls, but not for
scroll anchoring scrolls or content area resizes.

In most cases the effects will not be observable until we update the show
triggers in the compositor (an upcoming patch).

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

Review-Url: https://codereview.chromium.org/2835403002
Cr-Commit-Position: refs/heads/master@{#467252}
parent d7c81508
...@@ -4098,8 +4098,6 @@ void FrameView::UpdateScrollOffset(const ScrollOffset& offset, ...@@ -4098,8 +4098,6 @@ void FrameView::UpdateScrollOffset(const ScrollOffset& offset,
if (scroll_delta.IsZero()) if (scroll_delta.IsZero())
return; return;
ShowOverlayScrollbars();
if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) { if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
// Don't scroll the FrameView! // Don't scroll the FrameView!
ASSERT_NOT_REACHED(); ASSERT_NOT_REACHED();
...@@ -4144,6 +4142,8 @@ void FrameView::UpdateScrollOffset(const ScrollOffset& offset, ...@@ -4144,6 +4142,8 @@ void FrameView::UpdateScrollOffset(const ScrollOffset& offset,
} }
if (IsExplicitScrollType(scroll_type)) { if (IsExplicitScrollType(scroll_type)) {
if (scroll_type != kCompositorScroll)
ShowOverlayScrollbars();
ClearFragmentAnchor(); ClearFragmentAnchor();
ClearScrollAnchor(); ClearScrollAnchor();
} }
......
...@@ -637,9 +637,13 @@ RefPtr<WebTaskRunner> VisualViewport::GetTimerTaskRunner() const { ...@@ -637,9 +637,13 @@ RefPtr<WebTaskRunner> VisualViewport::GetTimerTaskRunner() const {
void VisualViewport::UpdateScrollOffset(const ScrollOffset& position, void VisualViewport::UpdateScrollOffset(const ScrollOffset& position,
ScrollType scroll_type) { ScrollType scroll_type) {
if (DidSetScaleOrLocation(scale_, FloatPoint(position)) && if (!DidSetScaleOrLocation(scale_, FloatPoint(position)))
scroll_type != kAnchoringScroll) return;
if (IsExplicitScrollType(scroll_type)) {
NotifyRootFrameViewport(); NotifyRootFrameViewport();
if (scroll_type != kCompositorScroll && LayerForScrolling())
LayerForScrolling()->PlatformLayer()->ShowScrollbars();
}
} }
GraphicsLayer* VisualViewport::LayerForContainer() const { GraphicsLayer* VisualViewport::LayerForContainer() const {
......
...@@ -372,7 +372,6 @@ void PaintLayerScrollableArea::UpdateScrollOffset( ...@@ -372,7 +372,6 @@ void PaintLayerScrollableArea::UpdateScrollOffset(
if (GetScrollOffset() == new_offset) if (GetScrollOffset() == new_offset)
return; return;
ShowOverlayScrollbars();
scroll_offset_ = new_offset; scroll_offset_ = new_offset;
LocalFrame* frame = Box().GetFrame(); LocalFrame* frame = Box().GetFrame();
...@@ -472,6 +471,8 @@ void PaintLayerScrollableArea::UpdateScrollOffset( ...@@ -472,6 +471,8 @@ void PaintLayerScrollableArea::UpdateScrollOffset(
} }
if (IsExplicitScrollType(scroll_type)) { if (IsExplicitScrollType(scroll_type)) {
if (scroll_type != kCompositorScroll)
ShowOverlayScrollbars();
frame_view->ClearFragmentAnchor(); frame_view->ClearFragmentAnchor();
if (RuntimeEnabledFeatures::scrollAnchoringEnabled()) if (RuntimeEnabledFeatures::scrollAnchoringEnabled())
GetScrollAnchor()->Clear(); GetScrollAnchor()->Clear();
......
...@@ -397,7 +397,6 @@ void ScrollableArea::WillRemoveScrollbar(Scrollbar& scrollbar, ...@@ -397,7 +397,6 @@ void ScrollableArea::WillRemoveScrollbar(Scrollbar& scrollbar,
} }
void ScrollableArea::ContentsResized() { void ScrollableArea::ContentsResized() {
ShowOverlayScrollbars();
if (ScrollAnimatorBase* scroll_animator = ExistingScrollAnimator()) if (ScrollAnimatorBase* scroll_animator = ExistingScrollAnimator())
scroll_animator->ContentsResized(); scroll_animator->ContentsResized();
} }
......
...@@ -544,8 +544,11 @@ void Scrollbar::SetEnabled(bool e) { ...@@ -544,8 +544,11 @@ void Scrollbar::SetEnabled(bool e) {
enabled_ = e; enabled_ = e;
GetTheme().UpdateEnabledState(*this); GetTheme().UpdateEnabledState(*this);
ScrollbarPart invalid_parts = GetTheme().InvalidateOnEnabledChange(); // We can skip thumb/track repaint when hiding an overlay scrollbar, but not
SetNeedsPaintInvalidation(invalid_parts); // when showing (since the proportions may have changed while hidden).
bool skipPartsRepaint = IsOverlayScrollbar() && scrollable_area_ &&
scrollable_area_->ScrollbarsHidden();
SetNeedsPaintInvalidation(skipPartsRepaint ? kNoPart : kAllParts);
} }
int Scrollbar::ScrollbarThickness() const { int Scrollbar::ScrollbarThickness() const {
......
...@@ -44,6 +44,7 @@ class MockScrollableArea : public GarbageCollectedFinalized<MockScrollableArea>, ...@@ -44,6 +44,7 @@ class MockScrollableArea : public GarbageCollectedFinalized<MockScrollableArea>,
MOCK_CONST_METHOD0(LayerForVerticalScrollbar, GraphicsLayer*()); MOCK_CONST_METHOD0(LayerForVerticalScrollbar, GraphicsLayer*());
MOCK_CONST_METHOD0(HorizontalScrollbar, Scrollbar*()); MOCK_CONST_METHOD0(HorizontalScrollbar, Scrollbar*());
MOCK_CONST_METHOD0(VerticalScrollbar, Scrollbar*()); MOCK_CONST_METHOD0(VerticalScrollbar, Scrollbar*());
MOCK_CONST_METHOD0(ScrollbarsHidden, bool());
bool UserInputScrollable(ScrollbarOrientation) const override { return true; } bool UserInputScrollable(ScrollbarOrientation) const override { return true; }
bool ScrollbarsCanBeActive() const override { return true; } bool ScrollbarsCanBeActive() const override { return true; }
......
...@@ -94,10 +94,6 @@ class PLATFORM_EXPORT ScrollbarTheme { ...@@ -94,10 +94,6 @@ class PLATFORM_EXPORT ScrollbarTheme {
return kAllParts; return kAllParts;
} }
// Returns parts of the scrollbar which must be repainted following a change
// in enabled state.
virtual ScrollbarPart InvalidateOnEnabledChange() const { return kAllParts; }
virtual void PaintScrollCorner(GraphicsContext&, virtual void PaintScrollCorner(GraphicsContext&,
const DisplayItemClient&, const DisplayItemClient&,
const IntRect& corner_rect); const IntRect& corner_rect);
......
...@@ -69,10 +69,6 @@ ScrollbarPart ScrollbarThemeOverlay::InvalidateOnThumbPositionChange( ...@@ -69,10 +69,6 @@ ScrollbarPart ScrollbarThemeOverlay::InvalidateOnThumbPositionChange(
return kNoPart; return kNoPart;
} }
ScrollbarPart ScrollbarThemeOverlay::InvalidateOnEnabledChange() const {
return kNoPart;
}
int ScrollbarThemeOverlay::ScrollbarThickness( int ScrollbarThemeOverlay::ScrollbarThickness(
ScrollbarControlSize control_size) { ScrollbarControlSize control_size) {
return thumb_thickness_ + scrollbar_margin_; return thumb_thickness_ + scrollbar_margin_;
......
...@@ -53,8 +53,6 @@ class PLATFORM_EXPORT ScrollbarThemeOverlay : public ScrollbarTheme { ...@@ -53,8 +53,6 @@ class PLATFORM_EXPORT ScrollbarThemeOverlay : public ScrollbarTheme {
float old_position, float old_position,
float new_position) const override; float new_position) const override;
ScrollbarPart InvalidateOnEnabledChange() const override;
int ScrollbarThickness(ScrollbarControlSize) override; int ScrollbarThickness(ScrollbarControlSize) override;
int ScrollbarMargin() const override; int ScrollbarMargin() const override;
bool UsesOverlayScrollbars() const override; bool UsesOverlayScrollbars() const override;
......
...@@ -115,17 +115,23 @@ TEST_F(ScrollbarThemeOverlayTest, PaintInvalidation) { ...@@ -115,17 +115,23 @@ TEST_F(ScrollbarThemeOverlayTest, PaintInvalidation) {
vertical_scrollbar->ClearThumbNeedsRepaint(); vertical_scrollbar->ClearThumbNeedsRepaint();
mock_scrollable_area->ClearNeedsPaintInvalidationForScrollControls(); mock_scrollable_area->ClearNeedsPaintInvalidationForScrollControls();
// Disabling the scrollbar is used to hide it so it should cause invalidation // Hiding the scrollbar should invalidate the layer (SetNeedsDisplay) but not
// but only in the general sense since the compositor will have hidden it // trigger repaint of the thumb resouce, since the compositor will give the
// without a repaint. // entire layer opacity 0.
EXPECT_CALL(*mock_scrollable_area, ScrollbarsHidden()).WillOnce(Return(true));
vertical_scrollbar->SetEnabled(false); vertical_scrollbar->SetEnabled(false);
EXPECT_FALSE(vertical_scrollbar->ThumbNeedsRepaint()); EXPECT_FALSE(vertical_scrollbar->ThumbNeedsRepaint());
EXPECT_TRUE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation()); EXPECT_TRUE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation());
mock_scrollable_area->ClearNeedsPaintInvalidationForScrollControls(); mock_scrollable_area->ClearNeedsPaintInvalidationForScrollControls();
// Showing the scrollbar needs to repaint the thumb resource, since it may
// have been repainted in the disabled state while hidden (e.g. from
// SetProportion on bounds changes).
EXPECT_CALL(*mock_scrollable_area, ScrollbarsHidden())
.WillOnce(Return(false));
vertical_scrollbar->SetEnabled(true); vertical_scrollbar->SetEnabled(true);
EXPECT_FALSE(vertical_scrollbar->ThumbNeedsRepaint()); EXPECT_TRUE(vertical_scrollbar->ThumbNeedsRepaint());
EXPECT_TRUE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation()); EXPECT_TRUE(mock_scrollable_area->VerticalScrollbarNeedsPaintInvalidation());
ThreadState::Current()->CollectAllGarbage(); ThreadState::Current()->CollectAllGarbage();
......
...@@ -11629,9 +11629,6 @@ TEST_F(WebFrameTest, TestNonCompositedOverlayScrollbarsFade) { ...@@ -11629,9 +11629,6 @@ TEST_F(WebFrameTest, TestNonCompositedOverlayScrollbarsFade) {
frame->ExecuteScript(WebScriptSource( frame->ExecuteScript(WebScriptSource(
"document.getElementById('space').style.height = '500px';")); "document.getElementById('space').style.height = '500px';"));
frame->View()->UpdateAllLifecyclePhases(); frame->View()->UpdateAllLifecyclePhases();
EXPECT_FALSE(scrollable_area->ScrollbarsHidden());
testing::RunDelayedTasks(kMockOverlayFadeOutDelayMs);
EXPECT_TRUE(scrollable_area->ScrollbarsHidden()); EXPECT_TRUE(scrollable_area->ScrollbarsHidden());
frame->ExecuteScript(WebScriptSource( frame->ExecuteScript(WebScriptSource(
......
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