Oilpan: Fix oilpan builds after r170720

Oilpan: Fix oilpan builds after r170720

r170720 broke oilpan builds seriously, so this CL tries to fix the build issue. This CL contains a bunch of inconsistent diffs about oilpan syntax, but it's just because r170720 introduced oilpan things that don't much make sense :)

BUG=340522

[haraken, 2014-04-03 01:55:14]

PTAL

[kouhei (in TOK), 2014-04-03 02:01:05]

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
File Source/core/animation/KeyframeEffectModel.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
Source/core/animation/KeyframeEffectModel.cpp:39: bool
Keyframe::compareOffsets(const RefPtrWillBeMember<Keyframe>& a, const
RefPtrWillBeMember<Keyframe>& b)
Why Member?

[haraken, 2014-04-03 02:02:05]

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
File Source/core/animation/KeyframeEffectModel.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
Source/core/animation/KeyframeEffectModel.cpp:39: bool
Keyframe::compareOffsets(const RefPtrWillBeMember<Keyframe>& a, const
RefPtrWillBeMember<Keyframe>& b)
On 2014/04/03 02:01:05, kouhei wrote:
> Why Member?

Because this is a callback function used in std::stable_sort that sorts Members.

[kouhei (in TOK), 2014-04-03 02:03:45]

lgtm

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
File Source/core/animation/KeyframeEffectModel.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
Source/core/animation/KeyframeEffectModel.cpp:39: bool
Keyframe::compareOffsets(const RefPtrWillBeMember<Keyframe>& a, const
RefPtrWillBeMember<Keyframe>& b)
On 2014/04/03 02:02:05, haraken wrote:
> On 2014/04/03 02:01:05, kouhei wrote:
> > Why Member?
> 
> Because this is a callback function used in std::stable_sort that sorts
Members.

Got it.

[haraken, 2014-04-03 03:56:11]

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Animat...
File Source/core/animation/AnimatableValueKeyframe.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Animat...
Source/core/animation/AnimatableValueKeyframe.cpp:77:
PropertySpecificKeyframe::trace(visitor);

Removed this line, because this causes an infinite loop.

Here PropertySpecificKeyframe means
AnimatableValueKeyframe::PropertySpecificKeyframe, not
KeyFrame::PropertySpecificKeyframe. Given that PropertySpecificKeyframe is
defined in all sub-classes, it's dangerous to define "typedef
Keyframe::PropertySpecificKeyframe PropertySpecificKeyframe". I removed the
typedef in the latest CL.

[haraken, 2014-04-03 03:56:15]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-04-03 03:56:20]

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

[commit-bot: I haz the power, 2014-04-03 04:10:12]

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

[commit-bot: I haz the power, 2014-04-03 04:10:12]

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

[haraken, 2014-04-03 04:12:52]

I'm landing this CL to fix build failures in oilpan bots, but I'd be happy if
one more oilpan reviewer takes a look at the CL.

[haraken, 2014-04-03 04:12:56]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-04-03 04:13:02]

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

[zerny-chromium, 2014-04-03 05:17:11]

Thanks for fixing the build Haraken. LGTM

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
File Source/core/animation/KeyframeEffectModel.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
Source/core/animation/KeyframeEffectModel.cpp:39: bool
Keyframe::compareOffsets(const RefPtrWillBeMember<Keyframe>& a, const
RefPtrWillBeMember<Keyframe>& b)
On 2014/04/03 02:03:46, kouhei wrote:
> On 2014/04/03 02:02:05, haraken wrote:
> > On 2014/04/03 02:01:05, kouhei wrote:
> > > Why Member?
> > 
> > Because this is a callback function used in std::stable_sort that sorts
> Members.
> 
> Got it.

Is the use of members needed in order to satisfy the compiler? Otherwise, using
pointers and relying on the Member->RawPtr conversion would be more consistent.

https://codereview.chromium.org/222893005/diff/40001/Source/core/animation/Ke...
File Source/core/animation/KeyframeEffectModel.h (right):

https://codereview.chromium.org/222893005/diff/40001/Source/core/animation/Ke...
Source/core/animation/KeyframeEffectModel.h:57: class
PropertySpecificKeyframeGroup : public
NoBaseWillBeGarbageCollected<PropertySpecificKeyframeGroup> {
nit: FINAL

(Seems reasonable to add when we add a non-virtual trace)

[commit-bot: I haz the power, 2014-04-03 05:47:15]

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

[commit-bot: I haz the power, 2014-04-03 05:47:16]

Failed to apply patch for Source/core/animation/AnimatableValueKeyframe.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: Source/core/animation/AnimatableValueKeyframe.cpp
  |diff --git a/Source/core/animation/AnimatableValueKeyframe.cpp
b/Source/core/animation/AnimatableValueKeyframe.cpp
  |index
48f0d551884fd35ba7736037e8c433b3a575d02e..81a0d8c1b19fda0a57bf65991e05b9f5fa280df0
100644
  |--- a/Source/core/animation/AnimatableValueKeyframe.cpp
  |+++ b/Source/core/animation/AnimatableValueKeyframe.cpp
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:       Source/core/animation/AnimatableValueKeyframe.cpp
Index: Source/core/animation/AnimatableValueKeyframe.cpp
diff --git a/Source/core/animation/AnimatableValueKeyframe.cpp
b/Source/core/animation/AnimatableValueKeyframe.cpp
index
48f0d551884fd35ba7736037e8c433b3a575d02e..81a0d8c1b19fda0a57bf65991e05b9f5fa280df0
100644
--- a/Source/core/animation/AnimatableValueKeyframe.cpp
+++ b/Source/core/animation/AnimatableValueKeyframe.cpp
@@ -31,38 +31,49 @@ PassRefPtrWillBeRawPtr<Keyframe>
AnimatableValueKeyframe::clone() const
     return adoptRefWillBeNoop(new AnimatableValueKeyframe(*this));
 }
 
-PassOwnPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::createPropertySpecificKeyframe(CSSPropertyID property)
const
+PassOwnPtrWillBeRawPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::createPropertySpecificKeyframe(CSSPropertyID property)
const
 {
-    return adoptPtr(new PropertySpecificKeyframe(offset(), easing(),
propertyValue(property), composite()));
+    return adoptPtrWillBeNoop(new PropertySpecificKeyframe(offset(), easing(),
propertyValue(property), composite()));
+}
+
+void AnimatableValueKeyframe::trace(Visitor* visitor)
+{
+    visitor->trace(m_propertyValues);
+    Keyframe::trace(visitor);
 }
 

AnimatableValueKeyframe::PropertySpecificKeyframe::PropertySpecificKeyframe(double
offset, PassRefPtr<TimingFunction> easing, const AnimatableValue* value,
AnimationEffect::CompositeOperation op)
     : Keyframe::PropertySpecificKeyframe(offset, easing, op)
-    , m_value(PassRefPtr<AnimatableValue>(const_cast<AnimatableValue*>(value)))
+    , m_value(const_cast<AnimatableValue*>(value))
 { }
 
-AnimatableValueKeyframe::PropertySpecificKeyframe::PropertySpecificKeyframe(double
offset, PassRefPtr<TimingFunction> easing, PassRefPtr<AnimatableValue> value)
+AnimatableValueKeyframe::PropertySpecificKeyframe::PropertySpecificKeyframe(double
offset, PassRefPtr<TimingFunction> easing,
PassRefPtrWillBeRawPtr<AnimatableValue> value)
     : Keyframe::PropertySpecificKeyframe(offset, easing,
AnimationEffect::CompositeReplace)
     , m_value(value)
 {
     ASSERT(!isNull(m_offset));
 }
 
-PassOwnPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::PropertySpecificKeyframe::cloneWithOffset(double
offset) const
+PassOwnPtrWillBeRawPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::PropertySpecificKeyframe::cloneWithOffset(double
offset) const
 {
     Keyframe::PropertySpecificKeyframe* theClone = new
PropertySpecificKeyframe(offset, m_easing, m_value);
-    return adoptPtr(theClone);
+    return adoptPtrWillBeNoop(theClone);
 }
 
-PassRefPtr<Interpolation>
AnimatableValueKeyframe::PropertySpecificKeyframe::createInterpolation(CSSPropertyID
property, Keyframe::PropertySpecificKeyframe* end) const
+PassRefPtrWillBeRawPtr<Interpolation>
AnimatableValueKeyframe::PropertySpecificKeyframe::createInterpolation(CSSPropertyID
property, Keyframe::PropertySpecificKeyframe* end) const
 {
     AnimatableValuePropertySpecificKeyframe* to =
toAnimatableValuePropertySpecificKeyframe(end);
     return LegacyStyleInterpolation::create(value(), to->value(), property);
 }
 
-PassOwnPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::PropertySpecificKeyframe::neutralKeyframe(double
offset, PassRefPtr<TimingFunction> easing) const
+PassOwnPtrWillBeRawPtr<Keyframe::PropertySpecificKeyframe>
AnimatableValueKeyframe::PropertySpecificKeyframe::neutralKeyframe(double
offset, PassRefPtr<TimingFunction> easing) const
+{
+    return adoptPtrWillBeNoop(new
AnimatableValueKeyframe::PropertySpecificKeyframe(offset, easing,
AnimatableValue::neutralValue(), AnimationEffect::CompositeAdd));
+}
+
+void AnimatableValueKeyframe::PropertySpecificKeyframe::trace(Visitor* visitor)
 {
-    return adoptPtr(new
AnimatableValueKeyframe::PropertySpecificKeyframe(offset, easing,
AnimatableValue::neutralValue(), AnimationEffect::CompositeAdd));
+    visitor->trace(m_value);
 }
 
 }

[haraken, 2014-04-03 06:10:59]

Unfortunately, this CL caused a bit serious conflict with ToT... Rebasing.

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
File Source/core/animation/KeyframeEffectModel.cpp (right):

https://codereview.chromium.org/222893005/diff/1/Source/core/animation/Keyfra...
Source/core/animation/KeyframeEffectModel.cpp:39: bool
Keyframe::compareOffsets(const RefPtrWillBeMember<Keyframe>& a, const
RefPtrWillBeMember<Keyframe>& b)
On 2014/04/03 05:17:11, zerny-chromium wrote:
> On 2014/04/03 02:03:46, kouhei wrote:
> > On 2014/04/03 02:02:05, haraken wrote:
> > > On 2014/04/03 02:01:05, kouhei wrote:
> > > > Why Member?
> > > 
> > > Because this is a callback function used in std::stable_sort that sorts
> > Members.
> > 
> > Got it.
> 
> Is the use of members needed in order to satisfy the compiler? Otherwise,
using
> pointers and relying on the Member->RawPtr conversion would be more
consistent.

Yes, it's needed for making the compiler happy.

https://codereview.chromium.org/222893005/diff/40001/Source/core/animation/Ke...
File Source/core/animation/KeyframeEffectModel.h (right):

https://codereview.chromium.org/222893005/diff/40001/Source/core/animation/Ke...
Source/core/animation/KeyframeEffectModel.h:57: class
PropertySpecificKeyframeGroup : public
NoBaseWillBeGarbageCollected<PropertySpecificKeyframeGroup> {
On 2014/04/03 05:17:11, zerny-chromium wrote:
> nit: FINAL
> 
> (Seems reasonable to add when we add a non-virtual trace)

Done.

[haraken, 2014-04-03 06:14:33]

Oh, the original CL was reverted in r170732.

dstockwell@, shanes@, rafaelw@: Would you consider merging this CL into your
original CL when you reland the original Cl?

[Mads Ager (chromium), 2014-04-03 06:22:50]

lgtm2

[shans, 2014-04-03 10:16:14]

On 2014/04/03 06:22:50, Mads Ager (chromium) wrote:
> lgtm2

Attempting to land as part of https://codereview.chromium.org/223443002/.