PdfMetafileSkia: start supporting more document types.

PdfMetafileSkia: start supporting more document types.

BUG=599185
Committed: https://crrev.com/ffa005bbe6d3d1ccaff4fab1e12d2e2c1eaa66ea
Cr-Commit-Position: refs/heads/master@{#399936}

[hal.canary, 2016-06-13 20:21:52]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-13 20:22:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/1

[hal.canary, 2016-06-13 20:36:48]

halcanary@google.com changed reviewers:
+ thestig@chromium.org

[hal.canary, 2016-06-13 20:36:49]

FYI

I don't think we should land this yet, since I need to call this new code, too.

[Lei Zhang, 2016-06-13 21:01:34]

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia.cc
File printing/pdf_metafile_skia.cc (right):

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:13: #include
"third_party/skia/src/utils/SkMultiPictureDocument.h"
The printing/DEPS rule just says "+third_party/skia" but is there any issues
with including skia headers not in third_party/skia/include?

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:96: printing::SkiaDocumentType type_;
Generally no need for $foo:: in namespace $foo.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:189: default:
No need. I believe if you add a new enum value and don't handle it, the compiler
will warn you... and Werror is on.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:41: PdfMetafileSkia(SkiaDocumentType type =
PDF_SKIA_DOCUMENT_TYPE);
explicit

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:41: PdfMetafileSkia(SkiaDocumentType type =
PDF_SKIA_DOCUMENT_TYPE);
Highly prefer no default arguments, especially with so few callers.

[commit-bot: I haz the power, 2016-06-13 21:35:29]

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

[commit-bot: I haz the power, 2016-06-13 21:35:30]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-06-14 16:34:49]

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia.cc
File printing/pdf_metafile_skia.cc (right):

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:13: #include
"third_party/skia/src/utils/SkMultiPictureDocument.h"
On 2016/06/13 21:01:34, Lei Zhang wrote:
> The printing/DEPS rule just says "+third_party/skia" but is there any issues
> with including skia headers not in third_party/skia/include?

Acknowledged.  I have noted the situation.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:96: printing::SkiaDocumentType type_;
On 2016/06/13 21:01:34, Lei Zhang wrote:
> Generally no need for $foo:: in namespace $foo.

Done.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.cc:189: default:
On 2016/06/13 21:01:34, Lei Zhang wrote:
> No need. I believe if you add a new enum value and don't handle it, the
compiler
> will warn you... and Werror is on.

Done.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia.h
File printing/pdf_metafile_skia.h (right):

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:41: PdfMetafileSkia(SkiaDocumentType type =
PDF_SKIA_DOCUMENT_TYPE);
On 2016/06/13 21:01:34, Lei Zhang wrote:
> Highly prefer no default arguments, especially with so few callers.

Done.

https://codereview.chromium.org/2064843002/diff/1/printing/pdf_metafile_skia....
printing/pdf_metafile_skia.h:41: PdfMetafileSkia(SkiaDocumentType type =
PDF_SKIA_DOCUMENT_TYPE);
On 2016/06/13 21:01:34, Lei Zhang wrote:
> explicit

Done.

[Lei Zhang, 2016-06-14 18:02:19]

lgtm with a rebase

https://codereview.chromium.org/2064843002/diff/20001/printing/image.cc
File printing/image.cc (right):

https://codereview.chromium.org/2064843002/diff/20001/printing/image.cc#newco...
printing/image.cc:153: bool Image::LoadMetafile(const std::string& data) {
Looks like this is gone now with r399738 landed.

[hal.canary, 2016-06-14 18:17:09]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-14 18:17:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/40001

[hal.canary, 2016-06-14 18:31:36]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2016-06-14 18:43:31]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[hal.canary, 2016-06-14 18:43:44]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-06-14 18:43:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/60001

[commit-bot: I haz the power, 2016-06-14 18:55:57]

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

[commit-bot: I haz the power, 2016-06-14 18:55:58]

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

[hal.canary, 2016-06-14 19:34:29]

Patchset #3 (id:60001) has been deleted

[hal.canary, 2016-06-14 19:34:46]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-14 19:35:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/80001

[hal.canary, 2016-06-14 19:43:07]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2016-06-15 14:37:29]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-15 14:37:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/80001

[hal.canary, 2016-06-15 14:43:54]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2016-06-15 14:44:12]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-15 14:44:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/100001

[hal.canary, 2016-06-15 15:04:41]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2016-06-15 15:24:58]

Patchset #3 (id:80001) has been deleted

[hal.canary, 2016-06-15 17:07:32]

The CQ bit was checked by halcanary@google.com

[hal.canary, 2016-06-15 17:07:33]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2064843002/#ps100001
(title: "2016-06-15 (Wednesday) 10:43:48 EDT")

[commit-bot: I haz the power, 2016-06-15 17:08:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064843002/100001

[commit-bot: I haz the power, 2016-06-15 17:13:37]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-06-15 17:16:00]

Description was changed from

==========
PdfMetafileSkia: start supporting more document types.

BUG=599185
==========

to

==========
PdfMetafileSkia: start supporting more document types.

BUG=599185

Committed: https://crrev.com/ffa005bbe6d3d1ccaff4fab1e12d2e2c1eaa66ea
Cr-Commit-Position: refs/heads/master@{#399936}
==========

[commit-bot: I haz the power, 2016-06-15 17:16:01]

Patchset 3 (id:??) landed as
https://crrev.com/ffa005bbe6d3d1ccaff4fab1e12d2e2c1eaa66ea
Cr-Commit-Position: refs/heads/master@{#399936}