Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1842473002:
Use SkDocument_PDF_None if pdf support is not enabled. (Closed)
|
Created:
4 years, 8 months ago by mbjorge Modified:
4 years, 8 months ago Reviewers:
vandebo (ex-Chrome), robertphillips, slan, robertphillips1, hal.canary, hal canary at chromium CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse SkDocument_PDF_None if pdf support is not enabled.
https://codereview.chromium.org/1833193002 added the //printing
dependency to //content/test:content_unittests, but when doing a Debug
build, linking content_unittests fails because it tries to reference the
SkDocument::CreatePDF function, which doesn't exists on platforms that
don't enable printing.
BUG= internal b/27859637
TEST= build content_unittests
Committed: https://crrev.com/4d5dc283f68c630748a82db1d3478902f68bb503
Cr-Commit-Position: refs/heads/master@{#383789}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address nits #Patch Set 3 : Use SkDoucment_PDF_None #Messages
Total messages: 33 (13 generated)
mbjorge@chromium.org changed reviewers: + slan@chromium.org
No [Chromecast] tag in the Commit Message, as this is just a general change. https://codereview.chromium.org/1842473002/diff/1/printing/pdf_metafile_skia.cc File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/1842473002/diff/1/printing/pdf_metafile_skia.... printing/pdf_metafile_skia.cc:68: #if defined(ENABLE_PRINTING) && ENABLE_PRINTING Just: #if defined(ENABLE_PRINTING) here and below.
Description was changed from ========== [Chromecast] Wrap skia pdf function with ENABLE_PRINTING https://codereview.chromium.org/1833193002 added the //printing dependency to //content/test:content_unittests, but when doing a Debug build, linking content_unittests fails because it tries to reference the SkDocument::CreatePDF function, which doesn't exists on platforms that don't enable printing. BUG= internal b/27859637 TEST= build content_unittests ========== to ========== Wrap skia pdf function with ENABLE_PRINTING https://codereview.chromium.org/1833193002 added the //printing dependency to //content/test:content_unittests, but when doing a Debug build, linking content_unittests fails because it tries to reference the SkDocument::CreatePDF function, which doesn't exists on platforms that don't enable printing. BUG= internal b/27859637 TEST= build content_unittests ==========
On 2016/03/28 at 20:03:44, slan wrote: > No [Chromecast] tag in the Commit Message, as this is just a general change. > Done
https://codereview.chromium.org/1842473002/diff/1/printing/pdf_metafile_skia.cc File printing/pdf_metafile_skia.cc (right): https://codereview.chromium.org/1842473002/diff/1/printing/pdf_metafile_skia.... printing/pdf_metafile_skia.cc:68: #if defined(ENABLE_PRINTING) && ENABLE_PRINTING On 2016/03/28 at 20:03:44, slan wrote: > Just: > > #if defined(ENABLE_PRINTING) > > here and below. Done
mbjorge@chromium.org changed reviewers: + halcanary@chromium.org, vandebo@chromium.org
On 2016/03/28 20:29:34, mbjorge wrote: You could compile in third_party/skia/src/pdf/SkDocument_PDF_None.cpp instead.
Description was changed from ========== Wrap skia pdf function with ENABLE_PRINTING https://codereview.chromium.org/1833193002 added the //printing dependency to //content/test:content_unittests, but when doing a Debug build, linking content_unittests fails because it tries to reference the SkDocument::CreatePDF function, which doesn't exists on platforms that don't enable printing. BUG= internal b/27859637 TEST= build content_unittests ========== to ========== Use SkDocument_PDF_None if pdf support is not enabled. https://codereview.chromium.org/1833193002 added the //printing dependency to //content/test:content_unittests, but when doing a Debug build, linking content_unittests fails because it tries to reference the SkDocument::CreatePDF function, which doesn't exists on platforms that don't enable printing. BUG= internal b/27859637 TEST= build content_unittests ==========
On 2016/03/28 at 20:32:04, halcanary wrote: > On 2016/03/28 20:29:34, mbjorge wrote: > > You could compile in third_party/skia/src/pdf/SkDocument_PDF_None.cpp instead. Thanks! I like that much better.
lgtm
The CQ bit was checked by slan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1842473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1842473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
halcanary@google.com changed reviewers: + halcanary@google.com
Lgtm @ latest patch
mbjorge@chromium.org changed reviewers: + robertphillips@chromium.org
+robertphillips for OWNERS
lgtm
The CQ bit was checked by mbjorge@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1842473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1842473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
mbjorge@chromium.org changed reviewers: + robertphillips@google.com - robertphillips@chromium.org
Missing OWNER for skia/BUILD.gn? Oh, is this because the owners file lists robertphillips@google.com, but you were added with robertphillips@chromium.org?
lgtm
The CQ bit was checked by mbjorge@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1842473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1842473002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from
==========
Use SkDocument_PDF_None if pdf support is not enabled.
https://codereview.chromium.org/1833193002 added the //printing
dependency to //content/test:content_unittests, but when doing a Debug
build, linking content_unittests fails because it tries to reference the
SkDocument::CreatePDF function, which doesn't exists on platforms that
don't enable printing.
BUG= internal b/27859637
TEST= build content_unittests
==========
to
==========
Use SkDocument_PDF_None if pdf support is not enabled.
https://codereview.chromium.org/1833193002 added the //printing
dependency to //content/test:content_unittests, but when doing a Debug
build, linking content_unittests fails because it tries to reference the
SkDocument::CreatePDF function, which doesn't exists on platforms that
don't enable printing.
BUG= internal b/27859637
TEST= build content_unittests
Committed: https://crrev.com/4d5dc283f68c630748a82db1d3478902f68bb503
Cr-Commit-Position: refs/heads/master@{#383789}
==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/4d5dc283f68c630748a82db1d3478902f68bb503 Cr-Commit-Position: refs/heads/master@{#383789} |
