cc: Don't perform image analysis if the DisplayItemList has no images.

cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2830243002
Cr-Commit-Position: refs/heads/master@{#469509}
Committed: https://chromium.googlesource.com/chromium/src/+/74c96e8453a7a5da13fb1ebef083a06b1566f2d2

[Khushal, 2017-04-21 03:51:37]

Description was changed from

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.
==========

to

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Khushal, 2017-04-21 03:51:47]

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

[commit-bot: I haz the power, 2017-04-21 03:52:03]

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

[Khushal, 2017-04-21 04:37:17]

Description was changed from

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.

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

[Khushal, 2017-04-21 04:40:03]

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

[commit-bot: I haz the power, 2017-04-21 05:03:36]

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

[commit-bot: I haz the power, 2017-04-21 05:03:38]

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_...)

[vmpstr, 2017-04-25 17:43:40]

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

[vmpstr, 2017-04-25 17:43:41]

lgtm % enne. Please wait until I land removing the should generate discardable
images flag though (I forgot to land that).

https://codereview.chromium.org/2830243002/diff/20001/cc/layers/recording_sou...
File cc/layers/recording_source.cc (left):

https://codereview.chromium.org/2830243002/diff/20001/cc/layers/recording_sou...
cc/layers/recording_source.cc:59: if (generate_discardable_images_metadata_)
This is the only place this flag is used afaik. If we're not relying on it then
we should remove it.

https://codereview.chromium.org/2830243002/diff/20001/cc/paint/paint_op_buffer.h
File cc/paint/paint_op_buffer.h (right):

https://codereview.chromium.org/2830243002/diff/20001/cc/paint/paint_op_buffe...
cc/paint/paint_op_buffer.h:743: return image ? image->isLazyGenerated() : false;
return image && image->isLazyGenerated();

[Khushal, 2017-04-25 18:18:10]

https://codereview.chromium.org/2830243002/diff/20001/cc/layers/recording_sou...
File cc/layers/recording_source.cc (left):

https://codereview.chromium.org/2830243002/diff/20001/cc/layers/recording_sou...
cc/layers/recording_source.cc:59: if (generate_discardable_images_metadata_)
On 2017/04/25 17:43:40, vmpstr wrote:
> This is the only place this flag is used afaik. If we're not relying on it
then
> we should remove it.

That was my plan. After rebasing on your patch.

[enne (OOO), 2017-04-25 20:07:58]

lgtm

Maybe we need a PaintOpWithFlags instead of template magic everywhere.  <_<

[Khushal, 2017-04-25 21:39:02]

On 2017/04/25 20:07:58, enne wrote:
> lgtm
> 
> Maybe we need a PaintOpWithFlags instead of template magic everywhere.  <_<

Yeah, I guess there will be more such cases. At least for images though, I'm
hoping this wouldn't be necessary when images are no longer buried inside
PaintFlags.

[Khushal, 2017-05-03 18:39:52]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-03 18:39:52]

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

[commit-bot: I haz the power, 2017-05-03 18:40:22]

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-05-03 20:01:39]

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

[commit-bot: I haz the power, 2017-05-03 20:01:40]

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-05-03 22:49:49]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2017-05-03 22:50:33]

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-05-03 23:00:45]

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

[commit-bot: I haz the power, 2017-05-03 23:00:46]

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-05-04 01:01:32]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-04 01:01:33]

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

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

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-05-04 01:43:24]

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

[commit-bot: I haz the power, 2017-05-04 01:43:26]

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

[Khushal, 2017-05-04 03:24:57]

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

[commit-bot: I haz the power, 2017-05-04 03:25: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-05-04 04:03:30]

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

[commit-bot: I haz the power, 2017-05-04 04:03:31]

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

[Khushal, 2017-05-04 20:55:54]

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

[commit-bot: I haz the power, 2017-05-04 20:56:22]

Dry run: CQ is trying da patch.

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

[Khushal, 2017-05-04 20:58:17]

https://codereview.chromium.org/2830243002/diff/160001/cc/paint/transform_dis...
File cc/paint/transform_display_item.cc (right):

https://codereview.chromium.org/2830243002/diff/160001/cc/paint/transform_dis...
cc/paint/transform_display_item.cc:14: this->transform.matrix().getType();
So looks like the IsIdentity check when rastering the transform display item is
inadvertently causing a race when coupled with this change. Yay mutable!
Does this look like a reasonable way to avoid that?

[commit-bot: I haz the power, 2017-05-04 21:00:19]

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

[commit-bot: I haz the power, 2017-05-04 21:00:20]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[enne (OOO), 2017-05-04 21:02:27]

https://codereview.chromium.org/2830243002/diff/160001/cc/paint/transform_dis...
File cc/paint/transform_display_item.cc (right):

https://codereview.chromium.org/2830243002/diff/160001/cc/paint/transform_dis...
cc/paint/transform_display_item.cc:14: this->transform.matrix().getType();
On 2017/05/04 at 20:58:17, Khushal wrote:
> So looks like the IsIdentity check when rastering the transform display item
is inadvertently causing a race when coupled with this change. Yay mutable!
> Does this look like a reasonable way to avoid that?

Yeah, that's what I had to do in PaintOpBuffer as well.  Good thing we've got
oodles of layers of display lists where we have to repeat the same patterns at
every level.

[Khushal, 2017-05-04 21:12:45]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-05-04 21:12:46]

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

[commit-bot: I haz the power, 2017-05-04 21:13:20]

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-05-04 23:16:59]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1493932365711290,
"parent_rev": "518c27b2f8b6f890ca41e560cede83555c90f1c9", "commit_rev":
"74c96e8453a7a5da13fb1ebef083a06b1566f2d2"}

[commit-bot: I haz the power, 2017-05-04 23:17:44]

Description was changed from

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.

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

to

==========
cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2830243002
Cr-Commit-Position: refs/heads/master@{#469509}
Committed:
https://chromium.googlesource.com/chromium/src/+/74c96e8453a7a5da13fb1ebef083...
==========

[commit-bot: I haz the power, 2017-05-04 23:17:45]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/74c96e8453a7a5da13fb1ebef083...