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

Issue 1882733005: CC Animation: Make LayerAnimationController to have just one value observer. (Closed)

Created:
4 years, 8 months ago by loyso (OOO)
Modified:
4 years, 8 months ago
Reviewers:
Ian Vollick, ajuma
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@privatelac
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

CC Animation: Make LayerAnimationController to have just one value observer. - Erase ElementAnimations::ValueObserver - Refactor active and pending observer notifications to that single observer. LayerAnimationController will be merged to ElementAnimations in the dependent CL. BUG=595571 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/032567ef1cded026eb25079f0e39c08f30fc852c Cr-Commit-Position: refs/heads/master@{#387856}

Patch Set 1 #

Total comments: 10

Patch Set 2 : Fix codereview issues. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+529 lines, -501 lines) Patch
M cc/animation/element_animations.h View 5 chunks +17 lines, -17 lines 0 comments Download
M cc/animation/element_animations.cc View 1 10 chunks +28 lines, -76 lines 0 comments Download
M cc/animation/element_animations_unittest.cc View 5 chunks +26 lines, -40 lines 0 comments Download
M cc/animation/layer_animation_controller.h View 1 2 chunks +23 lines, -5 lines 0 comments Download
M cc/animation/layer_animation_controller.cc View 3 chunks +48 lines, -86 lines 0 comments Download
M cc/animation/layer_animation_controller_unittest.cc View 1 90 chunks +301 lines, -212 lines 0 comments Download
M cc/animation/layer_animation_value_observer.h View 2 chunks +9 lines, -5 lines 0 comments Download
M cc/test/animation_test_common.h View 1 1 chunk +32 lines, -23 lines 0 comments Download
M cc/test/animation_test_common.cc View 1 2 chunks +30 lines, -22 lines 0 comments Download
M cc/trees/layer_tree_host_unittest_animation.cc View 2 chunks +15 lines, -15 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 15 (6 generated)
loyso (OOO)
4 years, 8 months ago (2016-04-14 07:05:01 UTC) #4
ajuma
https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h File cc/animation/layer_animation_controller.h (right): https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h#newcode104 cc/animation/layer_animation_controller.h:104: void SetValueObserver(LayerAnimationValueObserver* observer) { Nit: set_value_observer https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h#newcode115 cc/animation/layer_animation_controller.h:115: void ...
4 years, 8 months ago (2016-04-14 14:01:18 UTC) #5
loyso (OOO)
https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h File cc/animation/layer_animation_controller.h (right): https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h#newcode104 cc/animation/layer_animation_controller.h:104: void SetValueObserver(LayerAnimationValueObserver* observer) { On 2016/04/14 14:01:18, ajuma wrote: ...
4 years, 8 months ago (2016-04-15 03:32:21 UTC) #6
loyso (OOO)
On 2016/04/15 03:32:21, loyso wrote: > https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h > File cc/animation/layer_animation_controller.h (right): > > https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h#newcode104 > ...
4 years, 8 months ago (2016-04-15 03:33:34 UTC) #7
loyso (OOO)
Nevertheless, done. It was fast :) https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h File cc/animation/layer_animation_controller.h (right): https://codereview.chromium.org/1882733005/diff/1/cc/animation/layer_animation_controller.h#newcode104 cc/animation/layer_animation_controller.h:104: void SetValueObserver(LayerAnimationValueObserver* observer) ...
4 years, 8 months ago (2016-04-15 03:53:27 UTC) #8
ajuma
Thanks! lgtm
4 years, 8 months ago (2016-04-15 12:08:00 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1882733005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1882733005/20001
4 years, 8 months ago (2016-04-18 02:31:37 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-04-18 03:27:39 UTC) #13
commit-bot: I haz the power
4 years, 8 months ago (2016-04-18 03:29:20 UTC) #15
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/032567ef1cded026eb25079f0e39c08f30fc852c
Cr-Commit-Position: refs/heads/master@{#387856}

Powered by Google App Engine
This is Rietveld 408576698