Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Side by Side Diff: third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp

Issue 1678713002: Correctly handle cancelling compositor animations initiated from main thread. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (c) 2011, Google Inc. All rights reserved. 2 * Copyright (c) 2011, Google Inc. All rights reserved.
3 * 3 *
4 * Redistribution and use in source and binary forms, with or without 4 * Redistribution and use in source and binary forms, with or without
5 * modification, are permitted provided that the following conditions are 5 * modification, are permitted provided that the following conditions are
6 * met: 6 * met:
7 * 7 *
8 * * Redistributions of source code must retain the above copyright 8 * * Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * * Redistributions in binary form must reproduce the above 10 * * Redistributions in binary form must reproduce the above
(...skipping 90 matching lines...) Expand 10 before | Expand all | Expand 10 after
101 resetAnimationState(); 101 resetAnimationState();
102 } 102 }
103 return ScrollAnimatorBase::userScroll(orientation, granularity, step, de lta); 103 return ScrollAnimatorBase::userScroll(orientation, granularity, step, de lta);
104 } 104 }
105 105
106 float usedPixelDelta = computeDeltaToConsume(orientation, step * delta); 106 float usedPixelDelta = computeDeltaToConsume(orientation, step * delta);
107 FloatPoint pixelDelta = (orientation == VerticalScrollbar 107 FloatPoint pixelDelta = (orientation == VerticalScrollbar
108 ? FloatPoint(0, usedPixelDelta) : FloatPoint(usedPixelDelta, 0)); 108 ? FloatPoint(0, usedPixelDelta) : FloatPoint(usedPixelDelta, 0));
109 109
110 FloatPoint targetPos = desiredTargetPosition(); 110 FloatPoint targetPos = desiredTargetPosition();
111 targetPos.moveBy(pixelDelta); 111 targetPos.moveBy(pixelDelta);
ymalik 2016/02/07 21:23:37 @ajuma This is speculatively fix up the assert fa
ajuma 2016/02/08 15:05:35 Assuming that userScroll can get called at arbitra
112 112
113 // Reset animation state in case we get a scroll between when an animation
114 // was asked to be cancelled and updateCompositorAnimations is called.
115 if (m_runState == RunState::WaitingToCancelOnCompositor) {
116 abortAnimation();
ajuma 2016/02/08 15:05:35 It looks like this could touch compositing state i
ymalik 2016/02/08 18:41:23 Ah right. That's why we have the whole state machi
117 resetAnimationState();
118 }
119
113 if (m_animationCurve) { 120 if (m_animationCurve) {
114 if ((targetPos - m_targetOffset).isZero()) { 121 if ((targetPos - m_targetOffset).isZero()) {
115 // Report unused delta only if there is no animation running. See 122 // Report unused delta only if there is no animation running. See
116 // comment below regarding scroll latching. 123 // comment below regarding scroll latching.
117 return ScrollResultOneDimensional(/* didScroll */ true, /* unusedScr ollDelta */ 0); 124 return ScrollResultOneDimensional(/* didScroll */ true, /* unusedScr ollDelta */ 0);
118 } 125 }
119 126
120 m_targetOffset = targetPos; 127 m_targetOffset = targetPos;
121 ASSERT(m_runState == RunState::RunningOnMainThread 128 ASSERT(m_runState == RunState::RunningOnMainThread
122 || m_runState == RunState::RunningOnCompositor 129 || m_runState == RunState::RunningOnCompositor
(...skipping 58 matching lines...) Expand 10 before | Expand all | Expand 10 after
181 if (isFinished) 188 if (isFinished)
182 resetAnimationState(); 189 resetAnimationState();
183 else 190 else
184 scrollableArea()->scheduleAnimation(); 191 scrollableArea()->scheduleAnimation();
185 192
186 TRACE_EVENT0("blink", "ScrollAnimator::notifyPositionChanged"); 193 TRACE_EVENT0("blink", "ScrollAnimator::notifyPositionChanged");
187 notifyPositionChanged(); 194 notifyPositionChanged();
188 } 195 }
189 196
190 void ScrollAnimator::updateCompositorAnimations() 197 void ScrollAnimator::updateCompositorAnimations()
191 { 198 {
ymalik 2016/02/07 21:23:37 I don't think we should ever be in a state where w
ajuma 2016/02/08 15:05:35 Sounds reasonable.
ymalik 2016/02/08 18:41:23 Based on your feedback above, I moved the call to
192 if (m_compositorAnimationId && m_runState != RunState::RunningOnCompositor 199 if (m_runState == RunState::WaitingToCancelOnCompositor) {
193 && m_runState != RunState::RunningOnCompositorButNeedsUpdate) { 200 ASSERT(m_compositorAnimationId);
194 // If the current run state is WaitingToSendToCompositor but we have a
195 // non-zero compositor animation id, there's a currently running
196 // compositor animation that needs to be removed here before the new
197 // animation is added below.
198 ASSERT(m_runState == RunState::WaitingToCancelOnCompositor
199 || m_runState == RunState::WaitingToSendToCompositor);
200
201 abortAnimation(); 201 abortAnimation();
202 202 resetAnimationState();
203 m_compositorAnimationId = 0; 203 return;
204 m_compositorAnimationGroupId = 0;
205 if (m_runState == RunState::WaitingToCancelOnCompositor) {
206 resetAnimationState();
207 return;
208 }
209 } 204 }
210 205
211 if (m_runState == RunState::WaitingToSendToCompositor 206 if (m_runState == RunState::WaitingToSendToCompositor
212 || m_runState == RunState::RunningOnCompositorButNeedsUpdate) { 207 || m_runState == RunState::RunningOnCompositorButNeedsUpdate) {
213 if (m_runState == RunState::RunningOnCompositorButNeedsUpdate) { 208 if (m_runState == RunState::RunningOnCompositorButNeedsUpdate) {
214 // Abort the running animation before a new one with an updated 209 // Abort the running animation before a new one with an updated
215 // target is added. 210 // target is added.
216 abortAnimation(); 211 abortAnimation();
217 212
218 m_compositorAnimationId = 0; 213 m_compositorAnimationId = 0;
(...skipping 80 matching lines...) Expand 10 before | Expand all | Expand 10 after
299 } 294 }
300 return true; 295 return true;
301 } 296 }
302 297
303 DEFINE_TRACE(ScrollAnimator) 298 DEFINE_TRACE(ScrollAnimator)
304 { 299 {
305 ScrollAnimatorBase::trace(visitor); 300 ScrollAnimatorBase::trace(visitor);
306 } 301 }
307 302
308 } // namespace blink 303 } // namespace blink
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698