New issue
Advanced search Search tips

Issue 910777 link

Starred by 1 user
Status: Fixed
Owner:
smcgruer@chromium.org
Closed: Dec 3
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Code Health: Extract ScrollTimeline functions from worklet_animation.cc

Project Member Reported by smcgruer@chromium.org, Dec 1 
Currently third_party/blink/renderer/modules/animationworklet/worklet_animation.cc has 3 anon-namespace functions which deal with ScrollTimeline. These cover ~200 lines of code in total and are completely untested - and they have nothing to do with WorkletAnimation (for example, they will also be needed for Web Animations when we enable ScrollTimeline for that).

We should move these functions into their own utility namespace, and test them.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/254181fa745950cc503503ee9ad3f2dcd8c2d41c

commit 254181fa745950cc503503ee9ad3f2dcd8c2d41c
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Mon Dec 03 20:31:51 2018

Introduce scroll_timeline_util namespace for utility functions

Various functions in WorkletAnimation had gotten far too big to be anonymous
namespace methods. Pull them out into proper utility functions - which means
they can also now be tested!

Bug:  910777 
Change-Id: I50a65f0e613ff71e7fddc042ee80448f0c3fbcba
Reviewed-on: https://chromium-review.googlesource.com/c/1355984
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613221}
[modify] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/cc/animation/scroll_timeline.h
[modify] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/core/BUILD.gn
[modify] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/core/animation/BUILD.gn
[add] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/core/animation/scroll_timeline_util.cc
[add] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/core/animation/scroll_timeline_util.h
[add] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/core/animation/scroll_timeline_util_test.cc
[modify] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
[modify] https://crrev.com/254181fa745950cc503503ee9ad3f2dcd8c2d41c/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py

Comment 2 by smcgruer@chromium.org, Dec 3

Status: Fixed (was: Started)
►

Sign in to add a comment