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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 24409003: Re-land r158052 after removing ASSERT_NOT_REACHED that caused crash. (Closed)

Created:
7 years, 3 months ago by dshwang
Modified:
7 years, 2 months ago
Reviewers:
Ian Vollick, shawnsingh
CC:
blink-reviews, jamesr, shans, rjwright, jchaffraix+rendering, alancutter (OOO until 2018), Mike Lawther (Google), jeez, eae+blinkwatch, leviw+renderwatch, blink-layers+watch_chromium.org, dstockwell, Timothy Loh, Rik, dsinclair, danakj, darktears, Stephen Chennney, Steve Block, dino_apple.com, pdr., Eric Willigers
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Re-land r158052 after removing ASSERT_NOT_REACHED that caused crash. Revert "Revert "Refactoring animation code in accelerated path."" r158052 introduced invaild ASSERT_NOT_REACHED that caused crash. This CL re-land r158052 after removing invalid ASSERT_NOT_REACHED. BUG=N/A Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158315

Patch Set 1 #

Total comments: 3

Patch Set 2 : Return AnimatedPropertyInvalid if it is not supported. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+360 lines, -807 lines) Patch
M Source/core/core.gypi View 4 chunks +5 lines, -3 lines 0 comments Download
A + Source/core/platform/animation/AnimationTranslationUtil.h View 0 chunks +-1 lines, --1 lines 0 comments Download
A + Source/core/platform/animation/AnimationTranslationUtil.cpp View 1 chunk +1 line, -1 line 0 comments Download
A + Source/core/platform/animation/AnimationTranslationUtilTest.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/platform/graphics/GraphicsLayer.h View 5 chunks +4 lines, -12 lines 0 comments Download
M Source/core/platform/graphics/GraphicsLayer.cpp View 4 chunks +9 lines, -38 lines 0 comments Download
D Source/core/platform/graphics/chromium/AnimationTranslationUtil.h View 1 chunk +0 lines, -56 lines 0 comments Download
D Source/core/platform/graphics/chromium/AnimationTranslationUtil.cpp View 1 chunk +0 lines, -303 lines 0 comments Download
D Source/core/platform/graphics/chromium/AnimationTranslationUtilTest.cpp View 1 chunk +0 lines, -263 lines 0 comments Download
M Source/core/rendering/RenderLayerBacking.h View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderLayerBacking.cpp View 7 chunks +47 lines, -127 lines 0 comments Download
A Source/core/rendering/animation/WebAnimationProvider.h View 1 chunk +76 lines, -0 lines 0 comments Download
A Source/core/rendering/animation/WebAnimationProvider.cpp View 1 1 chunk +214 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
dshwang
This cl reland the CL that crrev.com/24239011 reverted. ASSERT in r158052 reveals existing bug. So ...
7 years, 3 months ago (2013-09-24 16:21:43 UTC) #1
dshwang
This ASSERT will be added in Issue 297641 https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp File Source/core/rendering/animation/WebAnimationProvider.cpp (right): https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp#newcode62 Source/core/rendering/animation/WebAnimationProvider.cpp:62: case ...
7 years, 3 months ago (2013-09-24 16:23:16 UTC) #2
Ian Vollick
https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp File Source/core/rendering/animation/WebAnimationProvider.cpp (right): https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp#newcode62 Source/core/rendering/animation/WebAnimationProvider.cpp:62: case CSSPropertyWebkitFilter: On 2013/09/24 16:23:17, dshwang wrote: > I ...
7 years, 3 months ago (2013-09-24 17:26:58 UTC) #3
dshwang
https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp File Source/core/rendering/animation/WebAnimationProvider.cpp (right): https://codereview.chromium.org/24409003/diff/1/Source/core/rendering/animation/WebAnimationProvider.cpp#newcode116 Source/core/rendering/animation/WebAnimationProvider.cpp:116: return getWebAnimationId(animationNameForTransition(animatedProperty)); That's good idea but it does not ...
7 years, 3 months ago (2013-09-24 18:05:08 UTC) #4
dshwang
could you review again?
7 years, 2 months ago (2013-09-25 07:07:16 UTC) #5
Ian Vollick
On 2013/09/25 07:07:16, dshwang wrote: > could you review again? Thanks, lgtm.
7 years, 2 months ago (2013-09-25 07:31:31 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/24409003/9001
7 years, 2 months ago (2013-09-25 08:39:01 UTC) #7
commit-bot: I haz the power
7 years, 2 months ago (2013-09-25 09:38:02 UTC) #8
Message was sent while issue was closed. 
Change committed as 158315

Powered by Google App Engine
This is Rietveld 408576698