Plumb PaintImage to the PictureImageLayer.

Plumb PaintImage to the PictureImageLayer.

This patch ensures that we pass PaintImage with all of the proper flags
to PictureImageLayer.

R=khushalsagar@chromium.org, pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2835373003
Cr-Commit-Position: refs/heads/master@{#468196}
Committed: https://chromium.googlesource.com/chromium/src/+/55c7657caad796ca2097e804cedb40b4d8d796d3

[vmpstr, 2017-04-25 22:04:23]

Description was changed from

==========
Plumb PaintImage to the PictureImageLayer.

This patch ensures that we pass PaintImage with all of the proper flags
to PictureImageLayer.

R=khushalsagar@chromium.org, pdr@chromium.org
==========

to

==========
Plumb PaintImage to the PictureImageLayer.

This patch ensures that we pass PaintImage with all of the proper flags
to PictureImageLayer.

R=khushalsagar@chromium.org, pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-04-25 22:05:46]

Please take a look.

https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebImageLayer.h (right):

https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebImageLayer.h:32: #if INSIDE_BLINK
Not really sure if there's a better magic here...

[Khushal, 2017-04-26 03:06:43]

lgtm.

https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right):

https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058: sk_image =
image->ImageForCurrentFrame();
May be a PaintImageForCurrentFrame on blink::Image so we don't end up doing the
animation and completion state thing in a lot of places?

[vmpstr, 2017-04-26 16:57:01]

On 2017/04/26 03:06:43, Khushal wrote:
> lgtm.
> 
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right):
> 
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058: sk_image =
> image->ImageForCurrentFrame();
> May be a PaintImageForCurrentFrame on blink::Image so we don't end up doing
the
> animation and completion state thing in a lot of places?

Do you mind if we do this as a follow-up? pdr@ suggested a similar thing, which
I pushed back on, but I take that back since it does seem to be in more places
than ideal.

[vmpstr, 2017-04-26 16:57:53]

On 2017/04/26 16:57:01, vmpstr wrote:
> On 2017/04/26 03:06:43, Khushal wrote:
> > lgtm.
> > 
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058: sk_image
=
> > image->ImageForCurrentFrame();
> > May be a PaintImageForCurrentFrame on blink::Image so we don't end up doing
> the
> > animation and completion state thing in a lot of places?
> 
> Do you mind if we do this as a follow-up? pdr@ suggested a similar thing,
which
> I pushed back on, but I take that back since it does seem to be in more places
> than ideal.

I also considered having a PaintImage ctor that just takes an SkImage and sets
animation = STATIC and completion = DONE. WDYT?

[Khushal, 2017-04-26 17:10:02]

On 2017/04/26 16:57:53, vmpstr wrote:
> On 2017/04/26 16:57:01, vmpstr wrote:
> > On 2017/04/26 03:06:43, Khushal wrote:
> > > lgtm.
> > > 
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058:
sk_image
> =
> > > image->ImageForCurrentFrame();
> > > May be a PaintImageForCurrentFrame on blink::Image so we don't end up
doing
> > the
> > > animation and completion state thing in a lot of places?
> > 
> > Do you mind if we do this as a follow-up? pdr@ suggested a similar thing,
> which
> > I pushed back on, but I take that back since it does seem to be in more
places
> > than ideal.

If its difficult because of include restrictions in blink, sure. I don't know
much about that so I'll defer to pdr@ for it.

> 
> I also considered having a PaintImage ctor that just takes an SkImage and sets
> animation = STATIC and completion = DONE. WDYT?

Sounds good. With a comment. :)

[vmpstr, 2017-04-26 17:39:49]

On 2017/04/26 17:10:02, Khushal wrote:
> On 2017/04/26 16:57:53, vmpstr wrote:
> > On 2017/04/26 16:57:01, vmpstr wrote:
> > > On 2017/04/26 03:06:43, Khushal wrote:
> > > > lgtm.
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
> (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058:
> sk_image
> > =
> > > > image->ImageForCurrentFrame();
> > > > May be a PaintImageForCurrentFrame on blink::Image so we don't end up
> doing
> > > the
> > > > animation and completion state thing in a lot of places?
> > > 
> > > Do you mind if we do this as a follow-up? pdr@ suggested a similar thing,
> > which
> > > I pushed back on, but I take that back since it does seem to be in more
> places
> > > than ideal.
> 
> If its difficult because of include restrictions in blink, sure. I don't know
> much about that so I'll defer to pdr@ for it.
> 
> > 
> > I also considered having a PaintImage ctor that just takes an SkImage and
sets
> > animation = STATIC and completion = DONE. WDYT?
> 
> Sounds good. With a comment. :)

I just added default values to the ctor, so I think it's pretty self
documenting. PTAL.

[Khushal, 2017-04-26 18:00:58]

On 2017/04/26 17:39:49, vmpstr wrote:
> On 2017/04/26 17:10:02, Khushal wrote:
> > On 2017/04/26 16:57:53, vmpstr wrote:
> > > On 2017/04/26 16:57:01, vmpstr wrote:
> > > > On 2017/04/26 03:06:43, Khushal wrote:
> > > > > lgtm.
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
> > (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058:
> > sk_image
> > > =
> > > > > image->ImageForCurrentFrame();
> > > > > May be a PaintImageForCurrentFrame on blink::Image so we don't end up
> > doing
> > > > the
> > > > > animation and completion state thing in a lot of places?
> > > > 
> > > > Do you mind if we do this as a follow-up? pdr@ suggested a similar
thing,
> > > which
> > > > I pushed back on, but I take that back since it does seem to be in more
> > places
> > > > than ideal.
> > 
> > If its difficult because of include restrictions in blink, sure. I don't
know
> > much about that so I'll defer to pdr@ for it.
> > 
> > > 
> > > I also considered having a PaintImage ctor that just takes an SkImage and
> sets
> > > animation = STATIC and completion = DONE. WDYT?
> > 
> > Sounds good. With a comment. :)
> 
> I just added default values to the ctor, so I think it's pretty self
> documenting. PTAL.

lgtm.

[pdr., 2017-04-26 21:34:09]

On 2017/04/26 at 18:00:58, khushalsagar wrote:
> On 2017/04/26 17:39:49, vmpstr wrote:
> > On 2017/04/26 17:10:02, Khushal wrote:
> > > On 2017/04/26 16:57:53, vmpstr wrote:
> > > > On 2017/04/26 16:57:01, vmpstr wrote:
> > > > > On 2017/04/26 03:06:43, Khushal wrote:
> > > > > > lgtm.
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > > > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
> > > (right):
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
https://codereview.chromium.org/2835373003/diff/1/third_party/WebKit/Source/p...
> > > > > > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:1058:
> > > sk_image
> > > > =
> > > > > > image->ImageForCurrentFrame();
> > > > > > May be a PaintImageForCurrentFrame on blink::Image so we don't end
up
> > > doing
> > > > > the
> > > > > > animation and completion state thing in a lot of places?
> > > > > 
> > > > > Do you mind if we do this as a follow-up? pdr@ suggested a similar
thing,
> > > > which
> > > > > I pushed back on, but I take that back since it does seem to be in
more
> > > places
> > > > > than ideal.
> > > 
> > > If its difficult because of include restrictions in blink, sure. I don't
know
> > > much about that so I'll defer to pdr@ for it.
> > > 
> > > > 
> > > > I also considered having a PaintImage ctor that just takes an SkImage
and
> > sets
> > > > animation = STATIC and completion = DONE. WDYT?
> > > 
> > > Sounds good. With a comment. :)
> > 
> > I just added default values to the ctor, so I think it's pretty self
> > documenting. PTAL.
> 
> lgtm.

LGTM

[vmpstr, 2017-04-26 23:39:42]

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

[vmpstr, 2017-04-26 23:39:43]

+nick for content

Please take a look.

[ncarter (slow), 2017-04-27 20:00:09]

lgtm

[vmpstr, 2017-04-27 20:01:19]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-04-27 20:01:51]

The CQ bit was unchecked by vmpstr@chromium.org

[vmpstr, 2017-04-27 20:02:09]

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

[commit-bot: I haz the power, 2017-04-27 20:03:33]

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-04-27 20:07:22]

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

[commit-bot: I haz the power, 2017-04-27 20:07:24]

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

[vmpstr, 2017-04-27 21:22:15]

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

[commit-bot: I haz the power, 2017-04-27 21:22:45]

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-04-27 21:42:48]

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

[commit-bot: I haz the power, 2017-04-27 21:42:49]

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

[vmpstr, 2017-04-28 16:40:47]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-04-28 16:40:48]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
nick@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/2835373003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-28 16:41:28]

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-04-28 17:29:26]

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

[commit-bot: I haz the power, 2017-04-28 17:29:28]

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

[vmpstr, 2017-04-28 18:59:13]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-04-28 18:59:13]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
nick@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/2835373003/#ps60001
(title: "fixwindows")

[commit-bot: I haz the power, 2017-04-28 19:00:44]

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-04-28 19:34:27]

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

[commit-bot: I haz the power, 2017-04-28 19:34:28]

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

[vmpstr, 2017-04-28 22:02:24]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-04-28 22:02:24]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
nick@chromium.org, khushalsagar@chromium.org
Link to the patchset: https://codereview.chromium.org/2835373003/#ps80001
(title: "update")

[commit-bot: I haz the power, 2017-04-28 22:03:28]

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-04-29 00:46:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493416944014440,
"parent_rev": "5ecf11b9c4f09559336a513b4d0486a3f74d2625", "commit_rev":
"55c7657caad796ca2097e804cedb40b4d8d796d3"}

[commit-bot: I haz the power, 2017-04-29 00:47:45]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493416944014440,
"parent_rev": "5ecf11b9c4f09559336a513b4d0486a3f74d2625", "commit_rev":
"55c7657caad796ca2097e804cedb40b4d8d796d3"}

[commit-bot: I haz the power, 2017-04-29 00:47:47]

Description was changed from

==========
Plumb PaintImage to the PictureImageLayer.

This patch ensures that we pass PaintImage with all of the proper flags
to PictureImageLayer.

R=khushalsagar@chromium.org, pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Plumb PaintImage to the PictureImageLayer.

This patch ensures that we pass PaintImage with all of the proper flags
to PictureImageLayer.

R=khushalsagar@chromium.org, pdr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2835373003
Cr-Commit-Position: refs/heads/master@{#468196}
Committed:
https://chromium.googlesource.com/chromium/src/+/55c7657caad796ca2097e804cedb...
==========

[commit-bot: I haz the power, 2017-04-29 00:47:49]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/55c7657caad796ca2097e804cedb...