Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2117713002: Print directly to CUPS using the IPP APIs (Closed)

Created:
2 years, 4 months ago by skau
Modified:
2 years, 3 months ago
Reviewers:
Lei Zhang, briannorris
CC:
chromium-reviews, adlr
Base URL:
https://chromium.googlesource.com/chromium/src.git@ippRead
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Print directly to CUPS using the IPP APIs Implements printing to CUPS with paper size, color, and duplexing configurable. Existing bug where the printer does not initialize the first time it is selected. BUG=607668 TEST=Enable --enable-native-cups. Print a webpage to a desired CUPS printer. Committed: https://crrev.com/b793195a8a91fa9a17eaf4af0fa21fed4da4d9cc Cr-Commit-Position: refs/heads/master@{#408179}

Patch Set 1 #

Patch Set 2 : It prints now #

Patch Set 3 : Loggin cleaned up #

Total comments: 10

Patch Set 4 : Address review comments. #

Patch Set 5 : Update patch #

Total comments: 17

Patch Set 6 : Fixed up #

Patch Set 7 : some checks #

Patch Set 8 : Only extend metafile for Chrome OS #

Patch Set 9 : Fix document build #

Patch Set 10 : Skip compiling the chromeos context if we don't compile against cups #

Patch Set 11 : Skip compiling the chromeos context if we don't compile against cups #

Total comments: 20

Patch Set 12 : lint #

Total comments: 16

Patch Set 13 : address comments #

Patch Set 14 : format and merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+527 lines, -19 lines) Patch
M chrome/browser/printing/pdf_to_emf_converter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +7 lines, -0 lines 0 comments Download
M printing/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +4 lines, -1 line 0 comments Download
M printing/backend/cups_deleters.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -0 lines 0 comments Download
M printing/backend/cups_deleters.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -0 lines 0 comments Download
M printing/backend/cups_ipp_util.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +9 lines, -0 lines 0 comments Download
M printing/backend/cups_ipp_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -2 lines 0 comments Download
M printing/metafile.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -2 lines 0 comments Download
M printing/printed_document.cc View 1 2 3 4 5 6 1 chunk +3 lines, -2 lines 0 comments Download
A printing/printed_document_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +47 lines, -0 lines 0 comments Download
M printing/printed_document_linux.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M printing/printing.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +4 lines, -0 lines 0 comments Download
A + printing/printing_context_chromeos.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +23 lines, -8 lines 0 comments Download
A printing/printing_context_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +401 lines, -0 lines 0 comments Download
M printing/printing_context_no_system_dialog.cc View 1 2 3 4 5 3 chunks +6 lines, -2 lines 0 comments Download

Messages

Total messages: 37 (24 generated)
skau
The last CL to implement printing to CUPS on Chrome OS. Please review at your ...
2 years, 4 months ago (2016-07-08 00:53:53 UTC) #2
Lei Zhang
https://codereview.chromium.org/2117713002/diff/40001/printing/metafile.h File printing/metafile.h (right): https://codereview.chromium.org/2117713002/diff/40001/printing/metafile.h#newcode95 printing/metafile.h:95: virtual bool GetDataAsVector(std::vector<char>* buffer) const = 0; Put this ...
2 years, 4 months ago (2016-07-08 01:35:26 UTC) #3
skau
I've responded to the comments and fixed most of them. I've left a comment regarding ...
2 years, 4 months ago (2016-07-09 00:28:33 UTC) #4
Lei Zhang
Still got red trybots. https://codereview.chromium.org/2117713002/diff/40001/printing/printed_document.cc File printing/printed_document.cc (right): https://codereview.chromium.org/2117713002/diff/40001/printing/printed_document.cc#newcode267 printing/printed_document.cc:267: // This function is not ...
2 years, 4 months ago (2016-07-13 01:43:06 UTC) #5
skau
Thanks for the review. I'm pretty sure I fixed all the trybots, I'll keep an ...
2 years, 4 months ago (2016-07-14 23:47:13 UTC) #15
Lei Zhang
https://codereview.chromium.org/2117713002/diff/200001/printing/metafile.h File printing/metafile.h (right): https://codereview.chromium.org/2117713002/diff/200001/printing/metafile.h#newcode167 printing/metafile.h:167: bool GetDataAsVector(std::vector<char>* buffer) const; This feels a bit weird. ...
2 years, 4 months ago (2016-07-20 01:26:57 UTC) #24
skau
I addressed the comments and unified the constants. PTAL. https://codereview.chromium.org/2117713002/diff/200001/printing/metafile.h File printing/metafile.h (right): https://codereview.chromium.org/2117713002/diff/200001/printing/metafile.h#newcode167 printing/metafile.h:167: ...
2 years, 3 months ago (2016-07-22 23:36:14 UTC) #25
Lei Zhang
https://codereview.chromium.org/2117713002/diff/200001/printing/printing_context_chromeos.cc File printing/printing_context_chromeos.cc (right): https://codereview.chromium.org/2117713002/diff/200001/printing/printing_context_chromeos.cc#newcode81 printing/printing_context_chromeos.cc:81: value.copy(*dst, value_len); On 2016/07/22 23:36:14, skau wrote: > On ...
2 years, 3 months ago (2016-07-25 21:06:04 UTC) #26
skau
Comments addressed. PTAL. https://codereview.chromium.org/2117713002/diff/200001/printing/printing_context_chromeos.cc File printing/printing_context_chromeos.cc (right): https://codereview.chromium.org/2117713002/diff/200001/printing/printing_context_chromeos.cc#newcode81 printing/printing_context_chromeos.cc:81: value.copy(*dst, value_len); On 2016/07/25 21:06:03, Lei ...
2 years, 3 months ago (2016-07-27 00:16:47 UTC) #29
Lei Zhang
lgtm
2 years, 3 months ago (2016-07-27 02:02:29 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2117713002/260001
2 years, 3 months ago (2016-07-27 17:57:45 UTC) #34
commit-bot: I haz the power
Committed patchset #14 (id:260001)
2 years, 3 months ago (2016-07-27 18:05:07 UTC) #35
commit-bot: I haz the power
2 years, 3 months ago (2016-07-27 18:07:16 UTC) #37
Message was sent while issue was closed.
Patchset 14 (id:??) landed as
https://crrev.com/b793195a8a91fa9a17eaf4af0fa21fed4da4d9cc
Cr-Commit-Position: refs/heads/master@{#408179}

Powered by Google App Engine
This is Rietveld 408576698