Delay activation/draw on GPU tile work completion

Delay activation/draw on GPU tile work completion

Design doc: https://goo.gl/Ku9VBh

This change delays the activation of the pending tree as well as
drawing of active tree tiles on the completion of the required GL
work in the GPU process. This differs from our current
approach, where we only delay activation/draw on the issuing of GL
commands in the renderer, not on the commands being executed in the
GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

BUG=673434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2555743004
Cr-Commit-Position: refs/heads/master@{#447346}
Committed: https://chromium.googlesource.com/chromium/src/+/7f6a27f1dbe4d8cafffb919445bc93bb8867a8fe

[ericrk, 2016-12-07 01:39:33]

Description was changed from

==========
WIP Wait for sync token to activate

BUG=
==========

to

==========
WIP Wait for sync token to activate

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ericrk, 2016-12-07 18:54:04]

Description was changed from

==========
WIP Wait for sync token to activate

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Delay activation on GPU tile work completion

This change delays the activation of the pending tree on the completion
of the required GL work in the GPU process. This differs from our current
approach, where we only delay activation on the issuing of GL commands in
the renderer, not on the commands being executed in the GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

As waiting on GPU work only applies to those RasterBufferProviders which
perform GPU work, this change introduces a new method on
RasterBufferProvider, "SetReadyToDrawCallback". This function takes a
list of resources and a callback to trigger when all of those resources
are ready to draw.

TileManager now waits on the callback before firing the ready to activate
signals.

BUG=514813
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ericrk, 2016-12-07 18:54:04]

ericrk@chromium.org changed reviewers:
+ sunnyps@chromium.org, vmiura@chromium.org, vmpstr@chromium.org

[ericrk, 2016-12-07 19:13:37]

I've taken a shot at the delay-activation part of the client-side scheduler
logic. Delaying drawing of tiles in the active tree will come in a follow-up CL.
PTAL. Thanks!

[vmpstr, 2016-12-07 21:00:25]

https://codereview.chromium.org/2555743004/diff/80001/cc/raster/one_copy_rast...
File cc/raster/one_copy_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/80001/cc/raster/one_copy_rast...
cc/raster/one_copy_raster_buffer_provider.cc:177: if (callback_id !=
pending_callback_id) {
Can you add a comment explaining what's going on here?

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:448: // activation
nit: reformat plz

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:454: ResourceProvider::ResourceIdArray array;
nit: resource_ids

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:455: auto
raster_priority_queue(client_->BuildRasterQueue(
nit: I kinda prefer = instead of ()

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:461: if (!draw_info.has_resource()) {
Can you verify that OOM case is still handled. I think it should be in all tiles
finished that we mark tiles as oom, so it should be OK (also might be special
logic in smoothness mode). However, if OOM tiles block activation because of
this, it isn't good :)

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:355: base::WeakPtrFactory<TileManager>
weak_ptr_factory_;
I'm assuming the two weakptrfactories here have different invalidation times?

If we do another schedule, we should invalidate the task set finished one.
However, should we still check if we can activate if we receive the sync token
callback?

I'm wondering whether we should also invalidate this one on new schedule,
because we're going to get another ready to activate signal. If so, then we only
really need one WeakPtrFactory

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager_u...
File cc/tiles/tile_manager_unittest.cc (right):

https://codereview.chromium.org/2555743004/diff/80001/cc/tiles/tile_manager_u...
cc/tiles/tile_manager_unittest.cc:1958: }
Can you add a test for non-smoothness mode as well to make sure it doesn't wait
for anything

[ericrk, 2016-12-08 00:08:55]

Discussed offline with vmpstr@, I think there's more complexity here than I
realized. Writing up a doc and revising this design.

[ericrk, 2016-12-12 23:35:52]

Description was changed from

==========
Delay activation on GPU tile work completion

This change delays the activation of the pending tree on the completion
of the required GL work in the GPU process. This differs from our current
approach, where we only delay activation on the issuing of GL commands in
the renderer, not on the commands being executed in the GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

As waiting on GPU work only applies to those RasterBufferProviders which
perform GPU work, this change introduces a new method on
RasterBufferProvider, "SetReadyToDrawCallback". This function takes a
list of resources and a callback to trigger when all of those resources
are ready to draw.

TileManager now waits on the callback before firing the ready to activate
signals.

BUG=514813
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Delay activation/draw on GPU tile work completion

Design doc: https://goo.gl/Ku9VBh

This change delays the activation of the pending tree as well as
drawing of active tree tiles on the completion of the required GL
work in the GPU process. This differs from our current
approach, where we only delay activation/draw on the issuing of GL
commands in the renderer, not on the commands being executed in the
GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

BUG=673434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ericrk, 2016-12-13 18:22:12]

Updated the CL to handle the missing cases from v1. Also wrote up a design doc
discussing the design: https://goo.gl/Ku9VBh

[ericrk, 2016-12-13 18:26:04]

rebase

[ericrk, 2016-12-13 19:53:29]

comments

[vmpstr, 2016-12-13 22:47:51]

I looked at TM only for now

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:423: auto found =
std::find(pending_ready_for_draw_tiles_.begin(),
You can just check for null resources in the check function (as discussed, there
can be tiles that are evicted, which lose their resource although the tile
itself remains)

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1057: pending_ready_for_draw_tiles_.push_back(tile);
You can immediately check if the resource is already ready here, so we may skip
pushing it to this list.

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1060: draw_info.set_was_ever_ready_to_draw();
You can fold this call into set_resource_ready_to_draw (since we would always
call it, it's a good spot to flip that flag)

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1293: DCHECK(resource);
This may fail

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1306: tile->set_required_for_activation(
can you instead add a tile function like "UpdateRequiredState" which would do
this?

As an aside, you can also use that function in the iterators instead of whatever
it currently does

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1311: if (tile->required_for_activation()) {
braces optional here

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1376: // Thus, member variables are signed.
xD

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1389: // size).
xD

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
cc/tiles/tile_manager.h:155: draw_info.set_is_resource_ready_for_draw();
s/is_//

[ericrk, 2016-12-13 23:10:13]

comments

[ericrk, 2016-12-13 23:57:29]

Patchset #10 (id:180001) has been deleted

[ericrk, 2016-12-14 00:31:12]

Patchset #9 (id:160001) has been deleted

[ericrk, 2016-12-14 00:36:43]

comments

[ericrk, 2016-12-14 00:48:45]

Updated. PTAL. Thanks!

On 2016/12/13 22:47:51, vmpstr wrote:
> I looked at TM only for now
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager.cc
> File cc/tiles/tile_manager.cc (right):
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:423: auto found =
> std::find(pending_ready_for_draw_tiles_.begin(),
> You can just check for null resources in the check function (as discussed,
there
> can be tiles that are evicted, which lose their resource although the tile
> itself remains)
Done.
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1057: pending_ready_for_draw_tiles_.push_back(tile);
> You can immediately check if the resource is already ready here, so we may
skip
> pushing it to this list.
Makes sense, check added.
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1060: draw_info.set_was_ever_ready_to_draw();
> You can fold this call into set_resource_ready_to_draw (since we would always
> call it, it's a good spot to flip that flag)
Done.
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1293: DCHECK(resource);
> This may fail
We now handle this case.
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1306: tile->set_required_for_activation(
> can you instead add a tile function like "UpdateRequiredState" which would do
> this?
There was one on PictureLayerTiling (private), made it public.
> 
> As an aside, you can also use that function in the iterators instead of
whatever
> it currently does
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1311: if (tile->required_for_activation()) {
> braces optional here
Done.
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1376: // Thus, member variables are signed.
> xD
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.cc:1389: // size).
> xD
> 
> https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager.h
> File cc/tiles/tile_manager.h (right):
> 
>
https://codereview.chromium.org/2555743004/diff/160001/cc/tiles/tile_manager....
> cc/tiles/tile_manager.h:155: draw_info.set_is_resource_ready_for_draw();
> s/is_//
Done

[vmpstr, 2016-12-14 18:03:31]

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:520: if (!pending_ready_for_draw_tiles_.empty())
If it's empty, then we won't do much work anyway. How about we just set it
unconditionally?

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:791:
resource_pool_->ReleaseResource(draw_info.resource_);
Can you add a TakeResource to the draw info that automatically sets it to null?
I'd like to move towards having draw_info.resource_ not be accessible directly
by the tile manager, since it's becoming easy to get the usage wrong.

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1287: Resource* resource = tile->draw_info().resource_;
Can you change this to a getter instead?

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1338: signals_.ready_to_activate =
I think there might be some counter-intuitive interaction here:

Suppose we get a DidFinishRunningTileTasksRequiredForDraw, with
DidFinishRunningTileTasksRequiredForActivation still pending (but about to run):

- The calling code would set ready_to_draw to true, ready_to_activate would be
false and we're in CheckAndIssueSignals
- This function is called and it determines (say we're not in smoothness mode)
that ready_to_activate is true and sets as such
- The AreTilesReady code runs and issues an activation signal
- Then, DidFinishRunningTileTasksRequiredForActivation runs and unconditionally
sets ready_to_activate to true again and runs the same code again
- This would issue a second activation (bad?)

I wonder if these setters should be ... &= ... and in the sync token callback
set both signals to true... although that might still issue a second activation
if there's a pending raster task signal, no?

[ericrk, 2016-12-14 19:52:51]

comments

[ericrk, 2016-12-14 19:53:48]

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:520: if (!pending_ready_for_draw_tiles_.empty())
On 2016/12/14 18:03:31, vmpstr wrote:
> If it's empty, then we won't do much work anyway. How about we just set it
> unconditionally?

true enough...

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:791:
resource_pool_->ReleaseResource(draw_info.resource_);
On 2016/12/14 18:03:31, vmpstr wrote:
> Can you add a TakeResource to the draw info that automatically sets it to
null?
> I'd like to move towards having draw_info.resource_ not be accessible directly
> by the tile manager, since it's becoming easy to get the usage wrong. 

sure, added.

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1287: Resource* resource = tile->draw_info().resource_;
On 2016/12/14 18:03:31, vmpstr wrote:
> Can you change this to a getter instead?

Done.

https://codereview.chromium.org/2555743004/diff/200001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1338: signals_.ready_to_activate =
On 2016/12/14 18:03:31, vmpstr wrote:
> I think there might be some counter-intuitive interaction here:
> 
> Suppose we get a DidFinishRunningTileTasksRequiredForDraw, with
> DidFinishRunningTileTasksRequiredForActivation still pending (but about to
run):
> 
> - The calling code would set ready_to_draw to true, ready_to_activate would be
> false and we're in CheckAndIssueSignals
> - This function is called and it determines (say we're not in smoothness mode)
> that ready_to_activate is true and sets as such
> - The AreTilesReady code runs and issues an activation signal
> - Then, DidFinishRunningTileTasksRequiredForActivation runs and
unconditionally
> sets ready_to_activate to true again and runs the same code again
> - This would issue a second activation (bad?)
> 
> I wonder if these setters should be ... &= ... and in the sync token callback
> set both signals to true... although that might still issue a second
activation
> if there's a pending raster task signal, no?

Looked through the code. I believe this is prevented by
Signals::did_notify_ready_to_activate_, which blocks further activations until
it is reset. We only reset it in PrepareTiles, so I think this is fine.

[ericrk, 2016-12-14 22:46:32]

rebase

[ericrk, 2016-12-21 19:58:48]

On 2016/12/14 22:46:32, ericrk wrote:
> rebase

ping, vmpstr@, can you take a look - I think this is good. Thanks!

[ericrk, 2016-12-28 19:48:22]

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

[commit-bot: I haz the power, 2016-12-28 19:48:42]

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

[ericrk, 2016-12-28 19:49:06]

Patchset #13 (id:280001) has been deleted

[commit-bot: I haz the power, 2016-12-28 19:49:56]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[ericrk, 2016-12-28 19:51:52]

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

[commit-bot: I haz the power, 2016-12-28 19:52:02]

Dry run: 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-12-28 20:01:50]

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

[commit-bot: I haz the power, 2016-12-28 20:01:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ericrk, 2016-12-28 20:04:25]

Patchset #13 (id:300001) has been deleted

[ericrk, 2016-12-28 20:17:01]

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

[commit-bot: I haz the power, 2016-12-28 20:17:17]

Dry run: 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-12-28 21:07:50]

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

[commit-bot: I haz the power, 2016-12-28 21:07:51]

Dry run: This issue passed the CQ dry run.

[vmiura, 2017-01-05 01:54:41]

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/gpu_raster_b...
File cc/raster/gpu_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/gpu_raster_b...
cc/raster/gpu_raster_buffer_provider.cc:201: sync_token.release_count());
This should be checking the release count on worker_context_provider_.

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/one_copy_ras...
File cc/raster/one_copy_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/one_copy_ras...
cc/raster/one_copy_raster_buffer_provider.cc:176: sync_token.release_count());
Ditto.

[ericrk, 2017-01-05 21:19:56]

rebase

[ericrk, 2017-01-05 21:20:40]

Thanks for the catch. Updated.

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/gpu_raster_b...
File cc/raster/gpu_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/gpu_raster_b...
cc/raster/gpu_raster_buffer_provider.cc:201: sync_token.release_count());
On 2017/01/05 01:54:41, vmiura wrote:
> This should be checking the release count on worker_context_provider_.

Good catch. Thanks!

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/one_copy_ras...
File cc/raster/one_copy_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/320001/cc/raster/one_copy_ras...
cc/raster/one_copy_raster_buffer_provider.cc:176: sync_token.release_count());
On 2017/01/05 01:54:41, vmiura wrote:
> Ditto.

Done.

[ericrk, 2017-01-05 21:20:46]

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

[commit-bot: I haz the power, 2017-01-05 21:21:05]

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

[vmpstr, 2017-01-05 22:00:47]

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/gpu_raster_b...
File cc/raster/gpu_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/gpu_raster_b...
cc/raster/gpu_raster_buffer_provider.cc:217: // If the callback we are about to
request has the same ID as the one which
It's a bit awkward that this comment is talking about the "else" case. I think
it's fine, but maybe rephrase it as "If callback_id is different, then ...;
Otherwise, the request is redundant"

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/one_copy_ras...
File cc/raster/one_copy_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/one_copy_ras...
cc/raster/one_copy_raster_buffer_provider.cc:193: // If the callback we are
about to request has the same ID as the one which
Same here.

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/raster_buffe...
File cc/raster/raster_buffer_provider.h (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/raster_buffe...
cc/raster/raster_buffer_provider.h:74: // avoid creating a new callback
unnecessarily.
Can you specify to pass 0 if no pending callback exists? Also, is this something
that the RasterBufferProvider (or its overrides) can keep track of? That is, it
should know the last callback id it returned, no?

Or is there something more subtle here?

https://codereview.chromium.org/2555743004/diff/360001/cc/resources/resource_...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/resources/resource_...
cc/resources/resource_provider.cc:975: gpu::SyncToken
ResourceProvider::GetSyncTokenForResource(
This seems to be a special case of the function below. Do we need both?

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_draw_inf...
File cc/tiles/tile_draw_info.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_draw_inf...
cc/tiles/tile_draw_info.cc:32: if (!resource_)
Don't need this if.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:427: // the
Reformat plz

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:440: if (tile->HasRasterTask()) {
Can you file a bug to clean this up please? We changed the raster task
ownership, so afaik we don't actually need to retain the tiles since the task
will still be alive even if it's pending completion/rasterization.

Or am I missing stuff again?

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1091: // In SMOOTHNESS_TAKES_PRIORITY mode, we wait for
GPU work to complete
Move this comment outside of the if plz

Also consider doing this pattern, but it's really up to you.

// If SMOOTHNESS ...
if (...) {
 pending_ready_for_draw_tiles_.insert(tile);
 return;
}

draw_info.set_resource_ready_for_draw();
client_->NotifyTileStateChanged(tile);

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1329: // The tile has been evicted or deleted. Remove
it from our list.
nit: or is pending deletion. (you remove it when it's proper deleted)

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1334: if (!in_smoothness_mode ||
You can just do the gobal_state_ == ... check here and remove the variable.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1343: if (pending_tile_requirements_dirty_)
I know we talked about this a bit, but is there a race here?
UpdateTilePriorities might have run and the scheduler is pending a PrepareTiles,
but one hasn't happened yet. Is that possible? What are the implications? It
seems that some times that _are_ now required might be missed.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1354: pending_required_for_activation_callback_id_ = 0;
Maybe

pending_required_for_activation_callback_id_ = 0;
if (!required_for_activation_ids.empty()) {
  ...
}

WDYT?

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.h:347: std::unordered_set<Tile*>
pending_ready_for_draw_tiles_;
Is this pending ready _to_ draw tiles?

Maybe something like pending_gpu_work_tiles_... Not sure. It's a bit confusing
what pending ready for draw tiles is for me.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
File cc/tiles/tile_manager_unittest.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:1950: no_tiles_recording_source_->Rerecord();
nit: solid_color_recording_source_ and just.. recording_source_ i guess?

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:1974: void SetupActiveTree() {
I'm a bit confused by the names here, since both SetupActiveTree and
SetupPendingTree sets up both trees?

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:2020: .WillRepeatedly(Return(false));
I'm always amazed by this gmock magic.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:2147: PictureLayerTilingSet* tiling_set =
&layer->tilings()[i];
This doesn't seem right. layer->tilings() returns a PictureLayerTilingSet*, it's
not an index-able thing. I think this might only work because num_tilings() is
always 1? So this is &layer->tilings()[0], which is &*layer->tilings(). I think
you just want layer->tilings();

xD

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:2198:
host_impl()->tile_manager()->PrepareTiles(host_impl()->global_tile_state());
My concern here (as I commented in the code) is whether it's possible that this
prepare tiles doesn't run fast enough and we get a task completion first
instead. I'm not sure whether it is possible or not, but can you please verify?

[commit-bot: I haz the power, 2017-01-05 22:21:06]

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

[commit-bot: I haz the power, 2017-01-05 22:21:07]

Dry run: This issue passed the CQ dry run.

[ericrk, 2017-01-09 21:28:56]

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

[commit-bot: I haz the power, 2017-01-09 21:29:53]

Dry run: 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, 2017-01-09 22:40:25]

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

[commit-bot: I haz the power, 2017-01-09 22:40:26]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ericrk, 2017-01-09 23:05:16]

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

[ericrk, 2017-01-09 23:05:22]

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/gpu_raster_b...
File cc/raster/gpu_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/gpu_raster_b...
cc/raster/gpu_raster_buffer_provider.cc:217: // If the callback we are about to
request has the same ID as the one which
On 2017/01/05 22:00:46, vmpstr wrote:
> It's a bit awkward that this comment is talking about the "else" case. I think
> it's fine, but maybe rephrase it as "If callback_id is different, then ...;
> Otherwise, the request is redundant"

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/one_copy_ras...
File cc/raster/one_copy_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/one_copy_ras...
cc/raster/one_copy_raster_buffer_provider.cc:193: // If the callback we are
about to request has the same ID as the one which
On 2017/01/05 22:00:46, vmpstr wrote:
> Same here.

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/raster_buffe...
File cc/raster/raster_buffer_provider.h (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/raster/raster_buffe...
cc/raster/raster_buffer_provider.h:74: // avoid creating a new callback
unnecessarily.
On 2017/01/05 22:00:46, vmpstr wrote:
> Can you specify to pass 0 if no pending callback exists? Also, is this
something
> that the RasterBufferProvider (or its overrides) can keep track of? That is,
it
> should know the last callback id it returned, no?
> 
> Or is there something more subtle here?

The RasterBufferProvider knows the last callback it returned, but the
TileManager simultaneously requests callbacks for both ready to draw and ready
to activate callbacks - RasterBufferProvider doesn't know the difference between
these so it can't really de-dupe them.

https://codereview.chromium.org/2555743004/diff/360001/cc/resources/resource_...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/resources/resource_...
cc/resources/resource_provider.cc:975: gpu::SyncToken
ResourceProvider::GetSyncTokenForResource(
On 2017/01/05 22:00:46, vmpstr wrote:
> This seems to be a special case of the function below. Do we need both?

Not really... removed.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_draw_inf...
File cc/tiles/tile_draw_info.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_draw_inf...
cc/tiles/tile_draw_info.cc:32: if (!resource_)
On 2017/01/05 22:00:46, vmpstr wrote:
> Don't need this if.

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:427: // the
On 2017/01/05 22:00:46, vmpstr wrote:
> Reformat plz

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:440: if (tile->HasRasterTask()) {
On 2017/01/05 22:00:46, vmpstr wrote:
> Can you file a bug to clean this up please? We changed the raster task
> ownership, so afaik we don't actually need to retain the tiles since the task
> will still be alive even if it's pending completion/rasterization.
> 
> Or am I missing stuff again?

In RasterTaskCompleted, we use/dereference tile, so we'd need to do a bit more
work to handle this safely. I had started writing a patch, but got sidetracked.
I did open a bug here: crbug.com/672606, I'll add a TODO.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1091: // In SMOOTHNESS_TAKES_PRIORITY mode, we wait for
GPU work to complete
On 2017/01/05 22:00:46, vmpstr wrote:
> Move this comment outside of the if plz
> 
> Also consider doing this pattern, but it's really up to you.
> 
> // If SMOOTHNESS ...
> if (...) {
>  pending_ready_for_draw_tiles_.insert(tile);
>  return;
> }
> 
> draw_info.set_resource_ready_for_draw();
> client_->NotifyTileStateChanged(tile);

good suggestion.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1329: // The tile has been evicted or deleted. Remove
it from our list.
On 2017/01/05 22:00:46, vmpstr wrote:
> nit: or is pending deletion. (you remove it when it's proper deleted)

now that tile deletion has been cleaned up, this is gone.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1334: if (!in_smoothness_mode ||
On 2017/01/05 22:00:46, vmpstr wrote:
> You can just do the gobal_state_ == ... check here and remove the variable.

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1343: if (pending_tile_requirements_dirty_)
On 2017/01/05 22:00:46, vmpstr wrote:
> I know we talked about this a bit, but is there a race here?
> UpdateTilePriorities might have run and the scheduler is pending a
PrepareTiles,
> but one hasn't happened yet. Is that possible? What are the implications? It
> seems that some times that _are_ now required might be missed.

Ok, I think there was a race here (mainly because of the scheduler sometimes
skipping PrepareTiles) - added a TileManager::DidModifyTilePriorities to handle
this.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1354: pending_required_for_activation_callback_id_ = 0;
On 2017/01/05 22:00:46, vmpstr wrote:
> Maybe
> 
> pending_required_for_activation_callback_id_ = 0;
> if (!required_for_activation_ids.empty()) {
>   ...
> }
> 
> WDYT?

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager....
cc/tiles/tile_manager.h:347: std::unordered_set<Tile*>
pending_ready_for_draw_tiles_;
On 2017/01/05 22:00:46, vmpstr wrote:
> Is this pending ready _to_ draw tiles?
> 
> Maybe something like pending_gpu_work_tiles_... Not sure. It's a bit confusing
> what pending ready for draw tiles is for me.

Yeah, I like pending_gpu_work_tiles_

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
File cc/tiles/tile_manager_unittest.cc (right):

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:1950: no_tiles_recording_source_->Rerecord();
On 2017/01/05 22:00:46, vmpstr wrote:
> nit: solid_color_recording_source_ and just.. recording_source_ i guess?

Done.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:1974: void SetupActiveTree() {
On 2017/01/05 22:00:47, vmpstr wrote:
> I'm a bit confused by the names here, since both SetupActiveTree and
> SetupPendingTree sets up both trees?

Renamed to make it clearer. We are always setting up both trees - the named tree
gets tiles, the other gets solid color.

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:2147: PictureLayerTilingSet* tiling_set =
&layer->tilings()[i];
On 2017/01/05 22:00:47, vmpstr wrote:
> This doesn't seem right. layer->tilings() returns a PictureLayerTilingSet*,
it's
> not an index-able thing. I think this might only work because num_tilings() is
> always 1? So this is &layer->tilings()[0], which is &*layer->tilings(). I
think
> you just want layer->tilings();
> 
> xD

Yup :D

https://codereview.chromium.org/2555743004/diff/360001/cc/tiles/tile_manager_...
cc/tiles/tile_manager_unittest.cc:2198:
host_impl()->tile_manager()->PrepareTiles(host_impl()->global_tile_state());
On 2017/01/05 22:00:47, vmpstr wrote:
> My concern here (as I commented in the code) is whether it's possible that
this
> prepare tiles doesn't run fast enough and we get a task completion first
> instead. I'm not sure whether it is possible or not, but can you please
verify?

Per other comment, updated so that we update the requirements change via a
different method.

[commit-bot: I haz the power, 2017-01-09 23:06:02]

Dry run: 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, 2017-01-10 00:24:24]

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

[commit-bot: I haz the power, 2017-01-10 00:24:26]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ericrk, 2017-01-11 19:21:02]

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

[ericrk, 2017-01-11 19:21:09]

The CQ bit was unchecked by ericrk@chromium.org

[ericrk, 2017-01-11 19:22:33]

rebase

[ericrk, 2017-01-11 19:22:42]

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

[commit-bot: I haz the power, 2017-01-11 19:23:12]

Dry run: 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, 2017-01-11 19:27:37]

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

[commit-bot: I haz the power, 2017-01-11 19:27:38]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[ericrk, 2017-01-13 22:26:43]

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

[commit-bot: I haz the power, 2017-01-13 22:27:12]

Dry run: 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, 2017-01-13 22:53:13]

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

[commit-bot: I haz the power, 2017-01-13 22:53:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ericrk, 2017-01-14 00:34:41]

Patchset #18 (id:420001) has been deleted

[ericrk, 2017-01-17 17:50:15]

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

[commit-bot: I haz the power, 2017-01-17 17:50:28]

Dry run: 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, 2017-01-17 18:06:30]

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

[commit-bot: I haz the power, 2017-01-17 18:06:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ericrk, 2017-01-17 18:28:30]

Patchset #18 (id:440001) has been deleted

[ericrk, 2017-01-17 18:31:23]

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

[commit-bot: I haz the power, 2017-01-17 18:32:10]

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

[ericrk, 2017-01-17 18:57:51]

Ping for review. Thanks!

[commit-bot: I haz the power, 2017-01-17 19:30:27]

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

[commit-bot: I haz the power, 2017-01-17 19:30:28]

Dry run: This issue passed the CQ dry run.

[ericrk, 2017-01-30 15:42:16]

On 2017/01/17 18:57:51, ericrk wrote:
> Ping for review. Thanks!

ping, PTAL. Thanks.

[vmpstr, 2017-01-30 19:51:19]

Other than my comment, the rest of the patch looks good.

https://codereview.chromium.org/2555743004/diff/460001/cc/raster/gpu_raster_b...
File cc/raster/gpu_raster_buffer_provider.cc (right):

https://codereview.chromium.org/2555743004/diff/460001/cc/raster/gpu_raster_b...
cc/raster/gpu_raster_buffer_provider.cc:198:
resource_provider_->GetSyncTokenForResources(resources);
nit: this can just be 

resource_provider_->GetSyncTokenForResources({resource_id});

(I think that's allowed?)

https://codereview.chromium.org/2555743004/diff/460001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/460001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1325: if (tile->required_for_activation())
While investigating an unrelated to this bug, I remembered that this might delay
activation in some cases. Specifically, we can have a layer where
HasValidTilePriorities is false. This means that the tile will be skipped during
tile iteration (both in assigngpumem and in checking whether we can activate).
Specifically, this means that it's required state is not checked.

We could check whether the layer that owns this tiling has valid tile priorities
and if not, assume the tile is done. However, that would come at a cost of a
virtual function call for every tile here. I'm gonna leave the decision of what
to do up to you.

We could also make the UpdateRequiredStatesOnTile make it false when the layer
doesn't have valid tile priorities, but it's the same cost basically.

Whatever you decide, can you add a test for this case as well, please?

[ericrk, 2017-01-31 19:23:54]

https://codereview.chromium.org/2555743004/diff/460001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2555743004/diff/460001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1325: if (tile->required_for_activation())
On 2017/01/30 19:51:19, vmpstr wrote:
> While investigating an unrelated to this bug, I remembered that this might
delay
> activation in some cases. Specifically, we can have a layer where
> HasValidTilePriorities is false. This means that the tile will be skipped
during
> tile iteration (both in assigngpumem and in checking whether we can activate).
> Specifically, this means that it's required state is not checked.
> 
> We could check whether the layer that owns this tiling has valid tile
priorities
> and if not, assume the tile is done. However, that would come at a cost of a
> virtual function call for every tile here. I'm gonna leave the decision of
what
> to do up to you.
> 
> We could also make the UpdateRequiredStatesOnTile make it false when the layer
> doesn't have valid tile priorities, but it's the same cost basically.
> 
> Whatever you decide, can you add a test for this case as well, please?

Added a TODO + crbug... will take care of this in a follow-up CL.

[ericrk, 2017-01-31 19:24:02]

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

[commit-bot: I haz the power, 2017-01-31 19:24:34]

Dry run: 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, 2017-01-31 19:43:33]

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

[commit-bot: I haz the power, 2017-01-31 19:43:34]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ericrk, 2017-01-31 19:56:18]

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

[commit-bot: I haz the power, 2017-01-31 19:57:02]

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

[vmpstr, 2017-01-31 21:33:48]

lgtm

[commit-bot: I haz the power, 2017-01-31 22:13:20]

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

[commit-bot: I haz the power, 2017-01-31 22:13:22]

Dry run: This issue passed the CQ dry run.

[ericrk, 2017-01-31 22:26:57]

The CQ bit was checked by ericrk@chromium.org

[commit-bot: I haz the power, 2017-01-31 22:27:49]

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, 2017-01-31 22:34:36]

CQ is committing da patch.

Bot data: {"patchset_id": 500001, "attempt_start_ts": 1485901617495300,
"parent_rev": "eb6c3ce7ce15059e6411b8b018df85b2dbf0e18d", "commit_rev":
"7f6a27f1dbe4d8cafffb919445bc93bb8867a8fe"}

[commit-bot: I haz the power, 2017-01-31 22:35:16]

Description was changed from

==========
Delay activation/draw on GPU tile work completion

Design doc: https://goo.gl/Ku9VBh

This change delays the activation of the pending tree as well as
drawing of active tree tiles on the completion of the required GL
work in the GPU process. This differs from our current
approach, where we only delay activation/draw on the issuing of GL
commands in the renderer, not on the commands being executed in the
GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

BUG=673434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Delay activation/draw on GPU tile work completion

Design doc: https://goo.gl/Ku9VBh

This change delays the activation of the pending tree as well as
drawing of active tree tiles on the completion of the required GL
work in the GPU process. This differs from our current
approach, where we only delay activation/draw on the issuing of GL
commands in the renderer, not on the commands being executed in the
GL process.

Note that this behavior is only added in cases where we've enabled the
async worker context via flags.

BUG=673434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2555743004
Cr-Commit-Position: refs/heads/master@{#447346}
Committed:
https://chromium.googlesource.com/chromium/src/+/7f6a27f1dbe4d8cafffb919445bc...
==========

[commit-bot: I haz the power, 2017-01-31 22:35:19]

Committed patchset #20 (id:500001) as
https://chromium.googlesource.com/chromium/src/+/7f6a27f1dbe4d8cafffb919445bc...