Commit 2504c3b7 authored by Yi Gu's avatar Yi Gu Committed by Commit Bot

[ScrollTimeline] Fix a clusterfuzz bug due to updating the wrong object

cc::WorkletAnimation has both scroll_timeline_ and animation_timeline_.
When a worklet animation updates its ScrollTimeline, it is the
scroll_timeline_ that should be updated. However, the other timeline
gets updated due to function inheritance. This will be fixed after
crbug.com/1023508. Until then we should apply a temporary fix.

This patch allows WorkletAnimation to override the base function to
update the scroll_timeline_ correctly.

Bug: 1045812
Change-Id: I7e83e7930c4d236e88198ef645c81db8f4041555
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020573Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735413}
parent 3b3993a7
...@@ -74,9 +74,12 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> { ...@@ -74,9 +74,12 @@ class CC_ANIMATION_EXPORT Animation : public base::RefCounted<Animation> {
virtual void PromoteScrollTimelinePendingToActive(); virtual void PromoteScrollTimelinePendingToActive();
// Should be called when the ScrollTimeline attached to this animation has a // Should be called when the ScrollTimeline attached to this animation has a
// change, such as when the scroll source changes ElementId. // change, such as when the scroll source changes ElementId.
void UpdateScrollTimeline(base::Optional<ElementId> scroller_id, // TODO(yigu): This is currently virtual because WorkletAnimation has a
base::Optional<double> start_scroll_offset, // separate ScrollTimeline member and the update should be applied to it
base::Optional<double> end_scroll_offset); // instead of the AnimationTimeline object in this class.
virtual void UpdateScrollTimeline(base::Optional<ElementId> scroller_id,
base::Optional<double> start_scroll_offset,
base::Optional<double> end_scroll_offset);
scoped_refptr<ElementAnimations> element_animations() const; scoped_refptr<ElementAnimations> element_animations() const;
......
...@@ -276,6 +276,14 @@ void WorkletAnimation::PromoteScrollTimelinePendingToActive() { ...@@ -276,6 +276,14 @@ void WorkletAnimation::PromoteScrollTimelinePendingToActive() {
ReleasePendingTreeLock(); ReleasePendingTreeLock();
} }
void WorkletAnimation::UpdateScrollTimeline(
base::Optional<ElementId> scroller_id,
base::Optional<double> start_scroll_offset,
base::Optional<double> end_scroll_offset) {
scroll_timeline_->UpdateScrollerIdAndScrollOffsets(
scroller_id, start_scroll_offset, end_scroll_offset);
}
void WorkletAnimation::RemoveKeyframeModel(int keyframe_model_id) { void WorkletAnimation::RemoveKeyframeModel(int keyframe_model_id) {
state_ = State::REMOVED; state_ = State::REMOVED;
Animation::RemoveKeyframeModel(keyframe_model_id); Animation::RemoveKeyframeModel(keyframe_model_id);
......
...@@ -76,6 +76,9 @@ class CC_ANIMATION_EXPORT WorkletAnimation final : public Animation { ...@@ -76,6 +76,9 @@ class CC_ANIMATION_EXPORT WorkletAnimation final : public Animation {
void PushPropertiesTo(Animation* animation_impl) override; void PushPropertiesTo(Animation* animation_impl) override;
void PromoteScrollTimelinePendingToActive() override; void PromoteScrollTimelinePendingToActive() override;
void UpdateScrollTimeline(base::Optional<ElementId> scroller_id,
base::Optional<double> start_scroll_offset,
base::Optional<double> end_scroll_offset) override;
// Called by Blink WorkletAnimation when its playback rate is updated. // Called by Blink WorkletAnimation when its playback rate is updated.
void UpdatePlaybackRate(double playback_rate); void UpdatePlaybackRate(double playback_rate);
......
...@@ -540,6 +540,18 @@ TEST_F(WorkletAnimationTest, SkipLockedAnimations) { ...@@ -540,6 +540,18 @@ TEST_F(WorkletAnimationTest, SkipLockedAnimations) {
EXPECT_EQ(input->updated_animations.size(), 1u); EXPECT_EQ(input->updated_animations.size(), 1u);
} }
TEST_F(WorkletAnimationTest, UpdateScrollTimelineScrollerId) {
auto scroll_timeline = base::WrapRefCounted(new MockScrollTimeline());
EXPECT_EQ(scroll_timeline->GetPendingIdForTest(), ElementId());
scoped_refptr<WorkletAnimation> worklet_animation = WorkletAnimation::Create(
worklet_animation_id_, "test_name", 1, scroll_timeline, nullptr, nullptr);
ElementId scroller_id = ElementId(1);
worklet_animation->UpdateScrollTimeline(scroller_id, base::nullopt,
base::nullopt);
EXPECT_EQ(scroll_timeline->GetPendingIdForTest(), scroller_id);
}
} // namespace } // namespace
} // namespace cc } // namespace cc
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