Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 974183003: Print Preview: Support the duplex preset in PDFs. (Closed)

Created:
5 years, 9 months ago by Lei Zhang
Modified:
5 years, 9 months ago
Reviewers:
Tom Sepez, raymes, jam
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, yusukes+watch_chromium.org, tzik, binji+watch_chromium.org, jam, raymes+watch_chromium.org, teravest+watch_chromium.org, darin-cc_chromium.org, arv+watch_chromium.org, bradnelson+warch_chromium.org, mkwst+moarreviews-renderer_chromium.org, piman+watch_chromium.org, ihf+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Print Preview: Support the duplex preset in PDFs. The preset from the PDF document gets plumbed all the way to print preview, but the ShortEdge setting is ignored at the end because print preview does not support it yet. BUG=169120 Committed: https://crrev.com/b335e7d621465955cf8b733660f82ffe0aab6de2 Cr-Commit-Position: refs/heads/master@{#319324}

Patch Set 1 #

Patch Set 2 : nits #

Patch Set 3 : nit #

Total comments: 4

Patch Set 4 : address comments #

Total comments: 2

Patch Set 5 : rebase #

Patch Set 6 : tsepez comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+129 lines, -27 lines) Patch
M chrome/browser/resources/print_preview/native_layer.js View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/print_preview/print_preview.js View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/print_preview_ui.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/webui/print_preview.js View 2 chunks +26 lines, -1 line 0 comments Download
M components/printing/renderer/print_web_view_helper.h View 1 chunk +2 lines, -2 lines 0 comments Download
M components/printing/renderer/print_web_view_helper.cc View 1 2 3 4 chunks +32 lines, -7 lines 0 comments Download
M content/renderer/pepper/pepper_plugin_instance_impl.cc View 1 2 3 3 chunks +15 lines, -2 lines 0 comments Download
M pdf/instance.cc View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M pdf/out_of_process_instance.cc View 1 2 3 3 chunks +4 lines, -2 lines 0 comments Download
M pdf/pdf_engine.h View 1 2 4 chunks +12 lines, -11 lines 0 comments Download
M pdf/pdfium/pdfium_assert_matching_enums.cc View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M pdf/pdfium/pdfium_engine.h View 1 chunk +1 line, -0 lines 0 comments Download
M pdf/pdfium/pdfium_engine.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M ppapi/c/private/ppp_pdf.h View 1 2 3 4 5 2 chunks +13 lines, -1 line 0 comments Download
M ppapi/proxy/ppapi_messages.h View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (4 generated)
Lei Zhang
Adding back some of the enums removed in http://crrev.com/311831, but they have been reordered to ...
5 years, 9 months ago (2015-03-04 19:37:35 UTC) #2
raymes
lgtm https://codereview.chromium.org/974183003/diff/40001/components/printing/renderer/print_web_view_helper.cc File components/printing/renderer/print_web_view_helper.cc (right): https://codereview.chromium.org/974183003/diff/40001/components/printing/renderer/print_web_view_helper.cc#newcode53 components/printing/renderer/print_web_view_helper.cc:53: static_assert(static_cast<int>(a) == static_cast<int>(b), \ nit: 4 spaces? https://codereview.chromium.org/974183003/diff/40001/ppapi/api/dev/pp_print_settings_dev.idl ...
5 years, 9 months ago (2015-03-05 03:28:20 UTC) #3
Lei Zhang
https://codereview.chromium.org/974183003/diff/40001/components/printing/renderer/print_web_view_helper.cc File components/printing/renderer/print_web_view_helper.cc (right): https://codereview.chromium.org/974183003/diff/40001/components/printing/renderer/print_web_view_helper.cc#newcode53 components/printing/renderer/print_web_view_helper.cc:53: static_assert(static_cast<int>(a) == static_cast<int>(b), \ On 2015/03/05 03:28:20, raymes wrote: ...
5 years, 9 months ago (2015-03-05 05:52:45 UTC) #4
Lei Zhang
+jam for contents/ +tsepez for ppapi/proxy/ppapi_messages.h
5 years, 9 months ago (2015-03-05 05:54:16 UTC) #6
Tom Sepez
Messages LGTM % Nit. https://codereview.chromium.org/974183003/diff/60001/ppapi/c/private/ppp_pdf.h File ppapi/c/private/ppp_pdf.h (right): https://codereview.chromium.org/974183003/diff/60001/ppapi/c/private/ppp_pdf.h#newcode30 ppapi/c/private/ppp_pdf.h:30: PP_COMPILE_ASSERT_SIZE_IN_BYTES(PP_PrivateDuplexMode_Dev, 4); nit: typically the ...
5 years, 9 months ago (2015-03-05 17:39:07 UTC) #7
jam
lgtm
5 years, 9 months ago (2015-03-05 18:31:13 UTC) #8
Lei Zhang
https://codereview.chromium.org/974183003/diff/60001/ppapi/c/private/ppp_pdf.h File ppapi/c/private/ppp_pdf.h (right): https://codereview.chromium.org/974183003/diff/60001/ppapi/c/private/ppp_pdf.h#newcode30 ppapi/c/private/ppp_pdf.h:30: PP_COMPILE_ASSERT_SIZE_IN_BYTES(PP_PrivateDuplexMode_Dev, 4); On 2015/03/05 17:39:06, Tom Sepez wrote: > ...
5 years, 9 months ago (2015-03-05 19:06:35 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/974183003/100001
5 years, 9 months ago (2015-03-05 19:08:02 UTC) #12
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 9 months ago (2015-03-05 21:06:21 UTC) #13
commit-bot: I haz the power
5 years, 9 months ago (2015-03-05 21:07:27 UTC) #14
Message was sent while issue was closed. 
Patchset 6 (id:??) landed as
https://crrev.com/b335e7d621465955cf8b733660f82ffe0aab6de2
Cr-Commit-Position: refs/heads/master@{#319324}

Powered by Google App Engine
This is Rietveld 408576698