Bug 231347

Summary: Clean up state maintenance around animated scrolls
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: ScrollingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cathiechen, changseok, clord, cmarcelo, esprehn+autocc, ews-watchlist, fred.wang, glenn, jamesr, kondapallykalyan, luiz, mrobinson, pdr, simon.fraser, tonikitoo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mrobinson: review+

Simon Fraser (smfr)
Reported 2021-10-06 21:28:55 PDT
Clean up state maintenance around animated scrolls
Attachments
Patch (21.34 KB, patch)
2021-10-06 21:35 PDT, Simon Fraser (smfr) 		mrobinson: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2021-10-06 21:35:00 PDT
Created attachment 440466 [details] Patch
Martin Robinson
Comment 2 2021-10-07 01:18:00 PDT
Comment on attachment 440466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440466&action=review Really glad to see this simplification. > Source/WebCore/platform/ScrollView.cpp:530 > + if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating && newScrollPosition == this->scrollPosition()) I wonder about the combination of testing `scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating` and the call to `scrollAnimator().cancelAnimations();` above. Perhaps `scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating` is always true?
Simon Fraser (smfr)
Comment 3 2021-10-07 08:58:12 PDT
Comment on attachment 440466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440466&action=review >> Source/WebCore/platform/ScrollView.cpp:530 >> + if ((!delegatesScrolling() || currentScrollType() == ScrollType::User) && scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating && newScrollPosition == this->scrollPosition()) > > I wonder about the combination of testing `scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating` and the call to `scrollAnimator().cancelAnimations();` above. Perhaps `scrollAnimationStatus() == ScrollAnimationStatus::NotAnimating` is always true? I think you're right.
Simon Fraser (smfr)
Comment 4 2021-10-07 09:21:07 PDT
https://trac.webkit.org/changeset/283716/webkit
Radar WebKit Bug Importer
Comment 5 2021-10-07 09:22:20 PDT
<rdar://problem/83984449>
Note You need to log in before you can comment on or make changes to this bug.