GitLab

This CL fixes a CHECK failure with a base::Optional type. After the CL
crrev.com/c/2553869 was landed, it exposed a pre-existing product bug
(crbug.com/1164008) and that led to an increase in crashes in
cc::ScrollOffsetAnimationCurve::UpdateTarget.

Problem:
In ScrollOffsetAnimationCurve::EaseInOutBoundedSegmentDuration the
variable duration_behavior_ gets referenced. This variable did not
have any value assigned to it and hence causes the crash.

Why it did not fail before:
Before the CL 2553869 landed, the logical flow was different. As in, if
the animation_type_ (in UpdateTarget) was kEaseInOut, the new_duration
would've been calculated by calling EaseInOutBoundedSegmentDuration.
This is the only function that references the base::Optional value
(duration_behavior_). All other animation types (kImpulse and kLinear)
would've led to the function ImpulseSegmentDuration being called where
duration_behavior_ was not referenced. In the crash dump, I observed
that the animation_type_ was kLinear. This type does *not* have
duration_behavior_ defined and UpdateTarget does *not* support kLinear
animation_type_ and hence the crash. So in fact, UpdateTarget being
called for a linear animation is the real issue here (and this issue is
a pre-existing bug in the product). The CL 2553869 merely exposed it.

Fix:
I've added a bandaid solution where UpdateTarget simply returns if the
animation_type_ is kLinear. Even after multiple tries, I was never able
to repro the original bug (where UpdateTarget is called on linear
animations). The original bug is tracked via crbug.com/1164008.

Bug: 1158655
Change-Id: I0aca4870e8578de7690a6c4f8ee91f6786da1f15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613934Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#842295}