Rename paint data structures

Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

This patch will be followed up with another patch to change all of
the identifiers that refer to old names.

See: https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIboBY/edit?disco=AAAAA5r88Q0

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

Review-Url: https://codereview.chromium.org/2640983002
Cr-Commit-Position: refs/heads/master@{#447861}
Committed: https://chromium.googlesource.com/chromium/src/+/34f6084cd35016a633942588724e6dd7dae428a3

[enne (OOO), 2017-01-18 20:06:20]

Description was changed from

==========
custom display list headers

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

to

==========
custom display list headers

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[enne (OOO), 2017-01-18 20:06:21]

Description was changed from

==========
custom display list headers

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
custom display list headers

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[enne (OOO), 2017-01-18 20:06:23]

Description was changed from

==========
custom display list headers

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
custom display list headers

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

[enne (OOO), 2017-01-18 20:58:10]

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

[commit-bot: I haz the power, 2017-01-18 20:58: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-01-18 21:06:55]

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

[commit-bot: I haz the power, 2017-01-18 21:06:57]

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...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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-01-18 23:10:52]

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

[commit-bot: I haz the power, 2017-01-18 23:11:30]

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-18 23:19:26]

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

[commit-bot: I haz the power, 2017-01-18 23:19:27]

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...)
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  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_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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...)

[enne (OOO), 2017-01-18 23:37:25]

Description was changed from

==========
custom display list headers

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
==========

[enne (OOO), 2017-01-18 23:37:45]

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

[enne (OOO), 2017-01-18 23:40:16]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
==========

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
==========

[enne (OOO), 2017-01-19 00:00:07]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
==========

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[enne (OOO), 2017-01-19 00:00:08]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[enne (OOO), 2017-01-19 00:00:10]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[enne (OOO), 2017-01-19 00:00:16]

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

[commit-bot: I haz the power, 2017-01-19 00:01: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-19 00:09:13]

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

[commit-bot: I haz the power, 2017-01-19 00:09:14]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[enne (OOO), 2017-01-19 01:17:57]

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

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

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-19 01:33:54]

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

[commit-bot: I haz the power, 2017-01-19 01:33:56]

Dry run: Try jobs failed on following builders:
  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...)

[enne (OOO), 2017-01-19 02:08:19]

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

[commit-bot: I haz the power, 2017-01-19 02:08:46]

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-19 02:16:41]

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

[commit-bot: I haz the power, 2017-01-19 02:16:43]

Dry run: 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...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[enne (OOO), 2017-01-19 21:34:58]

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

[commit-bot: I haz the power, 2017-01-19 21:35:28]

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

[enne (OOO), 2017-01-19 21:35:57]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

[enne (OOO), 2017-01-19 21:57:00]

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

[commit-bot: I haz the power, 2017-01-19 22:07:37]

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

[commit-bot: I haz the power, 2017-01-19 22:07:39]

Dry run: Try jobs failed on following builders:
  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-01-19 22:16:50]

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

[commit-bot: I haz the power, 2017-01-19 22:17:40]

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

[enne (OOO), 2017-01-19 23:28:13]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platfom without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.  One could imagine
display lists and property trees migrating here once Blink wanted
to use them.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

[enne (OOO), 2017-01-20 00:23:24]

I think this is ready to take a look at and trybots finally agree.

I will point out that I have left a bunch of
PaintCanvasPassThrough/WrapSkShader/ToSkPaint/ToSkPicture functions from
vmiura's patch here.  They're noops, except PaintCanvasPassThrough is an
indirection for things that want to write to bitmaps.  I'm happy to remove them
here if any of you think that it's just noise that's not being used, but I'm
pretty sure I'll need most of them shortly in the next patch.

[commit-bot: I haz the power, 2017-01-20 00:36:46]

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

[commit-bot: I haz the power, 2017-01-20 00:36:49]

Dry run: This issue passed the CQ dry run.

[vmiura, 2017-01-20 03:38:19]

On 2017/01/20 00:23:24, enne wrote:
> I think this is ready to take a look at and trybots finally agree.
> 
> I will point out that I have left a bunch of
> PaintCanvasPassThrough/WrapSkShader/ToSkPaint/ToSkPicture functions from
> vmiura's patch here.  They're noops, except PaintCanvasPassThrough is an
> indirection for things that want to write to bitmaps.  I'm happy to remove
them
> here if any of you think that it's just noise that's not being used, but I'm
> pretty sure I'll need most of them shortly in the next patch.

If it helps, we could remove PaintCanvasPassThrough that write to SkBitmaps and
assume PaintCanvas implements a constructor with SkBitmap support.

[enne (OOO), 2017-01-20 18:24:37]

On 2017/01/20 at 03:38:19, vmiura wrote:
> On 2017/01/20 00:23:24, enne wrote:
> > I think this is ready to take a look at and trybots finally agree.
> > 
> > I will point out that I have left a bunch of
> > PaintCanvasPassThrough/WrapSkShader/ToSkPaint/ToSkPicture functions from
> > vmiura's patch here.  They're noops, except PaintCanvasPassThrough is an
> > indirection for things that want to write to bitmaps.  I'm happy to remove
them
> > here if any of you think that it's just noise that's not being used, but I'm
> > pretty sure I'll need most of them shortly in the next patch.
> 
> If it helps, we could remove PaintCanvasPassThrough that write to SkBitmaps
and assume PaintCanvas implements a constructor with SkBitmap support.

I don't think it's an issue.  I just wanted to mention it because it sticks out
vs the rest of the patch.

[danakj, 2017-01-20 23:34:15]

ok made it halfway thru, into the middle of blink stuff

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn
File cc/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn#newcode547
cc/BUILD.gn:547: "//cc/paint",
Hm, is this really what you want? It does match the skia thing, but components
could use cc without using cc/paint/. Just making sure super intentional to be
public here.

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
File cc/layers/picture_layer.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
cc/layers/picture_layer.cc:152: // the picture. For now we just paint a fresh
one to get consistent results.
"picture" no longer right

https://codereview.chromium.org/2640983002/diff/120001/cc/output/renderer_pix...
File cc/output/renderer_pixeltest.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/output/renderer_pix...
cc/output/renderer_pixeltest.cc:953: PaintFlags black_paint;
missed opportunity for Black Flag reference

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/BUILD.gn
File cc/paint/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/BUILD.gn#newc...
cc/paint/BUILD.gn:23: "//skia",
Can you make comments about base stuff so people know?

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas.cc
File cc/paint/paint_canvas.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas....
cc/paint/paint_canvas.cc:9:
PaintCanvasPassThrough::PaintCanvasPassThrough(SkCanvas* canvas)
Should we unit test this thing?

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas....
cc/paint/paint_canvas.cc:22: PaintCanvasPassThrough::~PaintCanvasPassThrough()
{}
 = default

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

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas....
cc/paint/paint_canvas.h:15: typedef SkCanvas PaintCanvas;
s/typedef/using/g

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_shader.h
File cc/paint/paint_shader.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_shader....
cc/paint/paint_shader.h:33: inline sk_sp<PaintShader>
MakePaintShaderPicture(sk_sp<PaintRecord> record,
should this have Picture in its name or PaintRecord or something?

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
File cc/playback/display_item_list.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list.cc:102: // The flattened PaintRecord approach is
going away, and the proto
can rebase this away

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
File cc/playback/display_item_list_unittest.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:320: PaintCanvas
expected_canvas(expected_bitmap);
This should stay SkCanvas right

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:376: PaintCanvas
expected_canvas(expected_bitmap);
same

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:433: PaintCanvas
expected_canvas(expected_bitmap);
same

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:504: PaintCanvas
expected_canvas(expected_bitmap);
same

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:555: // Make an PaintRecord whose size
is known.
a

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/raster_sou...
File cc/playback/raster_source.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/raster_sou...
cc/playback/raster_source.cc:18: #include
"third_party/skia/include/core/SkClipStack.h"
is used?

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/skip_image...
File cc/playback/skip_image_canvas.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/skip_image...
cc/playback/skip_image_canvas.h:18: explicit SkipImageCanvas(PaintCanvas*
canvas);
do these make sense cuz it's inheriting from a SkCanvas?

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/transform_...
File cc/playback/transform_display_item.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/transform_...
cc/playback/transform_display_item.h:17: class PaintCanvas;
can u fwd decl a type alias like that?

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_content_l...
File cc/test/fake_content_layer_client.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_content_l...
cc/test/fake_content_layer_client.h:96: typedef
std::vector<std::pair<gfx::RectF, PaintFlags>> RectPaintVector;
nit: s/typedef/using/ while u here?

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_raster_so...
File cc/test/fake_raster_source.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_raster_so...
cc/test/fake_raster_source.cc:142: PaintCanvas* canvas,
shouldnt this be an SkCanvas? this is raster

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc
File cc/test/skia_common.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc...
cc/test/skia_common.cc:50: PaintCanvas canvas(bitmap);
this is raster, shouldn't this be SkCanvas?

https://codereview.chromium.org/2640983002/diff/120001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_pixeltest_masks.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_pixeltest_masks.cc:108: PaintCanvas* canvas =
surface->getCanvas();
SkSurface can't return a PaintCanvas

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/create_application_shortcut_view.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/create_application_shortcut_view.cc:191:
canvas->sk_canvas()->drawRoundRect(border_rect, SkIntToScalar(2),
sk_canvas() is a lie now, followup or?

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/infobars/infobar_background.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/infobars/infobar_background.cc:7: #include
"cc/paint/paint_canvas.h"
why paintcanvas? this uses gfx::canvas

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/infobars/infobar_container_view.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/infobars/infobar_container_view.cc:7: #include
"cc/paint/paint_shader.h"
should include paint_flags too?

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
File chrome/renderer/pepper/DEPS (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
chrome/renderer/pepper/DEPS:2: "+cc/paint",
i dont think this is needed, see following..

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
File chrome/renderer/pepper/pepper_flash_renderer_host.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
chrome/renderer/pepper/pepper_flash_renderer_host.cc:225: cc::PaintCanvas*
canvas = image->GetCanvas();
image doesn't return a PaintCanvas right? so nothing in here should change?

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
File components/printing/renderer/DEPS (right):

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
components/printing/renderer/DEPS:2: "+cc/paint",
printing doesn't go thru compositor, so should it be using PaintCanvas?

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
File components/printing/renderer/print_web_view_helper.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
components/printing/renderer/print_web_view_helper.cc:1880: cc::PaintCanvas*
canvas =
metafile is a PdfMetafileSkia, it won't return a PaintCanvas right?

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
File components/printing/renderer/print_web_view_helper_mac.mm (right):

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
components/printing/renderer/print_web_view_helper_mac.mm:138: cc::PaintCanvas*
canvas = metafile->GetVectorCanvasForNewPage(
same?

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
File components/test_runner/mock_web_theme_engine.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
components/test_runner/mock_web_theme_engine.cc:120: void box(cc::PaintCanvas*
canvas, const SkIRect& rect, SkColor fillColor) {
argh blink style names

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
File components/test_runner/pixel_dump.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
components/test_runner/pixel_dump.cc:101: std::unique_ptr<cc::PaintCanvas>
canvas = skia::TryCreateBitmapCanvas(
this looks like a type mismatch? also noting that this is printing so hm

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/media...
File content/renderer/media/html_video_element_capturer_source.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/media...
content/renderer/media/html_video_element_capturer_source.cc:143:
cc::GetSkCanvas(canvas)->imageInfo().colorType());
do these have to go thru GetSkCanvas? Can we do something a bit better cuz that
will get expensive later right just to get simple info off the canvas?

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/peppe...
File content/renderer/pepper/pepper_graphics_2d_host.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/peppe...
content/renderer/pepper/pepper_graphics_2d_host.cc:741: cc::PaintCanvas*
backing_canvas = image_data_->GetCanvas();
does image_data_ really return a cc::PaintCanvas?

https://codereview.chromium.org/2640983002/diff/120001/ios/build/config.gni
File ios/build/config.gni (right):

https://codereview.chromium.org/2640983002/diff/120001/ios/build/config.gni#n...
ios/build/config.gni:8: "//cc:cc",
//cc is how this is written

https://codereview.chromium.org/2640983002/diff/120001/printing/DEPS
File printing/DEPS (right):

https://codereview.chromium.org/2640983002/diff/120001/printing/DEPS#newcode2
printing/DEPS:2: "+cc/paint",
really not clear this dir should be using cc::paint. it's passing the canvas to
skia:: methods

https://codereview.chromium.org/2640983002/diff/120001/printing/metafile_skia...
File printing/metafile_skia_wrapper.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/printing/metafile_skia...
printing/metafile_skia_wrapper.cc:33: SkMetaData& meta =
skia::GetMetaData(canvas);
like so..

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/BoxReflectionUtils.cpp (right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/BoxReflectionUtils.cpp:59: // If
NinePieceImagePainter operated on SkCanvas, we'd only need an
an->a

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:115: if
(PaintCanvas* skCanvas = existingDrawingCanvas()) {
we should maybe keep track of some patterns for rename like

PaintCanvas skCanvas
PaintCanvas sk_canvas
PaintRecord picture

kinda stuff?

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/BUILD.gn:1444: "//cc/paint",
why public deps?

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/fonts/Font.cpp (right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/fonts/Font.cpp:412: SkPaint skPaint =
ToSkPaint(paint);
Why do you wanna SkPaint?

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
(right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:84: static
sk_sp<PaintSurface> createSkSurface(GrContext* gr,
I feel like this should stay SkSurface and we should just wrap the canvas from
it with a passthru PaintCanvas

[danakj, 2017-01-20 23:35:58]

High level questions so far are:
- Printing is raster/pdf/skia based so I think maybe it needs more wrapping into
a passthru PaintCanvas.
- PaintSurface seems like it should stay SkSurface and wrap the canvas in a
passthru PaintCanvas.

[vmpstr, 2017-01-21 01:11:28]

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

[vmpstr, 2017-01-21 01:11:31]

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
File cc/layers/picture_layer.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
cc/layers/picture_layer.cc:209: void
PictureLayer::ToLayerPropertiesProto(proto::LayerProperties* proto) {
I think these proto functions are gone. Might be worth rebasing on
https://codereview.chromium.org/2646623002/ (it's in tot already)

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_layer.h
File cc/layers/picture_layer.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
cc/layers/picture_layer.h:38: sk_sp<PaintRecord> GetPicture() const override;
nit: Need to rename GetPicture as well, (or at least file a bug/TODO please?)

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_flags.h
File cc/paint/paint_flags.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_flags.h...
cc/paint/paint_flags.h:11: typedef SkPaint PaintFlags;
nit: using

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_flags.h...
cc/paint/paint_flags.h:13: inline const SkPaint& ToSkPaint(const PaintFlags&
paint) {
I like these functions, thanks!

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_record.h
File cc/paint/paint_record.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_record....
cc/paint/paint_record.h:24: return record;
Should these be std::move? I think for g++, it might make a difference. But
AFAIK clang++ would move, since it's already implemented the c++14 portion of
this.

[vmiura, 2017-01-21 01:25:55]

On 2017/01/20 23:35:58, danakj (slow) wrote:
> High level questions so far are:
> - Printing is raster/pdf/skia based so I think maybe it needs more wrapping
into
> a passthru PaintCanvas.
> - PaintSurface seems like it should stay SkSurface and wrap the canvas in a
> passthru PaintCanvas.


Originally did this in CDL, but lifetime ownership of the passthru Canvas became
a lot more complex in several parts of code.  I think we should keep the
PainSurface.

[enne (OOO), 2017-01-23 18:44:36]

On 2017/01/21 at 01:25:55, vmiura wrote:
> On 2017/01/20 23:35:58, danakj (slow) wrote:
> > High level questions so far are:
> > - Printing is raster/pdf/skia based so I think maybe it needs more wrapping
into
> > a passthru PaintCanvas.
> > - PaintSurface seems like it should stay SkSurface and wrap the canvas in a
> > passthru PaintCanvas.
> 
> 
> Originally did this in CDL, but lifetime ownership of the passthru Canvas
became a lot more complex in several parts of code.  I think we should keep the
PainSurface.

Is this mostly about Canvas2DLayerBridge?

[enne (OOO), 2017-01-24 01:51:28]

Thanks for the detailed review, danakj and vmpstr.  I looked at everything
mentioned so far.  I reverted a number of modifications to printing and pepper.

danakj: It seems like you're not in agreement with me adding things to public
deps.  Can you help me understand when you think that's a thing to do and when
it's not?

Sorry about leaving things in such a "PaintRecord picture" state everywhere, but
I was trying not to change all the things.  I will follow up on it immediately
after this patch.

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn
File cc/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn#newcode547
cc/BUILD.gn:547: "//cc/paint",
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> Hm, is this really what you want? It does match the skia thing, but components
could use cc without using cc/paint/. Just making sure super intentional to be
public here.

Yeah.  cc/paint is in the public interface of cc.  I don't think you can use cc
without using cc/paint.  I think components that want to be used separately (hi
cc/surfaces and cc/animation) should be separate components.

What would you use out of cc that wouldn't involve cc/paint?

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
File cc/layers/picture_layer.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/layers/picture_laye...
cc/layers/picture_layer.cc:152: // the picture. For now we just paint a fresh
one to get consistent results.
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> "picture" no longer right

Changed here.  In general, I'll follow up with changing picture identifiers to
record in a different patch, since this patch is already gigantic.  I've left
some TODOs as per vmpstr's request.

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/BUILD.gn
File cc/paint/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/BUILD.gn#newc...
cc/paint/BUILD.gn:23: "//skia",
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> Can you make comments about base stuff so people know?

Done.

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas.cc
File cc/paint/paint_canvas.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_canvas....
cc/paint/paint_canvas.cc:9:
PaintCanvasPassThrough::PaintCanvasPassThrough(SkCanvas* canvas)
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> Should we unit test this thing?

This gets used in a bunch of tests.  I'm happy to add another test if you'd
like.  What in particular are you concerned about regressing?

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_record.h
File cc/paint/paint_record.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_record....
cc/paint/paint_record.h:24: return record;
On 2017/01/21 at 01:11:31, vmpstr wrote:
> Should these be std::move? I think for g++, it might make a difference. But
AFAIK clang++ would move, since it's already implemented the c++14 portion of
this.

I think this is ok because of the typedef.

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_shader.h
File cc/paint/paint_shader.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/paint/paint_shader....
cc/paint/paint_shader.h:33: inline sk_sp<PaintShader>
MakePaintShaderPicture(sk_sp<PaintRecord> record,
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> should this have Picture in its name or PaintRecord or something?

PaintShaderPicture => PaintShaderRecord.

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
File cc/playback/display_item_list_unittest.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/display_it...
cc/playback/display_item_list_unittest.cc:320: PaintCanvas
expected_canvas(expected_bitmap);
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> This should stay SkCanvas right

Good catch, thanks.

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/skip_image...
File cc/playback/skip_image_canvas.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/skip_image...
cc/playback/skip_image_canvas.h:18: explicit SkipImageCanvas(PaintCanvas*
canvas);
On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> do these make sense cuz it's inheriting from a SkCanvas?

I had a thought of how I could make this work in the future, but I'll revert
this for now and maybe it'll make more sense in the future.

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/transform_...
File cc/playback/transform_display_item.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/playback/transform_...
cc/playback/transform_display_item.h:17: class PaintCanvas;
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> can u fwd decl a type alias like that?

Oh toooootally <_< *hastily erases it*

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_content_l...
File cc/test/fake_content_layer_client.h (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/fake_content_l...
cc/test/fake_content_layer_client.h:96: typedef
std::vector<std::pair<gfx::RectF, PaintFlags>> RectPaintVector;
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> nit: s/typedef/using/ while u here?

Sure thing.

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc
File cc/test/skia_common.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc...
cc/test/skia_common.cc:50: PaintCanvas canvas(bitmap);
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> this is raster, shouldn't this be SkCanvas?

So, this depends on what you think the future should look like in terms of the
RasterSource/DisplayItemList/DisplayItem interface.  Should they have:

(a) Raster(PaintCanvas*), and if you want to use an SkCanvas to raster directly,
then you wrap it in a PaintCanvas.
(b) Raster(PaintCanvas*) and Raster(SkCanvas*) with extremely similar
functionality.

I think as part of the command buffer canvas step, we're going to want to
"raster" into the PaintCanvas interface.  In a far off future, I'd love to just
transport display lists and paint records (and even then only transport pieces
that have been changed), but I think we should ship the command buffer canvas
intermediate step in the short term.

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/create_application_shortcut_view.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/create_application_shortcut_view.cc:191:
canvas->sk_canvas()->drawRoundRect(border_rect, SkIntToScalar(2),
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> sk_canvas() is a lie now, followup or?

Followup.  Left a TODO.

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/infobars/infobar_background.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/infobars/infobar_background.cc:7: #include
"cc/paint/paint_canvas.h"
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> why paintcanvas? this uses gfx::canvas

canvas->sk_canvas()->drawLine(0, y, w, y, stroke);

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
File chrome/renderer/pepper/pepper_flash_renderer_host.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/chrome/renderer/pepper...
chrome/renderer/pepper/pepper_flash_renderer_host.cc:225: cc::PaintCanvas*
canvas = image->GetCanvas();
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> image doesn't return a PaintCanvas right? so nothing in here should change?

Thanks.  Reverted this.

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
File components/printing/renderer/DEPS (right):

https://codereview.chromium.org/2640983002/diff/120001/components/printing/re...
components/printing/renderer/DEPS:2: "+cc/paint",
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> printing doesn't go thru compositor, so should it be using PaintCanvas?

Sure.  Makes sense.  I think I was thinking about printing going through
painting, which would use PaintCanvas, but maybe that's not relevant here.

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
File components/test_runner/pixel_dump.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/components/test_runner...
components/test_runner/pixel_dump.cc:101: std::unique_ptr<cc::PaintCanvas>
canvas = skia::TryCreateBitmapCanvas(
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> this looks like a type mismatch? also noting that this is printing so hm

Yeah, I was trying to avoid changing too much logic here.  I rewrote this using
SkBitmap and tryAllocN32Pixels.

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/peppe...
File content/renderer/pepper/pepper_graphics_2d_host.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/content/renderer/peppe...
content/renderer/pepper/pepper_graphics_2d_host.cc:741: cc::PaintCanvas*
backing_canvas = image_data_->GetCanvas();
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> does image_data_ really return a cc::PaintCanvas?

Thanks, missed this one as a part of reverting changes.  I got the PPB_etc_etc
stuff, but not this.

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:115: if
(PaintCanvas* skCanvas = existingDrawingCanvas()) {
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> we should maybe keep track of some patterns for rename like
> 
> PaintCanvas skCanvas
> PaintCanvas sk_canvas
> PaintRecord picture
> 
> kinda stuff?

Yeah, I will follow up with a patch.  This patch is gigantic already without
also changing those.

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/BUILD.gn:1444: "//cc/paint",
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> why public deps?

Because it includes files from cc/paint? So if you include platform, you're also
including stuff from there? Do you feel like this shouldn't be a dependency?

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/fonts/Font.cpp (right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/fonts/Font.cpp:412: SkPaint skPaint =
ToSkPaint(paint);
On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> Why do you wanna SkPaint?

See my comments in comment #1.  I'll just remove these for now since they're
confusing.  I'll also remove the GetCanvas calls.

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
(right):

https://codereview.chromium.org/2640983002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:84: static
sk_sp<PaintSurface> createSkSurface(GrContext* gr,
On 2017/01/20 at 23:34:15, danakj (slow) wrote:
> I feel like this should stay SkSurface and we should just wrap the canvas from
it with a passthru PaintCanvas

Going to punt on this for now, given vmiura's comments.

[enne (OOO), 2017-01-24 01:51:32]

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

[commit-bot: I haz the power, 2017-01-24 01:52: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, 2017-01-24 04:45:06]

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

[commit-bot: I haz the power, 2017-01-24 04:45:08]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-24 15:26:41]

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn
File cc/BUILD.gn (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/BUILD.gn#newcode547
cc/BUILD.gn:547: "//cc/paint",
On 2017/01/24 01:51:27, enne wrote:
> On 2017/01/20 at 23:34:13, danakj (slow) wrote:
> > Hm, is this really what you want? It does match the skia thing, but
components
> could use cc without using cc/paint/. Just making sure super intentional to be
> public here.
> 
> Yeah.  cc/paint is in the public interface of cc.  I don't think you can use
cc
> without using cc/paint.  I think components that want to be used separately
(hi
> cc/surfaces and cc/animation) should be separate components.
> 
> What would you use out of cc that wouldn't involve cc/paint?

Pepper probably uses cc and not cc/paint for example. You only use it if you're
using cc to raster content.

[danakj, 2017-01-24 15:32:07]

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc
File cc/test/skia_common.cc (right):

https://codereview.chromium.org/2640983002/diff/120001/cc/test/skia_common.cc...
cc/test/skia_common.cc:50: PaintCanvas canvas(bitmap);
On 2017/01/24 01:51:28, enne wrote:
> On 2017/01/20 at 23:34:14, danakj (slow) wrote:
> > this is raster, shouldn't this be SkCanvas?
> 
> So, this depends on what you think the future should look like in terms of the
> RasterSource/DisplayItemList/DisplayItem interface.  Should they have:
> 
> (a) Raster(PaintCanvas*), and if you want to use an SkCanvas to raster
directly,
> then you wrap it in a PaintCanvas.
> (b) Raster(PaintCanvas*) and Raster(SkCanvas*) with extremely similar
> functionality.
> 
> I think as part of the command buffer canvas step, we're going to want to
> "raster" into the PaintCanvas interface.  In a far off future, I'd love to
just
> transport display lists and paint records (and even then only transport pieces
> that have been changed), but I think we should ship the command buffer canvas
> intermediate step in the short term.

OK ya I think you're right.

[danakj, 2017-01-24 15:32:31]

On Mon, Jan 23, 2017 at 8:51 PM, enne@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> Thanks for the detailed review, danakj and vmpstr. I looked at everything
> mentioned so far. I reverted a number of modifications to printing and
> pepper.
>
> danakj: It seems like you're not in agreement with me adding things to
> public
> deps. Can you help me understand when you think that's a thing to do and
> when
> it's not?
>

I think when you have to use the dependency in order to the the dependent
component, it makes sense as a public dep. If you would often reasonably
not use it then it doesn't. I don't have a very well-formed opinion here,
but I think we want to avoid public when reasonable. I've seen other public
things (flags rather than deps) cause surprising sadness, so now I'm wary.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[danakj, 2017-01-24 15:34:01]

On Mon, Jan 23, 2017 at 8:51 PM, enne@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> Thanks for the detailed review, danakj and vmpstr. I looked at everything
> mentioned so far. I reverted a number of modifications to printing and
> pepper.
>
> danakj: It seems like you're not in agreement with me adding things to
> public
> deps. Can you help me understand when you think that's a thing to do and
> when
> it's not?
>

I think when you have to use the dependency in order to the the dependent
component, it makes sense as a public dep. If you would often reasonably
not use it then it doesn't. I don't have a very well-formed opinion here,
but I think we want to avoid public when reasonable. I've seen other public
things (flags rather than deps) cause surprising sadness, so now I'm wary.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2017-01-24 15:35:22]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

[enne (OOO), 2017-01-24 21:53:36]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink, ui, and printing code.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

[enne (OOO), 2017-01-24 21:55:17]

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

[commit-bot: I haz the power, 2017-01-24 21:55:46]

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

[enne (OOO), 2017-01-24 21:57:13]

Ok, removed cc/paint from cc/ and ui/ public deps.  Left it in platform (which
already includes cc and skia in its public deps).

I also converted a bunch of SkPaints in ash that I missed.  I am certain that
this is not *everything* in the entire chrome codebase that needs to be changed,
but I can followup with a smaller conversion patch in the future.

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

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

[commit-bot: I haz the power, 2017-01-24 22:12:33]

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

[enne (OOO), 2017-01-24 22:45:14]

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

[commit-bot: I haz the power, 2017-01-24 22:46: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-24 23:03:51]

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

[commit-bot: I haz the power, 2017-01-24 23:03:54]

Dry run: 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_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[enne (OOO), 2017-01-25 00:42:47]

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

[commit-bot: I haz the power, 2017-01-25 00:43: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-01-25 03:15:04]

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

[commit-bot: I haz the power, 2017-01-25 03:15:07]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-25 17:33:29]

I made it up to ui/ now without looking back at stuff I already looked at.

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp:920: LoggingCanvas
canvas(bounds.width(), bounds.height());
This needs to be wrapped in a PaintCanvas. Or LoggingCanvas needs to not be an
SkCanvas

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/ReplayingCanvas.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/ReplayingCanvas.h:35: #include
"platform/graphics/paint/PaintFlags.h"
You didn't change the types below so I don't think you mean to change the
headers

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.h
(right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.h:37: //
TODO(enne): rename this to PaintRecord
GetPaintRecord I guess, or it'll collide in the blink rewrite :)

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintCanvas.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintCanvas.h:11: typedef
cc::PaintCanvas PaintCanvas;
using > typedef

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintFlags.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintFlags.h:11: typedef
cc::PaintFlags PaintFlags;
using > typedef

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintRecord.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintRecord.h:11: typedef
cc::PaintRecord PaintRecord;
using

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintRecorder.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintRecorder.h:11: typedef
cc::PaintRecorder PaintRecorder;
using

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintShader.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintShader.h:11: typedef
cc::PaintShader PaintShader;
using

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/PaintSurface.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/PaintSurface.h:11: typedef
cc::PaintSurface PaintSurface;
using

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebCanvas.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCanvas.h:34: #include "WebCommon.h"
unused

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCanvas.h:40: typedef cc::PaintCanvas
WebCanvas;
using

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebMediaPlayer.h:41: #include
"cc/paint/paint_flags.h"
is this ordered right?

[danakj, 2017-01-25 19:10:07]

Can you drop a readme.md or something in cc/paint/ saying things that are
special about it? Explaining the dependency rules and the use of sk_sp come to
mind.

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
File ui/gfx/canvas_paint_mac.mm (right):

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale, opaque);
This is going to have to make a passthru PaintCanvas to a bitmap canvas, and
store the SkCanvas or SkBitmap here, for the destructor to use instead of it
pulling it out of sk_canvas().

[danakj, 2017-01-25 19:11:31]

I didn't backtrack at all in each review session so if there's something I
looked at and you'd like me to look at it again please lmk. Otherwise LGTM.

[enne (OOO), 2017-01-25 21:06:05]

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

[commit-bot: I haz the power, 2017-01-25 21:06:55]

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

[enne (OOO), 2017-01-25 21:07:16]

Thanks for the review.  vmpstr/vmiura did you have any more comments before I go
find a billion owners and land this?

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp:920: LoggingCanvas
canvas(bounds.width(), bounds.height());
On 2017/01/25 at 17:33:28, danakj (slow) wrote:
> This needs to be wrapped in a PaintCanvas. Or LoggingCanvas needs to not be an
SkCanvas

Oops, missed this one.  LoggingCanvas and ReplayingCanvas are going to stay
SkCanvas things for the inspector, and will just be wrapped in a passthrough
canvas.

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebMediaPlayer.h (right):

https://codereview.chromium.org/2640983002/diff/260001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebMediaPlayer.h:41: #include
"cc/paint/paint_flags.h"
On 2017/01/25 at 17:33:29, danakj (slow) wrote:
> is this ordered right?

Yup yup.  check-webkit-style yells otherwise.

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
File ui/gfx/canvas_paint_mac.mm (right):

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale, opaque);
On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> This is going to have to make a passthru PaintCanvas to a bitmap canvas, and
store the SkCanvas or SkBitmap here, for the destructor to use instead of it
pulling it out of sk_canvas().

Why is that? gfx::Canvas creates a PaintSurface which gets a PaintCanvas off of
it and then this function will call some commands on the PaintCanvas.  This all
seems pretty straightforward to me.

[danakj, 2017-01-25 22:11:41]

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
File ui/gfx/canvas_paint_mac.mm (right):

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale, opaque);
On 2017/01/25 21:07:16, enne wrote:
> On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> > This is going to have to make a passthru PaintCanvas to a bitmap canvas, and
> store the SkCanvas or SkBitmap here, for the destructor to use instead of it
> pulling it out of sk_canvas().
> 
> Why is that? gfx::Canvas creates a PaintSurface which gets a PaintCanvas off
of
> it and then this function will call some commands on the PaintCanvas.  This
all
> seems pretty straightforward to me.

Cuz the destructor does peekPixels on the canvas. I think it needs an SkCanvas
behind it.

[danakj, 2017-01-25 22:12:28]

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
File ui/gfx/canvas_paint_mac.mm (right):

https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale, opaque);
On 2017/01/25 22:11:41, danakj (slow) wrote:
> On 2017/01/25 21:07:16, enne wrote:
> > On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> > > This is going to have to make a passthru PaintCanvas to a bitmap canvas,
and
> > store the SkCanvas or SkBitmap here, for the destructor to use instead of it
> > pulling it out of sk_canvas().
> > 
> > Why is that? gfx::Canvas creates a PaintSurface which gets a PaintCanvas off
> of
> > it and then this function will call some commands on the PaintCanvas.  This
> all
> > seems pretty straightforward to me.
> 
> Cuz the destructor does peekPixels on the canvas. I think it needs an SkCanvas
> behind it.

Or more precisely, I don't think PaintCanvas should even have peekPixels so it
will need an SkCanvas* inside this class that it can call that on.

[enne (OOO), 2017-01-25 22:30:24]

On 2017/01/25 at 22:12:28, danakj wrote:
>
https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
> File ui/gfx/canvas_paint_mac.mm (right):
> 
>
https://codereview.chromium.org/2640983002/diff/260001/ui/gfx/canvas_paint_ma...
> ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale, opaque);
> On 2017/01/25 22:11:41, danakj (slow) wrote:
> > On 2017/01/25 21:07:16, enne wrote:
> > > On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> > > > This is going to have to make a passthru PaintCanvas to a bitmap canvas,
and
> > > store the SkCanvas or SkBitmap here, for the destructor to use instead of
it
> > > pulling it out of sk_canvas().
> > > 
> > > Why is that? gfx::Canvas creates a PaintSurface which gets a PaintCanvas
off
> > of
> > > it and then this function will call some commands on the PaintCanvas. 
This
> > all
> > > seems pretty straightforward to me.
> > 
> > Cuz the destructor does peekPixels on the canvas. I think it needs an
SkCanvas
> > behind it.
> 
> Or more precisely, I don't think PaintCanvas should even have peekPixels so it
will need an SkCanvas* inside this class that it can call that on.

Definitely agreed that peekPixels should not exist. I'll follow up and change
CanvasSkiaPaint to paint directly into a bitmap.  I don't think that needs to
change in this patch.

[vmpstr, 2017-01-25 22:42:11]

lgtm

https://codereview.chromium.org/2640983002/diff/280001/chrome/browser/extensi...
File chrome/browser/extensions/bookmark_app_helper.cc (right):

https://codereview.chromium.org/2640983002/diff/280001/chrome/browser/extensi...
chrome/browser/extensions/bookmark_app_helper.cc:113:
background_paint.setAntiAlias(true);
tbh, the implementation of setAntiAlias is more complicated than set flags, but
¯\_(ツ)_/¯

[danakj, 2017-01-25 22:49:21]

On Wed, Jan 25, 2017 at 5:30 PM, enne@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2017/01/25 at 22:12:28, danakj wrote:
> >
> https://codereview.chromium.org/2640983002/diff/260001/ui/
> gfx/canvas_paint_mac.mm
> > File ui/gfx/canvas_paint_mac.mm (right):
> >
> >
> https://codereview.chromium.org/2640983002/diff/260001/ui/
> gfx/canvas_paint_mac.mm#newcode70
> > ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale,
> opaque);
> > On 2017/01/25 22:11:41, danakj (slow) wrote:
> > > On 2017/01/25 21:07:16, enne wrote:
> > > > On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> > > > > This is going to have to make a passthru PaintCanvas to a bitmap
> canvas,
> and
> > > > store the SkCanvas or SkBitmap here, for the destructor to use
> instead of
> it
> > > > pulling it out of sk_canvas().
> > > >
> > > > Why is that? gfx::Canvas creates a PaintSurface which gets a
> PaintCanvas
> off
> > > of
> > > > it and then this function will call some commands on the
> PaintCanvas.
> This
> > > all
> > > > seems pretty straightforward to me.
> > >
> > > Cuz the destructor does peekPixels on the canvas. I think it needs an
> SkCanvas
> > > behind it.
> >
> > Or more precisely, I don't think PaintCanvas should even have peekPixels
> so it
> will need an SkCanvas* inside this class that it can call that on.
>
> Definitely agreed that peekPixels should not exist. I'll follow up and
> change
> CanvasSkiaPaint to paint directly into a bitmap. I don't think that needs
> to
> change in this patch.
>

I agree, maybe a todo though? Also I think only the _mac one needs this.
The other doesn't peekPixels I don't think.


>
> https://codereview.chromium.org/2640983002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2017-01-25 22:52:46]

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

[commit-bot: I haz the power, 2017-01-25 22:52:49]

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

[danakj, 2017-01-25 22:56:46]

On Wed, Jan 25, 2017 at 5:30 PM, enne@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2017/01/25 at 22:12:28, danakj wrote:
> >
> https://codereview.chromium.org/2640983002/diff/260001/ui/
> gfx/canvas_paint_mac.mm
> > File ui/gfx/canvas_paint_mac.mm (right):
> >
> >
> https://codereview.chromium.org/2640983002/diff/260001/ui/
> gfx/canvas_paint_mac.mm#newcode70
> > ui/gfx/canvas_paint_mac.mm:70: RecreateBackingCanvas(size, scale,
> opaque);
> > On 2017/01/25 22:11:41, danakj (slow) wrote:
> > > On 2017/01/25 21:07:16, enne wrote:
> > > > On 2017/01/25 at 19:10:07, danakj (slow) wrote:
> > > > > This is going to have to make a passthru PaintCanvas to a bitmap
> canvas,
> and
> > > > store the SkCanvas or SkBitmap here, for the destructor to use
> instead of
> it
> > > > pulling it out of sk_canvas().
> > > >
> > > > Why is that? gfx::Canvas creates a PaintSurface which gets a
> PaintCanvas
> off
> > > of
> > > > it and then this function will call some commands on the
> PaintCanvas.
> This
> > > all
> > > > seems pretty straightforward to me.
> > >
> > > Cuz the destructor does peekPixels on the canvas. I think it needs an
> SkCanvas
> > > behind it.
> >
> > Or more precisely, I don't think PaintCanvas should even have peekPixels
> so it
> will need an SkCanvas* inside this class that it can call that on.
>
> Definitely agreed that peekPixels should not exist. I'll follow up and
> change
> CanvasSkiaPaint to paint directly into a bitmap. I don't think that needs
> to
> change in this patch.
>

I agree, maybe a todo though? Also I think only the _mac one needs this.
The other doesn't peekPixels I don't think.


>
> https://codereview.chromium.org/2640983002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[enne (OOO), 2017-01-25 23:22:18]

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

[commit-bot: I haz the power, 2017-01-25 23: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-26 01:00:26]

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

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

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

[enne (OOO), 2017-01-26 19:09:03]

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

[commit-bot: I haz the power, 2017-01-26 19:09:52]

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

[enne (OOO), 2017-01-26 19:20:04]

enne@chromium.org changed reviewers:
+ benwells@chromium.org, sdefresne@chromium.org, sky@chromium.org

[enne (OOO), 2017-01-26 19:20:07]

Sorry for the giant patch.

esprehn: Blink
benwells: apps, chrome/browser/(ui)/extensions
sky: ash, chrome/browser, chrome/renderer, ui
sdefresne: components/, ios/

[sdefresne, 2017-01-26 19:51:13]

sdefresne@chromium.org changed reviewers:
+ rohitrao@chromium.org

[sdefresne, 2017-01-26 19:51:13]

sdefresne@chromium.org changed required reviewers:
+ rohitrao@chromium.org

[sdefresne, 2017-01-26 19:51:17]

+rohitrao for //ios review

https://codereview.chromium.org/2640983002/diff/320001/ios/build/config.gni
File ios/build/config.gni (right):

https://codereview.chromium.org/2640983002/diff/320001/ios/build/config.gni#n...
ios/build/config.gni:8: "//cc",
I'm not sure we want to allow dependency on //cc:paint (at least as it is
currently) on iOS as it brings things from skia that are not supported on iOS
(canvases for example). iOS only supports a tiny subset of skia (geometry and
images IIRC).

If we want to allow that dependency, I think that //cc:paint should support
being seriously limited on iOS (due to the incomplete skia library).

[enne (OOO), 2017-01-26 20:11:33]

https://codereview.chromium.org/2640983002/diff/320001/ios/build/config.gni
File ios/build/config.gni (right):

https://codereview.chromium.org/2640983002/diff/320001/ios/build/config.gni#n...
ios/build/config.gni:8: "//cc",
On 2017/01/26 at 19:51:17, sdefresne wrote:
> I'm not sure we want to allow dependency on //cc:paint (at least as it is
currently) on iOS as it brings things from skia that are not supported on iOS
(canvases for example). iOS only supports a tiny subset of skia (geometry and
images IIRC).
> 
> If we want to allow that dependency, I think that //cc:paint should support
being seriously limited on iOS (due to the incomplete skia library).

The reason I added this is because of the ui/gfx dependency on cc/paint (in the
same way that it depends on skia).

https://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode...
//ios/web:ios_web_unittests_arch_executable_sources has an assert_no_deps entry:
  //cc/*
which fails for the dependency path:
  //ios/web:ios_web_unittests_arch_executable_sources ->
  //ios/web:run_all_unittests ->
  //ios/web:test_support ->
  //ios/web:web ->
  //ios/web:web_arc ->
  //components/url_formatter:url_formatter ->
  //ui/gfx:gfx ->
  //cc/paint:paint

Is there some way to only include this for tests? Or something else that would
satisfy your concerns?

[sky, 2017-01-26 23:00:13]

I got through a couple of files before realizing the name of variables should be
updated to reflect the new names. I recommend splitting this patch up to make
that more manageable. The other option is to proceed with not changing variable
names and come back to, but I suspect it would easier to do now. WDYT?

https://codereview.chromium.org/2640983002/diff/360001/apps/ui/views/app_wind...
File apps/ui/views/app_window_frame_view.cc (right):

https://codereview.chromium.org/2640983002/diff/360001/apps/ui/views/app_wind...
apps/ui/views/app_window_frame_view.cc:308: cc::PaintFlags paint;
paint_flags

https://codereview.chromium.org/2640983002/diff/360001/ash/touch/touch_hud_de...
File ash/touch/touch_hud_debug.cc (right):

https://codereview.chromium.org/2640983002/diff/360001/ash/touch/touch_hud_de...
ash/touch/touch_hud_debug.cc:307: cc::PaintFlags paint_;
If PaintFlags is going to be the final name, then I suggest renaming member
variables to match, e.g. paint_flags_.

https://codereview.chromium.org/2640983002/diff/360001/ash/touch_hud/touch_hu...
File ash/touch_hud/touch_hud_renderer.cc (right):

https://codereview.chromium.org/2640983002/diff/360001/ash/touch_hud/touch_hu...
ash/touch_hud/touch_hud_renderer.cc:123: cc::PaintFlags fill_paint_;
Similar comment about naming for these two.

https://codereview.chromium.org/2640983002/diff/360001/ash/utility/screenshot...
File ash/utility/screenshot_controller.cc (right):

https://codereview.chromium.org/2640983002/diff/360001/ash/utility/screenshot...
ash/utility/screenshot_controller.cc:163: cc::PaintFlags paint;
Similar comment about naming.

[enne (OOO), 2017-01-26 23:15:46]

I was going to come back and redo all the variable and parameter names.  It
seemed like this patch was big enough already.

[sky, 2017-01-26 23:32:51]

Ok, LGTM then. I have a feeling you should add DEPS to exclude the includes you
don't want people using anymore. But you can do that anytime.

[sky, 2017-01-26 23:33:49]

On 2017/01/26 23:32:51, sky wrote:
> Ok, LGTM then. I have a feeling you should add DEPS to exclude the includes
you
> don't want people using anymore. But you can do that anytime.

Also note, I did make it to ui before rubber stamping this. Too big!

[benwells, 2017-01-30 02:43:58]

lgtm.

[enne (OOO), 2017-01-30 19:05:47]

Re: ios.  Is there any explanation somewhere of what the restrictions are on
skia use in ios? It doesn't seem to me that including cc/paint doesn't really
change anything here, as skia is already included.

One possibility might be to stub out the parts of ui/gfx on ios and then drop
the cc/paint include, although that's a little bit more invasive to ui/gfx.  I'd
really like to understand how the current restrictions are handled to know how
to follow through with this patch.

[sdefresne, 2017-01-31 09:40:23]

On 2017/01/30 19:05:47, enne wrote:
> Re: ios.  Is there any explanation somewhere of what the restrictions are on
> skia use in ios? It doesn't seem to me that including cc/paint doesn't really
> change anything here, as skia is already included.
> 
> One possibility might be to stub out the parts of ui/gfx on ios and then drop
> the cc/paint include, although that's a little bit more invasive to ui/gfx. 
I'd
> really like to understand how the current restrictions are handled to know how
> to follow through with this patch.

On iOS, the part of skia that can be used are written down in ios/chrome/DEPS.
Here's the full list of what can be used on iOS from skia.

  # Only parts of skia are compiled on iOS, so we explicitly list the
  # files that can be included to avoid bringing in more code.
  "+skia/ext/skia_utils_ios.h",
  "+third_party/skia/include/core/SkBitmap.h",
  "+third_party/skia/include/core/SkColor.h",
  "+third_party/skia/include/core/SkGraphics.h",

You'll note that this does not include canvas that looks like it is a dependency
of canvas. Your CL currently builds on iOS because none of the application code
links against the canvas bit and thus the linker drop the object file.

I don't know whether placing the same restriction on cc/paint are okay (at least
on iOS). If they are, then I guess the dependency on cc/paint is okay on iOS.

[enne (OOO), 2017-02-01 01:39:33]

enne@chromium.org changed reviewers:
- rohitrao@chromium.org

[enne (OOO), 2017-02-01 01:39:37]

enne@chromium.org changed required reviewers:
- rohitrao@chromium.org

[enne (OOO), 2017-02-01 01:40:49]

Ok, I fixed up the ios issues in https://codereview.chromium.org/2661353002/ so
ios is back to -//cc* as it is now.

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

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

[commit-bot: I haz the power, 2017-02-01 01:41: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-01 04:20:28]

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

[commit-bot: I haz the power, 2017-02-01 04:20:33]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-02-01 12:41:30]

Thank you! lgtm (though I guess my lgtm is no longer necessary as no ios/ file
is changed ^_^).

[enne (OOO), 2017-02-01 19:57:13]

Still waiting on esprehn for a Blink review.

I rebased this to temporarily include
https://codereview.chromium.org/2668353002.

I also reverted all the cc/playback changes based on some feedback from danakj
that it made more sense to leave the rastering as Skia types for now and handle
the future command buffer approach with a separate path.

[esprehn, 2017-02-01 20:25:33]

lgtm, looks all mechanical?

[enne (OOO), 2017-02-01 21:00:19]

On 2017/02/01 at 20:25:33, esprehn wrote:
> lgtm, looks all mechanical?

Yeah, in Blink it is, other than the forwarding headers and including cc/paint
as a public dep for platform.

[enne (OOO), 2017-02-01 21:06:18]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

This patch will be followed up with another patch to change all of
the identifiers that refer to old names.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

[enne (OOO), 2017-02-01 21:25:09]

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

[commit-bot: I haz the power, 2017-02-01 21:25:34]

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

[enne (OOO), 2017-02-01 23:42:55]

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

[commit-bot: I haz the power, 2017-02-01 23:43:31]

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-02 02:08:35]

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

[commit-bot: I haz the power, 2017-02-02 02:08:39]

Dry run: This issue passed the CQ dry run.

[enne (OOO), 2017-02-02 17:03:20]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2017-02-02 17:03:23]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
vmpstr@chromium.org, sky@chromium.org, benwells@chromium.org,
sdefresne@chromium.org, esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2640983002/#ps480001
(title: "DrawingDisplayItem")

[commit-bot: I haz the power, 2017-02-02 17:03:53]

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-02 17:20:03]

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

[commit-bot: I haz the power, 2017-02-02 17:20:09]

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

[enne (OOO), 2017-02-02 22:17:51]

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

[enne (OOO), 2017-02-02 22:17:56]

nick: content/browser OWNERS

[ncarter (slow), 2017-02-02 22:20:11]

On 2017/02/02 22:17:56, enne wrote:
> nick: content/browser OWNERS

[ncarter (slow), 2017-02-02 22:20:14]

On 2017/02/02 22:17:56, enne wrote:
> nick: content/browser OWNERS

content/browser lgtm

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

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2017-02-02 22:21:46]

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-02 22:39:15]

CQ is committing da patch.

Bot data: {"patchset_id": 480001, "attempt_start_ts": 1486074070223170,
"parent_rev": "649a683e91abd33796fdaf787bce9bb7ef33cec4", "commit_rev":
"34f6084cd35016a633942588724e6dd7dae428a3"}

[commit-bot: I haz the power, 2017-02-02 22:40:25]

Description was changed from

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

This patch will be followed up with another patch to change all of
the identifiers that refer to old names.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

to

==========
Rename paint data structures

This patch typedefs a number of skia data structures used in painting in
Blink and ui.

 SkCanvas -> PaintCanvas
 SkSurface -> PaintSurface
 SkPicture -> PaintRecord
 SkPictureRecorder -> PaintRecorder
 SkPaint -> PaintFlags
 SkShader -> PaintShader

These are all typedef'd to Skia types in cc/paint headers.  They are
forwarded to the Blink namespace via platform/graphics/paint headers.
This first step in this patch is to rename uses of these classes
everywhere and then further patches will replace their implementation.

This adds a cc:paint gn component that only depends on Skia that can
then be depended on by ui and Blink platform without depending on the
rest of cc.  This allows ui and Blink platform to continue to
encapsulate its use of cc while still being able to use paint data
structures throughout the entire pipeline.

This patch will be followed up with another patch to change all of
the identifiers that refer to old names.

See:
https://docs.google.com/document/d/135-2dYsilJCPucW7edvuh-eGOmmGvazX4QuF7vIbo...

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

Review-Url: https://codereview.chromium.org/2640983002
Cr-Commit-Position: refs/heads/master@{#447861}
Committed:
https://chromium.googlesource.com/chromium/src/+/34f6084cd35016a633942588724e...
==========

[commit-bot: I haz the power, 2017-02-02 22:40:33]

Committed patchset #25 (id:480001) as
https://chromium.googlesource.com/chromium/src/+/34f6084cd35016a633942588724e...