cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

This makes it no longer exposed to unit tests, which helps because
Draw&Swap will be joining into a single action in LayerTreeHostImpl
since it doesn't actually "draw" anything, it just IPCs things.

Tests now have a SwapBuffersCompleteOnThread() instead, which is called
asynchronously from anything LayerTreeHostImpl is doing, so I removed
the LayerTreeHostImpl* passed to it, and changed tests to use
DrawLayersOnThread() when they need to run things at the time of
LayerTreeHostImpl doing things, and handle SwapBuffersCompleteOnThread()
being asynchronous.

This would be a bit simpler if there was no SwapBuffers after
DrawLayersOnThread() happening at all, but I work with it in the tests
anyhow.

R=enne
BUG=606056
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9c14156250323f5f994239a4597022cdd6ba9020
Cr-Commit-Position: refs/heads/master@{#405362}

[danakj, 2016-07-13 21:27:23]

Description was changed from

==========
cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

This makes it no longer exposed to unit tests, which helps because
Draw&Swap will be joining into a single action in LayerTreeHostImpl
since it doesn't actually "draw" anything, it just IPCs things.

Tests now have a SwapBuffersCompleteOnThread() instead, which is called
asynchronously from anything LayerTreeHostImpl is doing, so I removed
the LayerTreeHostImpl* passed to it, and changed tests to use
DrawLayersOnThread() when they need to run things at the time of
LayerTreeHostImpl doing things, and handle SwapBuffersCompleteOnThread()
being asynchronous.

This would be a bit simpler if there was no SwapBuffers after
DrawLayersOnThread() happening at all, but I work with it in the tests
anyhow.

R=enne
BUG=606056
==========

to

==========
cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

This makes it no longer exposed to unit tests, which helps because
Draw&Swap will be joining into a single action in LayerTreeHostImpl
since it doesn't actually "draw" anything, it just IPCs things.

Tests now have a SwapBuffersCompleteOnThread() instead, which is called
asynchronously from anything LayerTreeHostImpl is doing, so I removed
the LayerTreeHostImpl* passed to it, and changed tests to use
DrawLayersOnThread() when they need to run things at the time of
LayerTreeHostImpl doing things, and handle SwapBuffersCompleteOnThread()
being asynchronous.

This would be a bit simpler if there was no SwapBuffers after
DrawLayersOnThread() happening at all, but I work with it in the tests
anyhow.

R=enne
BUG=606056
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-07-13 21:50:18]

https://codereview.chromium.org/2151433004/diff/20001/cc/layers/surface_layer...
File cc/layers/surface_layer_unittest.cc (left):

https://codereview.chromium.org/2151433004/diff/20001/cc/layers/surface_layer...
cc/layers/surface_layer_unittest.cc:195: EXPECT_TRUE(result);
This function only happens now if swap happened, so it running at all implies
this true.

https://codereview.chromium.org/2151433004/diff/20001/cc/layers/texture_layer...
File cc/layers/texture_layer_unittest.cc (left):

https://codereview.chromium.org/2151433004/diff/20001/cc/layers/texture_layer...
cc/layers/texture_layer_unittest.cc:1069: // Source frame number during callback
is the same as the source frame
Resources are now returned asynchronously from anything LTHI is doing, so we
can't compare to the source frame number. The point of this test is that we
don't do another commit tho (the NOTREACHED() on 1103), so just removing this.

https://codereview.chromium.org/2151433004/diff/20001/cc/layers/texture_layer...
cc/layers/texture_layer_unittest.cc:1115: // Return all resources immediately.
This moved into FakeOutputSurface to happen automatically when a frame is
swapped to the previous frame, and to the last frame on shutdown.

https://codereview.chromium.org/2151433004/diff/20001/cc/output/delegating_re...
File cc/output/delegating_renderer_unittest.cc (right):

https://codereview.chromium.org/2151433004/diff/20001/cc/output/delegating_re...
cc/output/delegating_renderer_unittest.cc:54: drawn_viewport_ =
host_impl->DeviceViewport();
This saves state that was part of the draw here, to compare with what was
swapped later.

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest.cc:6682: EXPECT_EQ(2.0f,
host_impl->active_tree()->painted_device_scale_factor());
Check the tree state when we draw. Check the swapped frame only after swap.

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_animation.cc (right):

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_animation.cc:1347: // Animation state is
updated after drawing.
This one is a bit awk. ProxyImpl does Draw -> UpdateAnimationState -> Swap.

In the future I guess it'll be just Draw -> UpdateAnimationState. So we need to
do this CheckAnimations work after the update somehow. I used a post-task.
SeemsGood?

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_copyrequest.cc (left):

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_copyrequest.cc:938:
release->Run(gpu::SyncToken(), false);
Running this here means it might run before SwapBuffersComplete. So we run it in
AfterTest instead.

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_damage.cc (left):

https://codereview.chromium.org/2151433004/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_damage.cc:201: if (result)
This just isn't called if result was false. So we stop tracking the old
"did_swaps_", and EXPECT on the has_no_damage at prepare-to-draw time instead.

[danakj, 2016-07-13 21:50:42]

A few self-nits/cleanups after those comments.

[danakj, 2016-07-13 21:52:02]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 21:53:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[enne (OOO), 2016-07-13 22:51:14]

lgtm buuut

https://codereview.chromium.org/2151433004/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_damage.cc (right):

https://codereview.chromium.org/2151433004/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_damage.cc:497: ++did_swaps_;
did_swaps <_<

I hate to nit did_swaps here, but LTH tests are confusing enough as they are. 
Would you mind changing them where it makes sense?

[danakj, 2016-07-13 23:11:53]

https://codereview.chromium.org/2151433004/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_damage.cc (right):

https://codereview.chromium.org/2151433004/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_damage.cc:497: ++did_swaps_;
On 2016/07/13 22:51:14, enne wrote:
> did_swaps <_<
> 
> I hate to nit did_swaps here, but LTH tests are confusing enough as they are. 
> Would you mind changing them where it makes sense?

Done. num_draws_ instead throughout this file.

[danakj, 2016-07-13 23:12:19]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-07-13 23:12:20]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2151433004/#ps60001
(title: "novirtualswapbuffers: num_draws_")

[commit-bot: I haz the power, 2016-07-13 23:12:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-14 00:29:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-14 00:29:12]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 00:31:21]

Description was changed from

==========
cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

This makes it no longer exposed to unit tests, which helps because
Draw&Swap will be joining into a single action in LayerTreeHostImpl
since it doesn't actually "draw" anything, it just IPCs things.

Tests now have a SwapBuffersCompleteOnThread() instead, which is called
asynchronously from anything LayerTreeHostImpl is doing, so I removed
the LayerTreeHostImpl* passed to it, and changed tests to use
DrawLayersOnThread() when they need to run things at the time of
LayerTreeHostImpl doing things, and handle SwapBuffersCompleteOnThread()
being asynchronous.

This would be a bit simpler if there was no SwapBuffers after
DrawLayersOnThread() happening at all, but I work with it in the tests
anyhow.

R=enne
BUG=606056
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Make LayerTreeHostImpl::SwapBuffers not virtual.

This makes it no longer exposed to unit tests, which helps because
Draw&Swap will be joining into a single action in LayerTreeHostImpl
since it doesn't actually "draw" anything, it just IPCs things.

Tests now have a SwapBuffersCompleteOnThread() instead, which is called
asynchronously from anything LayerTreeHostImpl is doing, so I removed
the LayerTreeHostImpl* passed to it, and changed tests to use
DrawLayersOnThread() when they need to run things at the time of
LayerTreeHostImpl doing things, and handle SwapBuffersCompleteOnThread()
being asynchronous.

This would be a bit simpler if there was no SwapBuffers after
DrawLayersOnThread() happening at all, but I work with it in the tests
anyhow.

R=enne
BUG=606056
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9c14156250323f5f994239a4597022cdd6ba9020
Cr-Commit-Position: refs/heads/master@{#405362}
==========

[commit-bot: I haz the power, 2016-07-14 00:31:22]

Patchset 4 (id:??) landed as
https://crrev.com/9c14156250323f5f994239a4597022cdd6ba9020
Cr-Commit-Position: refs/heads/master@{#405362}