Oilpan: Move InterpolableValue's hierarchy to oilpan's heap

Oilpan: Move InterpolableValue's hierarchy to oilpan's heap

This CL changes OwnPtr<OwnPtr<InterporableValue>[]> in InterpolableValueList to Vector<OwnPtr<InterporableValue>>. As far as I see the code, I don't think this change will affect performance.

BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170298

[haraken, 2014-03-26 13:11:05]

PTAL

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
File Source/core/animation/InterpolableValue.h (right):

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
Source/core/animation/InterpolableValue.h:135:
WillBeHeapVector<OwnPtrWillBeMember<InterpolableValue> > m_values;

dstockwell@, shanes@: Would you just check that this data structure change is OK
for performance?

[Mads Ager (chromium), 2014-03-26 13:28:31]

LGTM for the oilpan part.

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
File Source/core/animation/InterpolableValue.h (right):

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
Source/core/animation/InterpolableValue.h:23: virtual ~InterpolableValue() { }
This should be removed and should lead to a compile error on the non-oilpan
build because of duplicate definitions.

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
File Source/core/animation/Interpolation.h (right):

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
Source/core/animation/Interpolation.h:17: class Interpolation : public
RefCountedWillBeGarbageCollectedFinalized<Interpolation> {
Looks like this hierarchy doesn't need to be finalized anymore. But that is an
optimization that we can get back to. :)

[haraken, 2014-03-26 13:42:30]

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
File Source/core/animation/InterpolableValue.h (right):

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
Source/core/animation/InterpolableValue.h:23: virtual ~InterpolableValue() { }
On 2014/03/26 13:28:31, Mads Ager (chromium) wrote:
> This should be removed and should lead to a compile error on the non-oilpan
> build because of duplicate definitions.

Nice catch. Done.

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
File Source/core/animation/Interpolation.h (right):

https://codereview.chromium.org/212213007/diff/1/Source/core/animation/Interp...
Source/core/animation/Interpolation.h:17: class Interpolation : public
RefCountedWillBeGarbageCollectedFinalized<Interpolation> {
On 2014/03/26 13:28:31, Mads Ager (chromium) wrote:
> Looks like this hierarchy doesn't need to be finalized anymore. But that is an
> optimization that we can get back to. :)

Done.

[wibling-chromium, 2014-03-26 14:25:43]

lgtm

[tkent, 2014-03-27 01:06:21]

lgtm

[haraken, 2014-03-27 01:27:35]

https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
File Source/core/animation/InterpolableValue.h (right):

https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
Source/core/animation/InterpolableValue.h:134:
WillBeHeapVector<OwnPtrWillBeMember<InterpolableValue> > m_values;

dstockwell@, shanes@: Would you just check that changing OwnPtr<OwnPtr<>[]> to
Vector<OwnPtr<>> would be OK for performance?

[haraken, 2014-03-28 06:00:59]

On 2014/03/27 01:27:35, haraken wrote:
>
https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
> File Source/core/animation/InterpolableValue.h (right):
> 
>
https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
> Source/core/animation/InterpolableValue.h:134:
> WillBeHeapVector<OwnPtrWillBeMember<InterpolableValue> > m_values;
> 
> dstockwell@, shanes@: Would you just check that changing OwnPtr<OwnPtr<>[]> to
> Vector<OwnPtr<>> would be OK for performance?

dstockwell@: ^^^ This is just one line change, but would you just take a quick
look? This is blocking other CLs :)

[dstockwell, 2014-03-28 06:21:02]

On 2014/03/28 06:00:59, haraken wrote:
> On 2014/03/27 01:27:35, haraken wrote:
> >
>
https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
> > File Source/core/animation/InterpolableValue.h (right):
> > 
> >
>
https://codereview.chromium.org/212213007/diff/90001/Source/core/animation/In...
> > Source/core/animation/InterpolableValue.h:134:
> > WillBeHeapVector<OwnPtrWillBeMember<InterpolableValue> > m_values;
> > 
> > dstockwell@, shanes@: Would you just check that changing OwnPtr<OwnPtr<>[]>
to
> > Vector<OwnPtr<>> would be OK for performance?
> 
> dstockwell@: ^^^ This is just one line change, but would you just take a quick
> look? This is blocking other CLs :)

lgtm

[haraken, 2014-03-28 06:21:53]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-28 06:21:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/212213007/90001

[haraken, 2014-03-28 06:22:05]

> > dstockwell@: ^^^ This is just one line change, but would you just take a
quick
> > look? This is blocking other CLs :)
> 
> lgtm

Thanks!

[commit-bot: I haz the power, 2014-03-28 07:31:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 07:31:58]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[haraken, 2014-03-28 07:33:49]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-28 07:33:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/212213007/90001

[commit-bot: I haz the power, 2014-03-28 08:38:23]

Change committed as 170298