Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/d30a199df83684947548304937ce3ca440a53811
Cr-Commit-Position: refs/heads/master@{#397426}

[loyso (OOO), 2016-05-27 05:43:07]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Erase gmock usage, check CC guts instead in the integrational unit tests.

This will allow us to erase CompositorFactory.

BUG=577016
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Erase gmock usage, check CC guts instead in the integrational unit tests.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[loyso (OOO), 2016-05-27 05:43:58]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Erase gmock usage, check CC guts instead in the integrational unit tests.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Erase gmock usage, check CC guts instead in the integrational unit tests.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[loyso (OOO), 2016-05-27 05:43:58]

loyso@chromium.org changed reviewers:
+ ajuma@chromium.org, alancutter@chromium.org, jbroman@chromium.org

[loyso (OOO), 2016-05-27 05:59:04]

Could you review specific areas, please?
ajuma@, cc/
alancutter@, core/animation and platform/animation
jbroman@, testing/CompositorAnimationsTestUtils.h, blink_platform.gypi and
core.gypi
Thanks!

[loyso (OOO), 2016-05-27 06:02:08]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

Erase gmock usage, check CC guts instead in the integrational unit tests.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check CC guts
instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ajuma, 2016-05-27 13:21:04]

cc/ lgtm

[jbroman, 2016-05-27 14:52:55]

I'm not a huge fan of checking cc guts within core; WDYT of accessing it through
code (not just type aliases) in platform?

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:64: //
CompositorFloatKeyframe is a plain struct, so we just create an == operator
Not introduced in this CL, but obligatory warning: this has the potential to
cause ODR issues down the road if operator== or PrintTo for
CompositorFloatKeyframe are ever wanted elsewhere. You can still have the print
function only compiled into unit tests; see e.g. platform/geometry/FloatPoint.h
+ platform/testing/GeometryPrinters.{h,cpp}.

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:268:
DCHECK_EQ(1U, result.size());
Here and elsewhere, you want ASSERT_EQ, not DCHECK_EQ. While Blink's ASSERT is
going away in favour of DCHECK, ASSERT_EQ is from gtest and is still good (marks
the test as failed instead of crashing the test runner, for instance).

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h:13:
using TimeDeltaForTesting = base::TimeDelta;
This doesn't seem super helpful and makes the Source/platform/ abstraction super
leaky, even if only for testing. I don't know that referring to
"CcAnimationForTesting" is any better than "cc::Animation" (except it's harder
to read and adds a weird indirection that doesn't actually hide the interface).

I'd much rather you provide actual testing utilities in platform; ideally ones
that operated on a blink::CompositorAnimation (etc) rather than cc::Animation.
For example:


std::unique_ptr<CcAnimationForTesting> animation =
convertToCompositorAnimation(*effect);
EXPECT_EQ(CompositorTargetProperty::OPACITY, animation->target_property());
// ...

could instead be written as

OwnPtr<CompositorAnimation> animation = convertToCompositorAnimation(*effect);
EXPECT_EQ(CompositorTargetProperty::OPACITY, animation->targetProperty());
// ...


const CcKeyframedFloatAnimationCurveForTesting* keyframedFloatCurve =
ToFloatCurve(animation.get());
DCHECK_EQ(2UL, keyframedFloatCurve->keyframes_for_testing().size());
const CcFloatKeyframeForTesting* keyframe1 =
keyframedFloatCurve->keyframes_for_testing()[0].get();
const CcFloatKeyframeForTesting* keyframe2 =
keyframedFloatCurve->keyframes_for_testing()[1].get();
EXPECT_EQ(TimeDeltaForTesting(), keyframe1->Time());
EXPECT_EQ(2.0f, keyframe1->Value());

could instead be written as

// first two lines could be grouped into a floatCurveKeyframesForAnimation(const
CompositorAnimation&).
OwnPtr<CompositorFloatAnimationCurve> floatAnimationCurve =
animation->floatCurveForTesting();
Vector<CompositorFloatKeyframe> keyframes =
floatAnimationCurve->keyframesForTesting();
ASSERT_EQ(2UL, keyframes.size());
EXPECT_EQ(0.0, keyframes[0].time);
EXPECT_EQ(2.0f, keyframes[0].value);

or perhaps, if you prefer, you could move some of the checks into helpers using
functions (returning bools or testing::AssertionResult) or gmock matchers


If you're worried about whether these accessors faithfully represent the
underlying cc::Animation, that should be tested in platform.

Alternatively, if some of the logic of that test doesn't require core, perhaps
it can be moved to blink_platform_unittests, where accessing underlying types is
appropriate.

If we do want to use base/cc types in core for testing, I'd rather see a
discussion about allowing that than a ton of type aliases.

[loyso (OOO), 2016-05-31 02:20:39]

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:64: //
CompositorFloatKeyframe is a plain struct, so we just create an == operator
On 2016/05/27 14:52:55, jbroman wrote:
> Not introduced in this CL, but obligatory warning: this has the potential to
> cause ODR issues down the road if operator== or PrintTo for
> CompositorFloatKeyframe are ever wanted elsewhere. You can still have the
print
> function only compiled into unit tests; see e.g.
platform/geometry/FloatPoint.h
> + platform/testing/GeometryPrinters.{h,cpp}.

Acknowledged. These particular helpers were not used :)

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:268:
DCHECK_EQ(1U, result.size());
On 2016/05/27 14:52:55, jbroman wrote:
> Here and elsewhere, you want ASSERT_EQ, not DCHECK_EQ. While Blink's ASSERT is
> going away in favour of DCHECK, ASSERT_EQ is from gtest and is still good
(marks
> the test as failed instead of crashing the test runner, for instance).
Partly done.

It looks like it doesn't work in function with return values.

error: no viable conversion from returned value of type 'void' to function
return type 'std::unique_ptr<CcAnimationForTesting>'

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h:13:
using TimeDeltaForTesting = base::TimeDelta;
On 2016/05/27 14:52:55, jbroman wrote:
> This doesn't seem super helpful and makes the Source/platform/ abstraction
super
> leaky, even if only for testing. I don't know that referring to
> "CcAnimationForTesting" is any better than "cc::Animation" (except it's harder
> to read and adds a weird indirection that doesn't actually hide the
interface).
> 
> I'd much rather you provide actual testing utilities in platform; ideally ones
> that operated on a blink::CompositorAnimation (etc) rather than cc::Animation.
> For example:
> 
> 
> std::unique_ptr<CcAnimationForTesting> animation =
> convertToCompositorAnimation(*effect);
> EXPECT_EQ(CompositorTargetProperty::OPACITY, animation->target_property());
> // ...
> 
> could instead be written as
> 
> OwnPtr<CompositorAnimation> animation = convertToCompositorAnimation(*effect);
> EXPECT_EQ(CompositorTargetProperty::OPACITY, animation->targetProperty());
> // ...
> 
> 
> const CcKeyframedFloatAnimationCurveForTesting* keyframedFloatCurve =
> ToFloatCurve(animation.get());
> DCHECK_EQ(2UL, keyframedFloatCurve->keyframes_for_testing().size());
> const CcFloatKeyframeForTesting* keyframe1 =
> keyframedFloatCurve->keyframes_for_testing()[0].get();
> const CcFloatKeyframeForTesting* keyframe2 =
> keyframedFloatCurve->keyframes_for_testing()[1].get();
> EXPECT_EQ(TimeDeltaForTesting(), keyframe1->Time());
> EXPECT_EQ(2.0f, keyframe1->Value());
> 
> could instead be written as
> 
> // first two lines could be grouped into a
floatCurveKeyframesForAnimation(const
> CompositorAnimation&).
> OwnPtr<CompositorFloatAnimationCurve> floatAnimationCurve =
> animation->floatCurveForTesting();
> Vector<CompositorFloatKeyframe> keyframes =
> floatAnimationCurve->keyframesForTesting();
> ASSERT_EQ(2UL, keyframes.size());
> EXPECT_EQ(0.0, keyframes[0].time);
> EXPECT_EQ(2.0f, keyframes[0].value);
> 
> or perhaps, if you prefer, you could move some of the checks into helpers
using
> functions (returning bools or testing::AssertionResult) or gmock matchers
> 
> 
> If you're worried about whether these accessors faithfully represent the
> underlying cc::Animation, that should be tested in platform.
> 
> Alternatively, if some of the logic of that test doesn't require core, perhaps
> it can be moved to blink_platform_unittests, where accessing underlying types
is
> appropriate.
> 
> If we do want to use base/cc types in core for testing, I'd rather see a
> discussion about allowing that than a ton of type aliases.

Done.

[loyso (OOO), 2016-05-31 02:21:22]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check CC guts
instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform
compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[loyso (OOO), 2016-05-31 04:04:59]

alancutter@, PTAL!

[loyso (OOO), 2016-05-31 23:52:01]

PTAL.

[alancutter (OOO until 2018), 2016-06-01 02:20:56]

Happy to see gmock tests being replaced.

https://codereview.chromium.org/2019613002/diff/1/cc/animation/timing_functio...
File cc/animation/timing_function.cc (right):

https://codereview.chromium.org/2019613002/diff/1/cc/animation/timing_functio...
cc/animation/timing_function.cc:91: }
Looks like we can delete these classes now (in another patch).

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h:21:
#endif // CompositorAnimationsTestUtils_h
This is only included once, why have a separate file?

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:668:
EXPECT_EQ(CubicBezierTimingFunction::EaseType::EASE,
keyframedFloatCurve->getKeyframeEaseTypeForTesting(1));
We should continue to test that the first keyframe is linear.

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:937:
EXPECT_EQ(CompositorAnimation::FillMode::NONE, animation->getFillMode());
It would be okay to just test the fill mode here but more coverage is fine too.

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:61:
auto cubicTimingFunction = static_cast<const
cc::CubicBezierTimingFunction*>(timingFunction);
Need a check that this static_cast is correct.

[loyso (OOO), 2016-06-01 02:36:47]

https://codereview.chromium.org/2019613002/diff/1/cc/animation/timing_functio...
File cc/animation/timing_function.cc (right):

https://codereview.chromium.org/2019613002/diff/1/cc/animation/timing_functio...
cc/animation/timing_function.cc:91: }
On 2016/06/01 02:20:55, alancutter wrote:
> Looks like we can delete these classes now (in another patch).
Yes, that was the plan. EaseInOutTimingFunction is static-only helper, unrelated
to base TimingFunction.

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h:21:
#endif // CompositorAnimationsTestUtils_h
On 2016/06/01 02:20:55, alancutter wrote:
> This is only included once, why have a separate file?
Irrelevant with a new patch set.

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:937:
EXPECT_EQ(CompositorAnimation::FillMode::NONE, animation->getFillMode());
On 2016/06/01 02:20:55, alancutter wrote:
> It would be okay to just test the fill mode here but more coverage is fine
too.

Acknowledged.

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:61:
auto cubicTimingFunction = static_cast<const
cc::CubicBezierTimingFunction*>(timingFunction);
On 2016/06/01 02:20:55, alancutter wrote:
> Need a check that this static_cast is correct.
This would require an extra data member in cc::TimingFunction type_ to store the
enum class { LINEAR, CUBIC_BEZIER, STEPS }; (we don't have it now)
That's an overhead of 4 bytes (?) pee curve and per keyframe.
ajuma@ is it ok? what do you think?

[loyso (OOO), 2016-06-01 02:38:21]

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/testing/CompositorAnimationsTestUtils.h:21:
#endif // CompositorAnimationsTestUtils_h
On 2016/06/01 02:20:55, alancutter wrote:
> This is only included once, why have a separate file?
The actual reason was the folder, where is file is located. You are allowed to
include and use cc only in platform/animation.

[ajuma, 2016-06-01 13:16:06]

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:61:
auto cubicTimingFunction = static_cast<const
cc::CubicBezierTimingFunction*>(timingFunction);
On 2016/06/01 02:36:47, loyso wrote:
> On 2016/06/01 02:20:55, alancutter wrote:
> > Need a check that this static_cast is correct.
> This would require an extra data member in cc::TimingFunction type_ to store
the
> enum class { LINEAR, CUBIC_BEZIER, STEPS }; (we don't have it now)
> That's an overhead of 4 bytes (?) pee curve and per keyframe.
> ajuma@ is it ok? what do you think?

Yeah, that sounds fine.

[jbroman, 2016-06-01 14:51:28]

There's now only one file that's not covered by other owners, so, uh, core.gypi
LGTM. :)

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:268:
DCHECK_EQ(1U, result.size());
On 2016/05/31 at 02:20:39, loyso wrote:
> On 2016/05/27 14:52:55, jbroman wrote:
> > Here and elsewhere, you want ASSERT_EQ, not DCHECK_EQ. While Blink's ASSERT
is
> > going away in favour of DCHECK, ASSERT_EQ is from gtest and is still good
(marks
> > the test as failed instead of crashing the test runner, for instance).
> Partly done.
> 
> It looks like it doesn't work in function with return values.
> 
> error: no viable conversion from returned value of type 'void' to function
return type 'std::unique_ptr<CcAnimationForTesting>'

Fair enough.

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:256:
return adoptPtr(result[0].leakPtr());
Please avoid using leakPtr unless you need a raw pointer. I'd suggest "return
std::move(result[0]);" instead.

Previously OwnPtr::release was the way to go, but yutak is midway through
migrating us to unique_ptr, and std::move is the way to do it there.

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:37:
for (auto& ccKeyframe : m_curve->keyframes_for_testing()) {
nit: prefer "const auto&" unless you're mutation (keyframes_for_testing is const
so it's the same anyway, but "const auto&" is more reassuring that nothing funny
is going on)

[loyso (OOO), 2016-06-02 02:16:46]

It's final. PTAL!

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:61:
auto cubicTimingFunction = static_cast<const
cc::CubicBezierTimingFunction*>(timingFunction);
On 2016/06/01 13:16:06, ajuma wrote:
> On 2016/06/01 02:36:47, loyso wrote:
> > On 2016/06/01 02:20:55, alancutter wrote:
> > > Need a check that this static_cast is correct.
> > This would require an extra data member in cc::TimingFunction type_ to store
> the
> > enum class { LINEAR, CUBIC_BEZIER, STEPS }; (we don't have it now)
> > That's an overhead of 4 bytes (?) pee curve and per keyframe.
> > ajuma@ is it ok? what do you think?
> 
> Yeah, that sounds fine.
Done. Without overhead. :)

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
(right):

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:256:
return adoptPtr(result[0].leakPtr());
On 2016/06/01 14:51:28, jbroman wrote:
> Please avoid using leakPtr unless you need a raw pointer. I'd suggest "return
> std::move(result[0]);" instead.
> 
> Previously OwnPtr::release was the way to go, but yutak is midway through
> migrating us to unique_ptr, and std::move is the way to do it there.

Done.

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp
(right):

https://codereview.chromium.org/2019613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/animation/CompositorFloatAnimationCurve.cpp:37:
for (auto& ccKeyframe : m_curve->keyframes_for_testing()) {
On 2016/06/01 14:51:28, jbroman wrote:
> nit: prefer "const auto&" unless you're mutation (keyframes_for_testing is
const
> so it's the same anyway, but "const auto&" is more reassuring that nothing
funny
> is going on)

Done.

[alancutter (OOO until 2018), 2016-06-02 04:03:07]

lgtm

[loyso (OOO), 2016-06-02 06:32:06]

The CQ bit was checked by loyso@chromium.org

[loyso (OOO), 2016-06-02 06:32:07]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org,
jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2019613002/#ps80001
(title: "Fix codereview issues. Add cc::TimingFunction::Type enum.")

[commit-bot: I haz the power, 2016-06-02 06:32:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2019613002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2019613002/80001

[commit-bot: I haz the power, 2016-06-02 09:24:55]

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

[commit-bot: I haz the power, 2016-06-02 09:24:56]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[loyso (OOO), 2016-06-02 12:09:45]

The CQ bit was checked by loyso@chromium.org

[commit-bot: I haz the power, 2016-06-02 12:09:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2019613002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2019613002/80001

[commit-bot: I haz the power, 2016-06-02 15:59:40]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform
compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform
compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-02 15:59:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-02 16:01:09]

Description was changed from

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform
compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Blink Compositor Animation: Make Animation and Curve methods non-virtual.

In unit tests for blink/cc integration: Erase gmock usage, check platform
compositor data instead.

This will allow us to erase CompositorFactory.

BUG=577016
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/d30a199df83684947548304937ce3ca440a53811
Cr-Commit-Position: refs/heads/master@{#397426}
==========

[commit-bot: I haz the power, 2016-06-02 16:01:10]

Patchset 5 (id:??) landed as
https://crrev.com/d30a199df83684947548304937ce3ca440a53811
Cr-Commit-Position: refs/heads/master@{#397426}