New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 645317 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
(currently inactive on Chromium)
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK hit when scrolling very large image

Project Member Reported by bsep@chromium.org, Sep 9 2016

Issue description

Version: 55.0.2853.0 (Developer Build) (64-bit)
OS: Windows 10

1. Open a very large image in a debug build (I was using https://s.yimg.com/fz/api/res/1.2/0pO_nYO0bEkpYAQq9CAztA--/YXBwaWQ9c3JjaGRkO2g9NTE4MztxPTk1O3c9OTMyOA--/http://cyberminingcompany.com/sombrero%20galaxy.jpg)
2. Click on the image to zoom in
3. Scroll around quickly. I was using the mouse wheel, it seems like grabbing the scrollbar doesn't work.

It's a bit random but eventually it hits a DCHECK in CubicBezier::SolveCurveX, see below for the stack trace. It looks like total_animation_duration_ - last_retarget_ is 0 in ScrollOffsetAnimationCurve::UpdateTarget and so the next line, which uses old_duration as a divisor, ends up as nan.

I don't see any symptoms on a release build and I'm not familiar with the code so I can't tell how important this is, but it seems bad that we're probably writing nan to some member variables?

Stack trace:
>	base.dll!base::debug::BreakDebugger() Line 21	C++
 	base.dll!logging::LogMessage::~LogMessage() Line 751	C++
 	geometry.dll!gfx::CubicBezier::SolveCurveX(double x, double epsilon) Line 141	C++
 	geometry.dll!gfx::CubicBezier::SlopeWithEpsilon(double x, double epsilon) Line 187	C++
 	geometry.dll!gfx::CubicBezier::Slope(double x) Line 195	C++
 	cc.dll!cc::CubicBezierTimingFunction::Velocity(double x) Line 65	C++
 	cc.dll!cc::ScrollOffsetAnimationCurve::UpdateTarget(double t, const gfx::ScrollOffset & new_target) Line 233	C++
 	cc.dll!cc::ScrollOffsetAnimationsImpl::ScrollAnimationUpdateTarget(cc::ElementId element_id, const gfx::Vector2dF & scroll_delta, const gfx::ScrollOffset & max_scroll_offset, base::TimeTicks frame_monotonic_time, base::TimeDelta delayed_by) Line 105	C++
 	cc.dll!cc::AnimationHost::ImplOnlyScrollAnimationUpdateTarget(cc::ElementId element_id, const gfx::Vector2dF & scroll_delta, const gfx::ScrollOffset & max_scroll_offset, base::TimeTicks frame_monotonic_time, base::TimeDelta delayed_by) Line 525	C++
 	cc.dll!cc::LayerTreeHostImpl::ScrollAnimationUpdateTarget(cc::ScrollNode * scroll_node, const gfx::Vector2dF & scroll_delta, base::TimeDelta delayed_by) Line 3915	C++
 	cc.dll!cc::LayerTreeHostImpl::ScrollAnimated(const gfx::Point & viewport_point, const gfx::Vector2dF & scroll_delta, base::TimeDelta delayed_by) Line 2840	C++
 	content.dll!ui::InputHandlerProxy::HandleGestureScrollUpdate(const blink::WebGestureEvent & gesture_event) Line 642	C++
 	content.dll!ui::InputHandlerProxy::HandleInputEvent(const blink::WebInputEvent & event) Line 305	C++
 	content.dll!ui::InputHandlerProxy::HandleInputEventWithLatencyInfo(const blink::WebInputEvent & event, ui::LatencyInfo * latency_info) Line 285	C++
 	content.dll!content::InputHandlerManager::HandleInputEvent(int routing_id, const blink::WebInputEvent * input_event, ui::LatencyInfo * latency_info) Line 218	C++
 	content.dll!content::InputEventFilter::ForwardToHandler(const IPC::Message & message, base::TimeTicks received_time) Line 238	C++
 	content.dll!base::internal::FunctorTraits<void (__cdecl content::InputEventFilter::*)(IPC::Message const & __ptr64,base::TimeTicks) __ptr64,void>::Invoke<scoped_refptr<content::InputEventFilter> const & __ptr64,IPC::Message const & __ptr64,base::TimeTicks const & __ptr64>(void(content::InputEventFilter::*)(const IPC::Message &, base::TimeTicks) method, const scoped_refptr<content::InputEventFilter> & receiver_ptr, const IPC::Message & <args_0>, const base::TimeTicks & <args_1>) Line 215	C++
 	content.dll!base::internal::InvokeHelper<0,void>::MakeItSo<void (__cdecl content::InputEventFilter::*const & __ptr64)(IPC::Message const & __ptr64,base::TimeTicks) __ptr64,scoped_refptr<content::InputEventFilter> const & __ptr64,IPC::Message const & __ptr64,base::TimeTicks const & __ptr64>(void(content::InputEventFilter::*)(const IPC::Message &, base::TimeTicks) & functor, const scoped_refptr<content::InputEventFilter> & <args_0>, const IPC::Message & <args_1>, const base::TimeTicks & <args_2>) Line 286	C++
 	content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl content::InputEventFilter::*)(IPC::Message const & __ptr64,base::TimeTicks) __ptr64,scoped_refptr<content::InputEventFilter>,IPC::Message,base::TimeTicks>,void __cdecl(void)>::RunImpl<void (__cdecl content::InputEventFilter::*const & __ptr64)(IPC::Message const & __ptr64,base::TimeTicks) __ptr64,std::tuple<scoped_refptr<content::InputEventFilter>,IPC::Message,base::TimeTicks> const & __ptr64,0,1,2>(void(content::InputEventFilter::*)(const IPC::Message &, base::TimeTicks) & functor, const std::tuple<scoped_refptr<content::InputEventFilter>,IPC::Message,base::TimeTicks> & bound, base::IndexSequence<0,1,2> __formal) Line 351	C++
 	content.dll!base::internal::Invoker<base::internal::BindState<void (__cdecl content::InputEventFilter::*)(IPC::Message const & __ptr64,base::TimeTicks) __ptr64,scoped_refptr<content::InputEventFilter>,IPC::Message,base::TimeTicks>,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 329	C++
 	base.dll!base::Callback<void __cdecl(void),1>::Run() Line 62	C++
 	base.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, const base::PendingTask & pending_task) Line 56	C++
 	base.dll!base::MessageLoop::RunTask(const base::PendingTask & pending_task) Line 489	C++
 	base.dll!base::MessageLoop::DeferOrRunPendingTask(base::PendingTask pending_task) Line 500	C++
 	base.dll!base::MessageLoop::DoWork() Line 621	C++
 	base.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate) Line 35	C++
 	base.dll!base::MessageLoop::RunHandler() Line 452	C++
 	base.dll!base::RunLoop::Run() Line 36	C++
 	base.dll!base::Thread::Run(base::RunLoop * run_loop) Line 229	C++
 	base.dll!base::Thread::ThreadMain() Line 304	C++
 	base.dll!base::`anonymous namespace'::ThreadFunc(void * params) Line 86	C++

 
Cc: skobes@chromium.org
Owner: ymalik@chromium.org
I think this might be from https://codereview.chromium.org/2040543002.  Yash do you mind taking a look?
Status: Assigned (was: Untriaged)
Investigating..
I couldn't repro this on Linux, will try on Windows.

I think total_animation_duration_ - last_retarget_ can never be 0 in ScrollOffsetAnimationCurve::UpdateTarget, but I do agree that the crash suggests that we're calling Velocity with a nan since any other value is bounded between 0 and 1 and the DCHECK should't fail.

For total_animation_duration_ - last_retarget_ to be 0, total_animation_duration_ = last_retarget_. If this total_animation_duration_ = 0, then the call to the UpdateTarget function would early out (https://cs.chromium.org/chromium/src/cc/animation/scroll_offset_animation_curve.cc?rcl=0&l=213). 

This suggests that total_animation_duration = last_retarget_ != 0.

The only other place we set last_retarget_ is at the end of the UpdateTarget function (https://cs.chromium.org/chromium/src/cc/animation/scroll_offset_animation_curve.cc?rcl=0&l=261).  Right above that, we also set total_animation_duration.

last_retarget_ is set to t, and total_animation_duration is set to t + new_duration. For last_retarget = total_animation_duration, new_duration has to be 0 (since t is always >= 0). But if new_duration = 0, then this would early out before getting here (https://cs.chromium.org/chromium/src/cc/animation/scroll_offset_animation_curve.cc?rcl=0&l=244).

So it's a bit unclear to me what's going on.

@skobes, WDYT?

Comment 4 by ymalik@chromium.org, Sep 12 2016

Hmm, so I was able to repro this on windows.

Comment 3 is not complete. The DCHECK fails for total_animation_duration = last_retarget_ != 0.

This can happen in the following scenerio:
UpdateTarget 1: t > 0 and we end up setting last_retarget > 0, total_animation_duration > 0
UpdateTarget 2: t < 0, we clamp t to last_retarget_, last_retarget = same as before, total_animation_duration = t = last_retarget, 
UpdateTarget 3: t < last_retarget, total_animation_duration = last_retarget != 0, and we hit the DCHECK.

I have a fix here: https://codereview.chromium.org/2332923002

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 13 2016

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

commit 8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d
Author: ymalik <ymalik@chromium.org>
Date: Tue Sep 13 19:50:50 2016

Fix scroll animation UpdateTarget for zero-duration segments

BUG= 645317 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2332923002
Cr-Commit-Position: refs/heads/master@{#418343}

[modify] https://crrev.com/8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d/cc/animation/scroll_offset_animation_curve.cc
[modify] https://crrev.com/8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d/cc/animation/scroll_offset_animation_curve_unittest.cc

Comment 6 by ymalik@chromium.org, Sep 13 2016

Labels: -Pri-3 Pri-2
Status: Fixed (was: Assigned)
Labels: Hotlist-Input-Dev

Sign in to add a comment