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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 204743002: Oilpan: Move AnimatableValue's hierarchy to oilpan's heap (Closed)

Created:
6 years, 9 months ago by haraken
Modified:
6 years, 9 months ago
CC:
blink-reviews, ed+blinkwatch_opera.com, shans, rjwright, alancutter (OOO until 2018), Mike Lawther (Google), rwlbuis, rune+blink, dglazkov+blink, dstockwell, Timothy Loh, apavlov+blink_chromium.org, darktears, Steve Block, dino_apple.com, Eric Willigers, sof
Visibility:
Public.

Description

Oilpan: Move AnimatableValue's hierarchy to oilpan's heap This CL is doing mechanical replacement and there's nothing non-trivial. BUG=340522 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169823

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Total comments: 8

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Total comments: 1

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 3

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : #

Patch Set 12 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+398 lines, -267 lines) Patch
M Source/core/animation/AnimatableClipPathOperation.h View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableClipPathOperation.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableColor.h View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableColor.cpp View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableColorTest.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableDouble.h View 1 2 3 1 chunk +6 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableDouble.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableDoubleTest.cpp View 2 chunks +5 lines, -5 lines 0 comments Download
M Source/core/animation/AnimatableFilterOperations.h View 1 2 3 1 chunk +6 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableFilterOperations.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableImage.h View 1 2 3 1 chunk +7 lines, -5 lines 0 comments Download
M Source/core/animation/AnimatableImage.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableLength.h View 4 chunks +13 lines, -11 lines 0 comments Download
M Source/core/animation/AnimatableLength.cpp View 1 2 3 5 chunks +10 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableLengthBox.h View 2 chunks +11 lines, -9 lines 0 comments Download
M Source/core/animation/AnimatableLengthBox.cpp View 1 2 3 3 chunks +10 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableLengthBoxAndBool.h View 2 chunks +8 lines, -6 lines 0 comments Download
M Source/core/animation/AnimatableLengthBoxAndBool.cpp View 1 2 3 3 chunks +7 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableLengthPoint.h View 2 chunks +9 lines, -7 lines 0 comments Download
M Source/core/animation/AnimatableLengthPoint.cpp View 1 2 3 3 chunks +8 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableLengthSize.h View 2 chunks +9 lines, -7 lines 0 comments Download
M Source/core/animation/AnimatableLengthSize.cpp View 1 2 3 3 chunks +8 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableLengthTest.cpp View 8 chunks +16 lines, -16 lines 0 comments Download
M Source/core/animation/AnimatableNeutral.h View 1 2 3 1 chunk +4 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableNeutralTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableRepeatable.h View 1 chunk +10 lines, -8 lines 0 comments Download
M Source/core/animation/AnimatableRepeatable.cpp View 1 2 3 5 chunks +17 lines, -10 lines 0 comments Download
M Source/core/animation/AnimatableSVGLength.h View 1 2 3 2 chunks +6 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableSVGLength.cpp View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableSVGPaint.h View 1 2 3 1 chunk +7 lines, -5 lines 0 comments Download
M Source/core/animation/AnimatableSVGPaint.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableShadow.h View 1 2 3 1 chunk +6 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableShadow.cpp View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableShapeValue.h View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableShapeValue.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableStrokeDasharrayList.h View 1 chunk +6 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableStrokeDasharrayList.cpp View 3 chunks +14 lines, -5 lines 0 comments Download
M Source/core/animation/AnimatableStrokeDasharrayListTest.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/animation/AnimatableTransform.h View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableTransform.cpp View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableUnknown.h View 1 2 3 2 chunks +11 lines, -6 lines 0 comments Download
M Source/core/animation/AnimatableUnknownTest.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableValue.h View 1 2 3 3 chunks +11 lines, -8 lines 0 comments Download
M Source/core/animation/AnimatableValue.cpp View 1 2 3 3 chunks +8 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableValueTestHelper.cpp View 2 chunks +2 lines, -4 lines 0 comments Download
M Source/core/animation/AnimatableValueTestHelperTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -9 lines 0 comments Download
M Source/core/animation/AnimatableVisibility.h View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableVisibility.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimationEffect.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/animation/CompositorAnimationsTest.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/InterpolableValue.h View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/animation/Interpolation.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/animation/KeyframeEffectModel.h View 1 2 3 4 5 6 4 chunks +18 lines, -11 lines 0 comments Download
M Source/core/animation/KeyframeEffectModel.cpp View 1 2 3 4 5 6 7 8 9 10 7 chunks +29 lines, -10 lines 0 comments Download
M Source/core/animation/KeyframeEffectModelTest.cpp View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M Source/core/animation/css/CSSAnimatableValueFactory.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/animation/css/CSSAnimatableValueFactory.cpp View 1 2 9 chunks +18 lines, -18 lines 0 comments Download
M Source/core/animation/css/CSSAnimations.h View 1 2 3 4 5 6 7 8 9 2 chunks +22 lines, -6 lines 0 comments Download
M Source/core/animation/css/CSSAnimations.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/css/resolver/AnimatedStyleBuilder.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 24 (0 generated)
haraken
Still crashing a bunch of tests, so I'll take a look tomorrow. Not yet for ...
6 years, 9 months ago (2014-03-19 16:41:24 UTC) #1
sof
https://codereview.chromium.org/204743002/diff/10001/Source/core/animation/KeyframeEffectModel.h File Source/core/animation/KeyframeEffectModel.h (left): https://codereview.chromium.org/204743002/diff/10001/Source/core/animation/KeyframeEffectModel.h#oldcode84 Source/core/animation/KeyframeEffectModel.h:84: PropertyValueMap m_propertyValues; Doesn't look as if this is trace()d.
6 years, 9 months ago (2014-03-20 07:20:39 UTC) #2
haraken
OK, now it's ready for review. PTAL. Currently the plugin seems not working (as I ...
6 years, 9 months ago (2014-03-20 14:26:05 UTC) #3
Mads Ager (chromium)
LGTM with comments addressed. https://codereview.chromium.org/204743002/diff/10001/Source/core/animation/AnimatableFilterOperations.h File Source/core/animation/AnimatableFilterOperations.h (right): https://codereview.chromium.org/204743002/diff/10001/Source/core/animation/AnimatableFilterOperations.h#newcode48 Source/core/animation/AnimatableFilterOperations.h:48: virtual void trace(Visitor*); OVERRIDE? https://codereview.chromium.org/204743002/diff/30001/Source/core/animation/AnimatableFilterOperations.h ...
6 years, 9 months ago (2014-03-20 14:51:54 UTC) #4
haraken
Thanks for review. https://codereview.chromium.org/204743002/diff/30001/Source/core/animation/AnimatableFilterOperations.h File Source/core/animation/AnimatableFilterOperations.h (right): https://codereview.chromium.org/204743002/diff/30001/Source/core/animation/AnimatableFilterOperations.h#newcode48 Source/core/animation/AnimatableFilterOperations.h:48: virtual void trace(Visitor*); On 2014/03/20 14:51:55, ...
6 years, 9 months ago (2014-03-21 13:45:04 UTC) #5
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 9 months ago (2014-03-21 13:45:12 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/204743002/50001
6 years, 9 months ago (2014-03-21 13:45:15 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 13:47:49 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg tryserver.blink on win_blink_rel
6 years, 9 months ago (2014-03-21 13:47:49 UTC) #9
Mads Ager (chromium)
https://codereview.chromium.org/204743002/diff/50001/Source/core/animation/KeyframeEffectModel.h File Source/core/animation/KeyframeEffectModel.h (right): https://codereview.chromium.org/204743002/diff/50001/Source/core/animation/KeyframeEffectModel.h#newcode124 Source/core/animation/KeyframeEffectModel.h:124: PassOwnPtr<PropertySpecificKeyframe> cloneWithOffset(double offset) const; Where is this PassOwnPtr consumed? ...
6 years, 9 months ago (2014-03-21 13:49:29 UTC) #10
Mads Ager (chromium)
https://codereview.chromium.org/204743002/diff/50001/Source/core/animation/KeyframeEffectModel.h File Source/core/animation/KeyframeEffectModel.h (right): https://codereview.chromium.org/204743002/diff/50001/Source/core/animation/KeyframeEffectModel.h#newcode92 Source/core/animation/KeyframeEffectModel.h:92: typedef Vector<OwnPtr<PropertySpecificKeyframe> > PropertySpecificKeyframeVector; In particular, this one does ...
6 years, 9 months ago (2014-03-21 13:52:53 UTC) #11
haraken
On 2014/03/21 13:52:53, Mads Ager (chromium) wrote: > https://codereview.chromium.org/204743002/diff/50001/Source/core/animation/KeyframeEffectModel.h > File Source/core/animation/KeyframeEffectModel.h (right): > > ...
6 years, 9 months ago (2014-03-21 14:03:05 UTC) #12
Mads Ager (chromium)
https://codereview.chromium.org/204743002/diff/70001/Source/core/animation/KeyframeEffectModel.h File Source/core/animation/KeyframeEffectModel.h (right): https://codereview.chromium.org/204743002/diff/70001/Source/core/animation/KeyframeEffectModel.h#newcode145 Source/core/animation/KeyframeEffectModel.h:145: PropertySpecificKeyframeVector m_keyframes; OK, now it bubbles down here. You ...
6 years, 9 months ago (2014-03-21 14:08:05 UTC) #13
haraken
On 2014/03/21 14:08:05, Mads Ager (chromium) wrote: > https://codereview.chromium.org/204743002/diff/70001/Source/core/animation/KeyframeEffectModel.h > File Source/core/animation/KeyframeEffectModel.h (right): > > ...
6 years, 9 months ago (2014-03-21 15:08:27 UTC) #14
Mads Ager (chromium)
LGTM, thanks for iterating! Looks like the compilation failures are telling us that the trace ...
6 years, 9 months ago (2014-03-21 17:05:44 UTC) #15
haraken
https://codereview.chromium.org/204743002/diff/120001/Source/core/animation/AnimatableValueTestHelperTest.cpp File Source/core/animation/AnimatableValueTestHelperTest.cpp (right): https://codereview.chromium.org/204743002/diff/120001/Source/core/animation/AnimatableValueTestHelperTest.cpp#newcode102 Source/core/animation/AnimatableValueTestHelperTest.cpp:102: PrintToString(const_cast<AnimatableValue*>(AnimatableValue::neutralValue())), I think you're talking about this const_cast, but ...
6 years, 9 months ago (2014-03-21 17:21:33 UTC) #16
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 9 months ago (2014-03-24 01:22:16 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/204743002/170001
6 years, 9 months ago (2014-03-24 01:22:17 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-24 01:23:36 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on mac_blink_rel
6 years, 9 months ago (2014-03-24 01:23:36 UTC) #20
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 9 months ago (2014-03-24 02:28:12 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/204743002/190001
6 years, 9 months ago (2014-03-24 02:28:17 UTC) #22
commit-bot: I haz the power
Change committed as 169823
6 years, 9 months ago (2014-03-24 06:48:32 UTC) #23
zerny-chromium
6 years, 9 months ago (2014-03-24 08:19:56 UTC) #24
Message was sent while issue was closed. 
lgtm

Powered by Google App Engine
This is Rietveld 408576698