cc: Move pre-decodes to background worker.

cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

3) If the image is in both pre-paint and pre-decode region, then we
could be decoding them on both, the raster worker and image worker.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

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

Review-Url: https://codereview.chromium.org/2924233002
Cr-Commit-Position: refs/heads/master@{#482062}
Committed: https://chromium.googlesource.com/chromium/src/+/de93e30d4aa72f1c1a5c88e2aafba0021697b351

[Khushal, 2017-06-08 05:27:04]

Description was changed from

==========
cc: Move pre-decodes to background worker.

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

to

==========
cc: Move pre-decodes to background worker.

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

[Khushal, 2017-06-08 11:09:47]

Description was changed from

==========
cc: Move pre-decodes to background worker.

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

to

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

[Khushal, 2017-06-08 17:26:17]

Description was changed from

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

to

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

[Khushal, 2017-06-08 17:38:20]

khushalsagar@chromium.org changed reviewers:
+ enne@chromium.org, vmpstr@chromium.org

[Khushal, 2017-06-08 17:38:21]

PTAL.

[enne (OOO), 2017-06-08 20:08:11]

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

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:909: // Ensure that we don't schedule any pre-decode
work till all tile tasks have
I'm mostly going to defer to vmpstr on this review, but I just had some higher
level questions:

(1) Is this the right priority for checker image decodes (i.e. last)? Sure,
visible tiles probably should have highest priority, but should we fix checker
images before we do pre-raster work of offscreen tiles?  Maybe this is correct,
but I just wanted clarification that this is intended.

(2) Similarly, checker image scheduling seems kind of bolted onto the side and
not prioritized against other work.  I suspect maybe it's not desirable to mix
tile prioritization with decode prioritization, but mixing the two systems seems
a little awkward from an outside perspective.

[Khushal, 2017-06-08 20:34:15]

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

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:909: // Ensure that we don't schedule any pre-decode
work till all tile tasks have
On 2017/06/08 20:08:10, enne wrote:
> I'm mostly going to defer to vmpstr on this review, but I just had some higher
> level questions:
> 
> (1) Is this the right priority for checker image decodes (i.e. last)? Sure,
> visible tiles probably should have highest priority, but should we fix checker
> images before we do pre-raster work of offscreen tiles?  Maybe this is
correct,
> but I just wanted clarification that this is intended.

This is only going to defer the decoding for pre-decode tiles. The decodes for
checkered images from any tiles that are being rasterized will still continue in
parallel with the raster work.

> 
> (2) Similarly, checker image scheduling seems kind of bolted onto the side and
> not prioritized against other work.  I suspect maybe it's not desirable to mix
> tile prioritization with decode prioritization, but mixing the two systems
seems
> a little awkward from an outside perspective.

The fact that checker image scheduling is completely independent of work done by
the raster worker pool creates interesting scenarios on devices that have a
limited number of cores. Currently the raster worker pool first runs visible
tile tasks on the foreground workers, followed by pre-paint tiles on the
background worker, and lastly decodes for pre-decoded tiles. With
checker-imaging on the side, we could have cases where the visible tile work is
scheduled in parallel with the decode for an offscreen tile on the image worker
and it starts pre-empting the raster work. Should we try to avoid that? Listen
to when all NOW bin tiles are done before starting checkered decodes for
pre-paint tiles for instance? At the least I wanted to keep decodes for
pre-decode tiles from interrupting raster work we know is higher priority.

[Khushal, 2017-06-12 20:59:26]

Description was changed from

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

to

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

3) If the image is in both pre-paint and pre-decode region, then we
could be decoding them on both, the raster worker and image worker.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

[vmpstr, 2017-06-12 21:45:30]

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.cc:259: if (!image_decode_queue_.empty() &&
Can you break this up a bit. Like, check if empty and return early. Then the
rest of the function can assume not empty.

What's guarding the ordering here? Are we guaranteed to get Raster tasks before
PreDecode ones?

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.h:36: enum class DecodeType { kRaster, kPreDecode
};
Comment please

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

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:1321:
checker_image_tracker_.SetCanSchedulePredecodeImages(true);
Can you document the order of execution somewhere? Maybe on TileManager class to
explain the total order of things

[Khushal, 2017-06-14 00:29:20]

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.cc:259: if (!image_decode_queue_.empty() &&
On 2017/06/12 21:45:30, vmpstr wrote:
> Can you break this up a bit. Like, check if empty and return early. Then the
> rest of the function can assume not empty.

Done.

> 
> What's guarding the ordering here? Are we guaranteed to get Raster tasks
before
> PreDecode ones?

I added a DCHECK in ScheduleImageDecodeQueue to ensure that the images are
prioritized in the correct order.

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/checker_image_...
cc/tiles/checker_image_tracker.h:36: enum class DecodeType { kRaster, kPreDecode
};
On 2017/06/12 21:45:30, vmpstr wrote:
> Comment please

Done.

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

https://codereview.chromium.org/2924233002/diff/80001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:1321:
checker_image_tracker_.SetCanSchedulePredecodeImages(true);
On 2017/06/12 21:45:30, vmpstr wrote:
> Can you document the order of execution somewhere? Maybe on TileManager class
to
> explain the total order of things

I added a comment on TileManager. Does that look sufficient?

[Khushal, 2017-06-20 17:45:35]

pingy.

[vmpstr, 2017-06-20 18:41:02]

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:38: kNone,
This is an awkward name. Just from looking at this, I wouldn't know when to say
DecodeType::kNone. How is this used? Is there a more descriptive name we can use
here?

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:65: // The max decode priority type that is
allowed to run.
Can you use the word "max" or "limit" in the function name to make it clear that
everything specified and below can run?

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:912: CheckerImageTracker::DecodeType::kNone);
Maybe just renaming this to AllowedDecodeType or something? then kNone makes
more sense.

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1123: if
(prioritized_tile.should_decode_checkered_images_for_tile())
nit: braces

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1328: CheckerImageTracker::DecodeType decode_type =
Do we need this to be strictly in CheckAndIssueSignals? Can we do this from the
tile task completion callbacks instead? Like NotifyRasterFinished
NotifyReadyTo... functions? This feels like a common place, but I think it makes
the function do more things that it was designed to do (ie it's just just
checking and issuing signals anymore).

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

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.h:115: // 1) RequiredForActivation/Draw Tiles: These are
the highest priority tiles
Thanks for the comment! also you don't need to capitalize Tiles :P

https://codereview.chromium.org/2924233002/diff/100001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_unittest_checkerimaging.cc (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_unittest_checkerimaging.cc:125:
EXPECT_TRUE(tile->HasRasterTask()) << i << "," << j;
you can use

for (int i = 0; i < 4; ++i) {
  SCOPED_TRACE(i);
  for (int j = 0; j < 2; ++j) {
    SCOPED_TRACE(j);
    ...
  }
}

This will log the values on any failure below it in the scope

[Khushal, 2017-06-21 03:35:58]

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:65: // The max decode priority type that is
allowed to run.
On 2017/06/20 18:41:01, vmpstr wrote:
> Can you use the word "max" or "limit" in the function name to make it clear
that
> everything specified and below can run?

Done.

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:912: CheckerImageTracker::DecodeType::kNone);
On 2017/06/20 18:41:01, vmpstr wrote:
> Maybe just renaming this to AllowedDecodeType or something? then kNone makes
> more sense.

This is also used when specifying the DecodeType in the ImageDecodeRequest.
AllowedDecodeType there looks awkward.

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1123: if
(prioritized_tile.should_decode_checkered_images_for_tile())
On 2017/06/20 18:41:01, vmpstr wrote:
> nit: braces

Done.

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1328: CheckerImageTracker::DecodeType decode_type =
On 2017/06/20 18:41:01, vmpstr wrote:
> Do we need this to be strictly in CheckAndIssueSignals? Can we do this from
the
> tile task completion callbacks instead? Like NotifyRasterFinished
> NotifyReadyTo... functions? This feels like a common place, but I think it
makes
> the function do more things that it was designed to do (ie it's just just
> checking and issuing signals anymore). 

There can be a race between the DidFinish* task running for the previous graph
and the compositor thread scheduling another PrepareTiles. I think we try to
guard against this by actually iterating through the queue to check if the
required tiles are ready to draw. That's why I wanted to key off the notify_*
signals to be consistent with the logic that is used to issue these signals to
the rest of the system. And CheckAndIssueSignals is the place where that
happens.

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

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/tile_manager....
cc/tiles/tile_manager.h:115: // 1) RequiredForActivation/Draw Tiles: These are
the highest priority tiles
On 2017/06/20 18:41:01, vmpstr wrote:
> Thanks for the comment! also you don't need to capitalize Tiles :P

Its a heading to the bullet point. Looked prettier that way. :P

https://codereview.chromium.org/2924233002/diff/100001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_unittest_checkerimaging.cc (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_unittest_checkerimaging.cc:125:
EXPECT_TRUE(tile->HasRasterTask()) << i << "," << j;
On 2017/06/20 18:41:01, vmpstr wrote:
> you can use
> 
> for (int i = 0; i < 4; ++i) {
>   SCOPED_TRACE(i);
>   for (int j = 0; j < 2; ++j) {
>     SCOPED_TRACE(j);
>     ...
>   }
> }
> 
> This will log the values on any failure below it in the scope

Thanks for the suggestion. Done.

[Khushal, 2017-06-21 06:12:01]

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

[commit-bot: I haz the power, 2017-06-21 06:12:11]

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-06-21 07:00:45]

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

[commit-bot: I haz the power, 2017-06-21 07:00:46]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Khushal, 2017-06-21 23:45:28]

I moved the call we had discussed to PrepareTiles instead of ScheduleTasks so it
does not unnecessarily disable it if its coming from
CheckIfMoreTilesNeedToBePrepared.

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/100001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:38: kNone,
On 2017/06/20 18:41:01, vmpstr wrote:
> This is an awkward name. Just from looking at this, I wouldn't know when to
say
> DecodeType::kNone. How is this used? Is there a more descriptive name we can
use
> here?

I just made it all internal so we wouldn't need to expose something in this enum
to let the caller disable all decodes.

[Khushal, 2017-06-22 07:46:10]

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

[commit-bot: I haz the power, 2017-06-22 07:46:24]

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-06-22 08:57:43]

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

[commit-bot: I haz the power, 2017-06-22 08:57:44]

Dry run: This issue passed the CQ dry run.

[vmpstr, 2017-06-22 16:57:58]

lgtm with nits

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.cc:64: DCHECK_LE(decode_type, DecodeType::kLast);
I'm a bit hesitant for adding kLast just for this dcheck, but meh.

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:36: enum DecodeType : int {
Can you comment why you're specifying int explicitly?

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:100: const int kNoDecodeAllowedPriority = -1;
static

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:497: // the raster work for visible tiles is done.
Can you elaborate a bit more by stating when exactly we'll allow decodes (ie
when required for {draw,activate} tiles are done if there are any... in any
case, this is done when we issue a draw/activate signal)

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1327: // are done. And pre-decode tiles once all tile
tasks are done.
Can you elaborate again to just say that the order is important since all of
those signals could have become true, and if that's the case we want to allow
the most decodes.

[Khushal, 2017-06-22 20:11:52]

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.cc (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.cc:64: DCHECK_LE(decode_type, DecodeType::kLast);
On 2017/06/22 16:57:57, vmpstr wrote:
> I'm a bit hesitant for adding kLast just for this dcheck, but meh.

Acknowledged.

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
File cc/tiles/checker_image_tracker.h (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:36: enum DecodeType : int {
On 2017/06/22 16:57:57, vmpstr wrote:
> Can you comment why you're specifying int explicitly?

Done.

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/checker_image...
cc/tiles/checker_image_tracker.h:100: const int kNoDecodeAllowedPriority = -1;
On 2017/06/22 16:57:57, vmpstr wrote:
> static

Done.

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:497: // the raster work for visible tiles is done.
On 2017/06/22 16:57:58, vmpstr wrote:
> Can you elaborate a bit more by stating when exactly we'll allow decodes (ie
> when required for {draw,activate} tiles are done if there are any... in any
> case, this is done when we issue a draw/activate signal)

Done.

https://codereview.chromium.org/2924233002/diff/180001/cc/tiles/tile_manager....
cc/tiles/tile_manager.cc:1327: // are done. And pre-decode tiles once all tile
tasks are done.
On 2017/06/22 16:57:57, vmpstr wrote:
> Can you elaborate again to just say that the order is important since all of
> those signals could have become true, and if that's the case we want to allow
> the most decodes.

Done.

[Khushal, 2017-06-22 20:11:59]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-06-22 20:11:59]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2924233002/#ps200001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-06-22 20:12:27]

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-06-22 20:22:53]

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

[commit-bot: I haz the power, 2017-06-22 20:22:55]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Khushal, 2017-06-22 20:58:29]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-06-22 20:58:30]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2924233002/#ps220001
(title: "..")

[commit-bot: I haz the power, 2017-06-22 20:58:47]

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-06-22 22:17:51]

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

[commit-bot: I haz the power, 2017-06-22 22:17:52]

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

[Khushal, 2017-06-23 05:17:17]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-06-23 05:17:18]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2924233002/#ps240001
(title: "test")

[commit-bot: I haz the power, 2017-06-23 05:17:29]

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-06-23 05:19:39]

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

[commit-bot: I haz the power, 2017-06-23 05:19:41]

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-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Khushal, 2017-06-23 05:48:06]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-06-23 05:48:07]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2924233002/#ps260001
(title: "rebase")

[commit-bot: I haz the power, 2017-06-23 05:48:16]

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-06-23 06:04:07]

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

[commit-bot: I haz the power, 2017-06-23 06:04:08]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Khushal, 2017-06-23 21:54:14]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-06-23 21:54:14]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2924233002/#ps280001
(title: "flake is flaky. T_T")

[commit-bot: I haz the power, 2017-06-23 21:54:35]

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-06-23 23:03:48]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1498254854418510,
"parent_rev": "20c4a42f9303cb2e5f219dfc7410399dc098bc34", "commit_rev":
"de93e30d4aa72f1c1a5c88e2aafba0021697b351"}

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

Description was changed from

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

3) If the image is in both pre-paint and pre-decode region, then we
could be decoding them on both, the raster worker and image worker.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

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

to

==========
cc: Move pre-decoding for checkerable images to image worker.

Currently images for pre-decoded but not pre-painted tiles are decoded
on the background tile worker, even for checkerable images. This has
the following downsides:

1) It creates cases where decoding of images for lower priority
pre-decode tiles starts pre-empting a higher priority decode running on
the image worker.

2) Since the checker image tracking is not aware of pre-decoded images,
we still checker them when these tiles are first rasterized, even while
the image could be currently locked in the decode cache.

3) If the image is in both pre-paint and pre-decode region, then we
could be decoding them on both, the raster worker and image worker.

This change moves decoding of checkerable images for these tiles to the
image worker, similar to the rasterized tiles. While invalidating is
not neccesary for these images, we use the same mechanism in order to
generate tasks for uploading these images in the GPU cache.

BUG=728796

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2924233002
Cr-Commit-Position: refs/heads/master@{#482062}
Committed:
https://chromium.googlesource.com/chromium/src/+/de93e30d4aa72f1c1a5c88e2aafb...
==========

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

Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/de93e30d4aa72f1c1a5c88e2aafb...