Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2188623006: Add pre finalizer to Animation to ensure compositor handles get cleaned up (Closed)

Created:
2 years, 9 months ago by alancutter (OOO until 2018)
Modified:
2 years, 9 months ago
Reviewers:
haraken
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, chromium-reviews, Eric Willigers, jbroman, rjwright, shans, ymalik
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add pre finalizer to Animation to ensure compositor handles get cleaned up BUG=631052 Committed: https://crrev.com/7fec064b3a37032605f091ba1ba52846c0e05824 Cr-Commit-Position: refs/heads/master@{#408554}

Patch Set 1 #

Patch Set 2 : Real fix #

Patch Set 3 : Moved registration #

Patch Set 4 : Remove AnimationTimeline pre-finalizer #

Total comments: 1

Patch Set 5 : Remove AnimationTimeline destructor #

Patch Set 6 : Remove AnimationTimeline changes #

Patch Set 7 : Add bool check #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -0 lines) Patch
M third_party/WebKit/Source/core/animation/Animation.h View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/animation/Animation.cpp View 1 2 3 4 5 6 2 chunks +7 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 37 (16 generated)
alancutter (OOO until 2018)
2 years, 9 months ago (2016-07-28 10:21:56 UTC) #3
alancutter (OOO until 2018)
I'm not sure what the best way to test this change is as the bug ...
2 years, 9 months ago (2016-07-28 10:25:45 UTC) #4
haraken
On 2016/07/28 10:25:45, alancutter wrote: > I'm not sure what the best way to test ...
2 years, 9 months ago (2016-07-28 10:31:25 UTC) #7
alancutter (OOO until 2018)
On 2016/07/28 at 10:31:25, haraken wrote: > On 2016/07/28 10:25:45, alancutter wrote: > > I'm ...
2 years, 9 months ago (2016-07-28 10:37:51 UTC) #8
haraken
On 2016/07/28 10:37:51, alancutter wrote: > On 2016/07/28 at 10:31:25, haraken wrote: > > On ...
2 years, 9 months ago (2016-07-28 10:40:46 UTC) #11
alancutter (OOO until 2018)
On 2016/07/28 at 10:40:46, haraken wrote: > On 2016/07/28 10:37:51, alancutter wrote: > > On ...
2 years, 9 months ago (2016-07-28 10:52:22 UTC) #12
haraken
On 2016/07/28 10:52:22, alancutter wrote: > On 2016/07/28 at 10:40:46, haraken wrote: > > On ...
2 years, 9 months ago (2016-07-28 10:57:11 UTC) #13
alancutter (OOO until 2018)
On 2016/07/28 at 10:57:11, haraken wrote: > Ah, thanks. Now I understand. Then this CL ...
2 years, 9 months ago (2016-07-28 11:13:26 UTC) #14
haraken
On 2016/07/28 11:13:26, alancutter wrote: > On 2016/07/28 at 10:57:11, haraken wrote: > > Ah, ...
2 years, 9 months ago (2016-07-28 11:16:26 UTC) #15
alancutter (OOO until 2018)
On 2016/07/28 at 11:16:26, haraken wrote: > On 2016/07/28 11:13:26, alancutter wrote: > > On ...
2 years, 9 months ago (2016-07-28 11:41:32 UTC) #16
haraken
On 2016/07/28 11:41:32, alancutter wrote: > On 2016/07/28 at 11:16:26, haraken wrote: > > On ...
2 years, 9 months ago (2016-07-28 11:51:51 UTC) #21
alancutter (OOO until 2018)
On 2016/07/28 at 11:51:51, haraken wrote: > On 2016/07/28 11:41:32, alancutter wrote: > > Moved ...
2 years, 9 months ago (2016-07-28 12:01:04 UTC) #22
haraken
https://codereview.chromium.org/2188623006/diff/60001/third_party/WebKit/Source/core/animation/AnimationTimeline.cpp File third_party/WebKit/Source/core/animation/AnimationTimeline.cpp (right): https://codereview.chromium.org/2188623006/diff/60001/third_party/WebKit/Source/core/animation/AnimationTimeline.cpp#newcode92 third_party/WebKit/Source/core/animation/AnimationTimeline.cpp:92: animation->dispose(); I was assuming that we can entirely stop ...
2 years, 9 months ago (2016-07-28 12:04:26 UTC) #23
alancutter (OOO until 2018)
On 2016/07/28 at 12:04:26, haraken wrote: > https://codereview.chromium.org/2188623006/diff/60001/third_party/WebKit/Source/core/animation/AnimationTimeline.cpp > File third_party/WebKit/Source/core/animation/AnimationTimeline.cpp (right): > > https://codereview.chromium.org/2188623006/diff/60001/third_party/WebKit/Source/core/animation/AnimationTimeline.cpp#newcode92 ...
2 years, 9 months ago (2016-07-28 12:34:31 UTC) #24
haraken
On 2016/07/28 12:34:31, alancutter wrote: > On 2016/07/28 at 12:04:26, haraken wrote: > > > ...
2 years, 9 months ago (2016-07-28 12:35:38 UTC) #25
alancutter (OOO until 2018)
On 2016/07/28 at 12:35:38, haraken wrote: > On 2016/07/28 12:34:31, alancutter wrote: > > On ...
2 years, 9 months ago (2016-07-28 12:41:38 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2188623006/100001
2 years, 9 months ago (2016-07-28 12:42:01 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/103603)
2 years, 9 months ago (2016-07-28 12:53:09 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2188623006/120001
2 years, 9 months ago (2016-07-29 00:52:15 UTC) #34
commit-bot: I haz the power
Committed patchset #7 (id:120001)
2 years, 9 months ago (2016-07-29 02:03:50 UTC) #35
commit-bot: I haz the power
2 years, 9 months ago (2016-07-29 02:05:52 UTC) #37
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/7fec064b3a37032605f091ba1ba52846c0e05824
Cr-Commit-Position: refs/heads/master@{#408554}

Powered by Google App Engine
This is Rietveld 408576698