Blink CompositorAnimations: Add more tests for integration with CC.

Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer, 
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer
objects properly installed. 

This allows us to run Animation::preCommit 
(Animation::maybeStartAnimationOnCompositor and 
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504

[loyso (OOO), 2015-12-29 06:21:06]

Description was changed from

==========
Blink CompositorAnimations: Add more tests for integration with CC.

BUG=571504
==========

to

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and Weblayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

BUG=571504
==========

[loyso (OOO), 2015-12-29 06:52:27]

loyso@chromium.org changed reviewers:
+ dstockwell@chromium.org, shans@chromium.org, vollick@chromium.org

[loyso (OOO), 2015-12-29 06:52:27]

shans@chromium.org: Please review changes in 

vollick@chromium.org: Please review changes in 

dstockwell@chromium.org: Please review changes in

[loyso (OOO), 2015-12-29 06:55:58]

Description was changed from

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and Weblayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

BUG=571504
==========

to

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

BUG=571504
==========

[Ian Vollick, 2015-12-29 20:24:54]

vollick@chromium.org changed reviewers:
+ esprehn@chromium.org

[Ian Vollick, 2015-12-29 20:32:24]

This is really interesting. Your tests want to muck with blink to set up
animation related stuff and also want to poke at the compositor to ask it
questions. Your plan isn't unreasonable, but given that the code and rules are
in flux, I'm not sure of the best approach. I've added esprehn who may have a
plan already.

Could an approach similar to ScrollingCoordinatorTest.cpp be used?
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
 Though with this approach, if you needed to ask deeper questions of cc, you'd
have to plumb more data through platform.

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

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:1265:
animation->cancelAnimationOnCompositor();
I may be misreading the purpose of this test -- it sounded like you were
checking that if we disposed the animation, then we'd got canceled
automatically? If that's the case, should we be canceling explicitly?

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/WebLayerMock.h (right):

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/WebLayerMock.h:38: public:
How will developers know to update this if they change the WebLayer interface?

[loyso (OOO), 2015-12-29 23:54:12]

On 2015/12/29 20:32:24, vollick wrote:
> This is really interesting. Your tests want to muck with blink to set up
> animation related stuff and also want to poke at the compositor to ask it
> questions. Your plan isn't unreasonable, but given that the code and rules are
> in flux, I'm not sure of the best approach. I've added esprehn who may have a
> plan already.
Mmmm. Not the compositor. Just the compositor-related orphaned objects.
Surprisingly this is enough to exercise blink::Animation code. 

> Could an approach similar to ScrollingCoordinatorTest.cpp be used?
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
>  Though with this approach, if you needed to ask deeper questions of cc, you'd
> have to plumb more data through platform.
I think you would need a whole LayerTreeView setup (cc stuff:
LTH/ThreadedProxy/LTHI)

[loyso (OOO), 2015-12-29 23:54:25]

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

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp:1265:
animation->cancelAnimationOnCompositor();
On 2015/12/29 20:32:24, vollick wrote:
> I may be misreading the purpose of this test -- it sounded like you were
> checking that if we disposed the animation, then we'd got canceled
> automatically? If that's the case, should we be canceling explicitly?
That was just my assumption about the crash (ideas from Doug needed).
Yes, we can call cancelAnimationOnCompositor in dispose. But we still call
cancelAnimationOnCompositor from many other different places.
So, I made it idempotent (see landed 'depend-on' patch set). And this is the
test.

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/WebLayerMock.h (right):

https://codereview.chromium.org/1550243002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/WebLayerMock.h:38: public:
On 2015/12/29 20:32:24, vollick wrote:
> How will developers know to update this if they change the WebLayer interface?
webkit_unit_tests target will fail to compile, AFAIU?

[loyso (OOO), 2015-12-29 23:55:23]

Description was changed from

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

BUG=571504
==========

to

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504
==========

[esprehn, 2016-01-04 21:10:49]

Description was changed from

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer,
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer objects
properly installed.

This allows us to run Animation::preCommit
(Animation::maybeStartAnimationOnCompositor and
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504
==========

to

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer, 
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer
objects properly installed. 

This allows us to run Animation::preCommit 
(Animation::maybeStartAnimationOnCompositor and 
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504
==========

[esprehn, 2016-01-04 21:15:21]

This is a ton of mocks, I'm not sure we should do this. I'm also not sure we
should be using more of this LayoutObjectProxy thing, calling setLayoutObject()
outside recalcStyle like this is a bug. This whole test setup doesn't seem
right, it should use DummyPageHolder or the SimTest framework, and we should
implement proper mocks and reuse existing things, not add 400 lines of new
mocks.

https://codereview.chromium.org/1550243002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/animation/AnimationTranslationUtilTest.h
(right):

https://codereview.chromium.org/1550243002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/animation/AnimationTranslationUtilTest.h:2: *
Copyright (C) 2015 Google Inc. All rights reserved.
short copyright

https://codereview.chromium.org/1550243002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/testing/WebLayerMock.h (right):

https://codereview.chromium.org/1550243002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/WebLayerMock.h:2: * Copyright (C)
2015 Google Inc. All rights reserved.
use short copyright for all new files

[loyso (OOO), 2016-01-18 23:36:28]

On 2016/01/04 21:15:21, esprehn wrote:
> This is a ton of mocks, I'm not sure we should do this. I'm also not sure we
> should be using more of this LayoutObjectProxy thing, calling
setLayoutObject()
> outside recalcStyle like this is a bug. This whole test setup doesn't seem
> right, it should use DummyPageHolder or the SimTest framework

I'm trying to upload a Sim version of this test.
The linter says:
  third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
      Illegal include: "web/tests/sim/SimCompositor.h"
    Because of "-web" from third_party/WebKit/Source/core's include_rules.

Should we allow it for core/ ?

[esprehn, 2016-01-20 02:32:22]

esprehn@chromium.org changed reviewers:
+ haraken@chromium.org

[esprehn, 2016-01-20 02:32:48]

On 2016/01/18 at 23:36:28, loyso wrote:
> On 2016/01/04 21:15:21, esprehn wrote:
> > This is a ton of mocks, I'm not sure we should do this. I'm also not sure we
> > should be using more of this LayoutObjectProxy thing, calling
setLayoutObject()
> > outside recalcStyle like this is a bug. This whole test setup doesn't seem
> > right, it should use DummyPageHolder or the SimTest framework
> 
> I'm trying to upload a Sim version of this test.
> The linter says:
>   third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
>       Illegal include: "web/tests/sim/SimCompositor.h"
>     Because of "-web" from third_party/WebKit/Source/core's include_rules.
> 
> Should we allow it for core/ ?

You cant use that from core/ you should put your tests in web/ with the other
tests for big systems there. 

We hope to move much of web into core but the core/modules split made that
complicated.

[loyso (OOO), 2016-02-23 06:04:18]

Closing this CL (to use https://codereview.chromium.org/1727443002/ instead)

[loyso (OOO), 2016-02-23 06:04:27]

Description was changed from

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer, 
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer
objects properly installed. 

This allows us to run Animation::preCommit 
(Animation::maybeStartAnimationOnCompositor and 
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504
==========

to

==========
Blink CompositorAnimations: Add more tests for integration with CC.

This is a significant enhancement for our test coverage.
Now we can test blink Elements with LayoutObject, PaintLayer, 
CompositedLayerMapping, GraphicsLayer, WebContentLayer and WebLayer
objects properly installed. 

This allows us to run Animation::preCommit 
(Animation::maybeStartAnimationOnCompositor and 
CompositorAnimations::startAnimationOnCompositor)

No more 'simulation' and 'ForTesting'.

The fix itself: https://codereview.chromium.org/1543143002

BUG=571504
==========