Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2332923002:
Fix scroll animation UpdateTarget for zero-duration segments (Closed)
DescriptionFix scroll animation UpdateTarget for zero-duration segments
BUG=645317
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
Committed: https://crrev.com/8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d
Cr-Commit-Position: refs/heads/master@{#418343}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Address steve's comments #
Total comments: 2
Patch Set 3 : update tests #
Messages
Total messages: 29 (16 generated)
Description was changed from ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 ========== to ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
Description was changed from ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
ymalik@chromium.org changed reviewers: + skobes@chromium.org
@skobes, please see the comment in the bug or the unittest to see the failure mode.
The CQ bit was checked by ymalik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... File cc/animation/scroll_offset_animation_curve.cc (right): https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... cc/animation/scroll_offset_animation_curve.cc:213: if (total_animation_duration_.is_zero()) { I think this block is redundant now? https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... cc/animation/scroll_offset_animation_curve.cc:235: SegmentDuration(new_delta, duration_behavior_, base::TimeDelta()); Pass delayed_by here?
PTAL :) https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... File cc/animation/scroll_offset_animation_curve.cc (right): https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... cc/animation/scroll_offset_animation_curve.cc:213: if (total_animation_duration_.is_zero()) { On 2016/09/12 22:45:46, skobes wrote: > I think this block is redundant now? Yes, you're right. https://codereview.chromium.org/2332923002/diff/1/cc/animation/scroll_offset_... cc/animation/scroll_offset_animation_curve.cc:235: SegmentDuration(new_delta, duration_behavior_, base::TimeDelta()); On 2016/09/12 22:45:46, skobes wrote: > Pass delayed_by here? Done.
lgtm
ymalik@chromium.org changed reviewers: + ajuma@chromium.org
+ajuma for cc/animations
lgtm https://codereview.chromium.org/2332923002/diff/20001/cc/animation/scroll_off... File cc/animation/scroll_offset_animation_curve_unittest.cc (right): https://codereview.chromium.org/2332923002/diff/20001/cc/animation/scroll_off... cc/animation/scroll_offset_animation_curve_unittest.cc:260: // This test verifies that if the last segment duration is zero, ::updateTarget nit: "UpdateTarget"
The CQ bit was checked by ymalik@chromium.org
The CQ bit was unchecked by ymalik@chromium.org
https://codereview.chromium.org/2332923002/diff/20001/cc/animation/scroll_off... File cc/animation/scroll_offset_animation_curve_unittest.cc (right): https://codereview.chromium.org/2332923002/diff/20001/cc/animation/scroll_off... cc/animation/scroll_offset_animation_curve_unittest.cc:260: // This test verifies that if the last segment duration is zero, ::updateTarget On 2016/09/13 14:51:32, ajuma wrote: > nit: "UpdateTarget" Done.
The CQ bit was checked by ymalik@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org, skobes@chromium.org Link to the patchset: https://codereview.chromium.org/2332923002/#ps40001 (title: "update tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_precise_blink_rel on master.tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_precise_blink_rel/...)
The CQ bit was checked by ymalik@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel ========== to ========== Fix scroll animation UpdateTarget for zero-duration segments BUG=645317 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Committed: https://crrev.com/8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d Cr-Commit-Position: refs/heads/master@{#418343} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/8576ae6fc3b91b6f5bc952eb40d0f0a0fd685e4d Cr-Commit-Position: refs/heads/master@{#418343} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
