Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(152)

Issue 2489703002: Revert of Implement overlay scrollbar fade out for non-composited scrollers. (Closed)

Created:
4 years, 1 month ago by bokan
Modified:
4 years, 1 month ago
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-api_chromium.org, blink-reviews-paint_chromium.org, chromium-reviews, darin-cc_chromium.org, dglazkov+blink, dshwang, dtapuska+blinkwatch_chromium.org, Eric Willigers, jam, mac-reviews_chromium.org, Navid Zolghadr, rjwright, shans, chaopeng
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Implement overlay scrollbar fade out for non-composited scrollers. (patchset #11 id:200001 of https://codereview.chromium.org/2467693002/ ) Reason for revert: Speculative revert for issue crbug.com/662402 BUG=662402 Original issue's description: > [Reland] Implement overlay scrollbar fade out for non-composited scrollers. > > In crrev.com/2442573002 and crrev.com/2453553003 I added fade out for Aura > overlay scrollbars. In this patch I add a simple instantly disappearing > fade-out for scrollers that aren't composited. > > The changes in this patch are: > > -Plumb through the fade out durations into Blink. Any time a ScrollableArea is > scrolled or resized we show the scrollbars and start a timer to make them > disappear. The scrollbars are marked as "hidden" and made disabled and invisible > to hit testing. > > -Fixed hit testing for overlay scrollbars so we can't scroll by clicking on the > track (which isn't painted). > > -Fixed two layout tests: > third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-hittest.html > overlay-scrollbar-mouse-capture.html > These tests turn on overlay scrollbars but try to scroll by clicking a > button/track which I fixed. The tests now drag the thumb instead. > > -In crrev.com/2453553003 I added didChangeScrollbarsHidden to disable overlay > scrollbars so they're invisible to hit testing. It turns out Mac overlay > scrollbars already have this functionality in > ScrollableArea::scrollbarVisibilityChanged and > Scrollbar::shouldParticipateInHitTesting so I removed didChangeScrollbarsHidden > and used these. I also removed some redundancy in the Mac path so that Mac and > Aura overlays disable hit testing in the same way. > > -Removed the static compile assert about ScrollableArea staying small. There's > no reason ScrollableArea is particularily special and this encourages > developers to duplicate functionality in the descendant classes. > > BUG=592098 > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > > Committed: https://crrev.com/4430234f2f5f52efe60ad2b8e6608112352259da > Committed: https://crrev.com/60d2bbbc8fa4cce01bbd90c97d3e989a47d6dedf > Cr-Original-Commit-Position: refs/heads/master@{#429616} > Cr-Commit-Position: refs/heads/master@{#430065} TBR=jbroman@chromium.org,dtapuska@chromium.org,piman@chromium.org,pkasting@chromium.org,rbyers@chromium.org,skobes@chromium.org,thestig@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=592098 Committed: https://crrev.com/424b967d331e2c317501abd82f275ecb3c7d24d5 Cr-Commit-Position: refs/heads/master@{#430824}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+112 lines, -304 lines) Patch
M content/child/webthemeengine_impl_android.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M content/child/webthemeengine_impl_default.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/child/webthemeengine_impl_default.cc View 2 chunks +0 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-hittest.html View 1 chunk +1 line, -2 lines 0 comments Download
A third_party/WebKit/LayoutTests/platform/mac/plugins/overlay-scrollbar-mouse-capture-expected.txt View 1 chunk +15 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/plugins/overlay-scrollbar-mouse-capture.html View 2 chunks +7 lines, -18 lines 0 comments Download
M third_party/WebKit/LayoutTests/plugins/overlay-scrollbar-mouse-capture-expected.txt View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/FrameView.cpp View 5 chunks +7 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/frame/RootFrameViewport.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/frame/RootFrameViewport.cpp View 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayer.cpp View 2 chunks +1 line, -7 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp View 6 chunks +5 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/exported/WebScrollbarThemeClientImpl.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/exported/WebScrollbarThemeClientImpl.cpp View 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm View 1 chunk +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollAnimatorBase.h View 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollableArea.h View 7 chunks +5 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp View 5 chunks +20 lines, -58 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/Scrollbar.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/Scrollbar.cpp View 3 chunks +6 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarTheme.h View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarTheme.cpp View 1 chunk +0 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarThemeClient.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarThemeMac.mm View 2 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.h View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlay.cpp View 3 chunks +2 lines, -21 lines 0 comments Download
M third_party/WebKit/Source/platform/scroll/ScrollbarThemeOverlayMock.h View 1 chunk +1 line, -12 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebFrameTest.cpp View 2 chunks +0 lines, -84 lines 0 comments Download
M third_party/WebKit/public/platform/WebThemeEngine.h View 1 chunk +2 lines, -13 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
bokan
Created Revert of Implement overlay scrollbar fade out for non-composited scrollers.
4 years, 1 month ago (2016-11-08 21:43:24 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2489703002/1
4 years, 1 month ago (2016-11-08 21:44:09 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 1 month ago (2016-11-09 01:57:01 UTC) #5
commit-bot: I haz the power
4 years, 1 month ago (2016-11-09 02:01:38 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/424b967d331e2c317501abd82f275ecb3c7d24d5
Cr-Commit-Position: refs/heads/master@{#430824}

Powered by Google App Engine
This is Rietveld 408576698