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

Issue 10874064: Fixes crash introduced @ 153047 (you can hit crash by maximizing a (Closed)

Created:
8 years, 4 months ago by sky
Modified:
8 years, 4 months ago
Reviewers:
Ian Vollick
CC:
chromium-reviews, piman+watch_chromium.org, jonathan.backer, sadrul, ben+watch_chromium.org
Visibility:
Public.

Description

Fixes crash introduced @ 153047 (you can hit crash by maximizing a window). The cross fade code deletes the layer when the animation finishes. The newly added code was accessing members after the animation finished and the animator was deleted, resulting in the crash. Since I'm sure this will come up more in the future I've restructured the code to allow for deletion when calling out like this. The cross fade test exercises this code path now, but I'll see about a more focused tests shortly. BUG=129033 TEST=covered by tests. R=vollick@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153291

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -26 lines) Patch
M ash/wm/window_animations.h View 1 chunk +0 lines, -4 lines 0 comments Download
M ash/wm/window_animations.cc View 3 chunks +1 line, -12 lines 0 comments Download
M ash/wm/window_animations_unittest.cc View 4 chunks +18 lines, -6 lines 0 comments Download
M ui/compositor/layer_animation_observer.h View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/compositor/layer_animation_observer.cc View 2 chunks +11 lines, -2 lines 0 comments Download
M ui/compositor/layer_animator.h View 1 chunk +4 lines, -0 lines 0 comments Download
M ui/compositor/layer_animator.cc View 3 chunks +11 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
sky
8 years, 4 months ago (2012-08-24 17:47:14 UTC) #1
Ian Vollick
On 2012/08/24 17:47:14, sky wrote: LGTM. Thank you for doing this.
8 years, 4 months ago (2012-08-24 18:41:17 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sky@chromium.org/10874064/1
8 years, 4 months ago (2012-08-24 19:01:28 UTC) #3
commit-bot: I haz the power
8 years, 4 months ago (2012-08-24 21:25:12 UTC) #4
Change committed as 153291

Powered by Google App Engine
This is Rietveld 408576698