Remove uses of skia::GetWritablePixels(PaintCanvas)

Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

Since all of these GetWritablePixels callers are using
SkCanvasVideoRenderer, take the opportunity to properly convert
that to PaintCanvas as well.

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

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

[enne (OOO), 2017-02-09 22:00:44]

Description was changed from

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

BUG=671433
==========

to

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

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

[enne (OOO), 2017-02-09 22:01:07]

enne@chromium.org changed reviewers:
+ danakj@chromium.org, miu@chromium.org

[enne (OOO), 2017-02-09 22:01:26]

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

[commit-bot: I haz the power, 2017-02-09 22:02:16]

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

[danakj, 2017-02-09 23:13:50]

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc
File cc/paint/paint_canvas.cc (right):

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc#ne...
cc/paint/paint_canvas.cc:25: if (!canvas || !output) {
ಠ_ಠ

is needed?

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h
File cc/paint/paint_canvas.h (right):

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h#new...
cc/paint/paint_canvas.h:27: CC_PAINT_EXPORT bool ToPixmap(PaintCanvas* canvas,
SkPixmap* output);
Should gfx::Canvas::ToBitmap use this?

[enne (OOO), 2017-02-09 23:18:24]

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h
File cc/paint/paint_canvas.h (right):

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h#new...
cc/paint/paint_canvas.h:27: CC_PAINT_EXPORT bool ToPixmap(PaintCanvas* canvas,
SkPixmap* output);
On 2017/02/09 at 23:13:50, danakj (slow) wrote:
> Should gfx::Canvas::ToBitmap use this?

I don't think so.  I think this is SkPixmap vs SkBitmap.

[danakj, 2017-02-09 23:21:09]

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h
File cc/paint/paint_canvas.h (right):

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.h#new...
cc/paint/paint_canvas.h:27: CC_PAINT_EXPORT bool ToPixmap(PaintCanvas* canvas,
SkPixmap* output);
On 2017/02/09 23:18:24, enne wrote:
> On 2017/02/09 at 23:13:50, danakj (slow) wrote:
> > Should gfx::Canvas::ToBitmap use this?
> 
> I don't think so.  I think this is SkPixmap vs SkBitmap.

Oh, I'm thinkin of SkPixelRef

[danakj, 2017-02-09 23:23:58]

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc
File cc/paint/paint_canvas.cc (right):

https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc#ne...
cc/paint/paint_canvas.cc:25: if (!canvas || !output) {
On 2017/02/09 23:13:50, danakj (slow) wrote:
> ಠ_ಠ
> 
> is needed?

I can't see why this would be needed, so LGTM with this gone

[enne (OOO), 2017-02-09 23:32:12]

Description was changed from

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

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

to

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

Since all of these GetWritablePixels callers are using
SkCanvasVideoRenderer, take the opportunity to properly convert
that to PaintCanvas as well.

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

[enne (OOO), 2017-02-09 23:33:14]

On 2017/02/09 at 23:23:58, danakj wrote:
> https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc
> File cc/paint/paint_canvas.cc (right):
> 
>
https://codereview.chromium.org/2686173003/diff/1/cc/paint/paint_canvas.cc#ne...
> cc/paint/paint_canvas.cc:25: if (!canvas || !output) {
> On 2017/02/09 23:13:50, danakj (slow) wrote:
> > ಠ_ಠ
> > 
> > is needed?
> 
> I can't see why this would be needed, so LGTM with this gone

Oops, sorry for the cruft.


Since all of these callers are using SkCanvasVideoRenderer at the callsites of
GetWritablePixels, I converted that as well here.

[miu, 2017-02-09 23:39:10]

lgtm (Note: I'm not the owner of any of this code, but the change seems
straightforward; so, look's good!)

[enne (OOO), 2017-02-09 23:45:04]

On 2017/02/09 at 23:39:10, miu wrote:
> lgtm (Note: I'm not the owner of any of this code, but the change seems
straightforward; so, look's good!)

https://cs.chromium.org/chromium/src/content/renderer/media/OWNERS?q=content/...
<_<

[enne (OOO), 2017-02-09 23:45:36]

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

[enne (OOO), 2017-02-09 23:46:38]

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

[enne (OOO), 2017-02-09 23:46:39]

rjkroege: content/browser/renderer_host OWNERS
watk: media OWNERS

[enne (OOO), 2017-02-09 23:47:08]

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

[commit-bot: I haz the power, 2017-02-09 23:47:44]

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-02-10 02:00:21]

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

[commit-bot: I haz the power, 2017-02-10 02:00:22]

Dry run: This issue passed the CQ dry run.

[rjkroege, 2017-02-10 02:56:24]

lgtm

[enne (OOO), 2017-02-10 18:52:51]

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

[commit-bot: I haz the power, 2017-02-10 18:53:49]

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-02-10 20:53:47]

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

[commit-bot: I haz the power, 2017-02-10 20:53:48]

Dry run: This issue passed the CQ dry run.

[enne (OOO), 2017-02-10 21:02:07]

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

[enne (OOO), 2017-02-10 21:02:17]

+ddorwin as alternate media OWNER

[watk, 2017-02-10 21:44:09]

sorry, looking now

[watk, 2017-02-10 21:52:27]

lgtm

[enne (OOO), 2017-02-10 22:02:36]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2017-02-10 22:02:36]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
rjkroege@chromium.org, miu@chromium.org
Link to the patchset: https://codereview.chromium.org/2686173003/#ps40001
(title: "Add more deps")

[commit-bot: I haz the power, 2017-02-10 22:03:18]

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-02-10 22:10:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486764156425340,
"parent_rev": "c02aa3d0557d8f3b5ea70e3a362700f2f87cc310", "commit_rev":
"f08671daf99407814b72eb5cbfd3d1af7069ef13"}

[commit-bot: I haz the power, 2017-02-10 22:11:08]

Description was changed from

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

Since all of these GetWritablePixels callers are using
SkCanvasVideoRenderer, take the opportunity to properly convert
that to PaintCanvas as well.

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

to

==========
Remove uses of skia::GetWritablePixels(PaintCanvas)

The platform_canvas function is not usable when PaintCanvas is a
different type than SkCanvas.  Route callers through a new function
instead.

There a number of callers that still want to use SkCanvas, so
have left the original function.

Since all of these GetWritablePixels callers are using
SkCanvasVideoRenderer, take the opportunity to properly convert
that to PaintCanvas as well.

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

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

[commit-bot: I haz the power, 2017-02-10 22:11:09]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f08671daf99407814b72eb5cbfd3...