Implement the basic OS-level printing mechanics on Mac

Implement the basic OS-level printing mechanics on Mac

Part two of printing implementation on the Mac. This adds a Mac implementation of PrintSettings to get page setup and printer information, basic PDF->context rendering in PrintedDocument, and most of PrintingContext to allow getting print settings (both default and interactive).

Reworks the message flow a bit when asking for print settings on the Mac, since it can only be done from the UI thread. Uses a modal dialog for now, but will later be modified further to allow for a sheet.

BUG=13158
TEST=None; no user-visible effect yet.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=28857

[stuartmorgan, 2009-10-12 19:00:46]

I'll be filing post-beta follow-up bugs for these TODOs once basic printing is
landed.

[pink (ping after 24hrs), 2009-10-12 19:21:20]

All the ifdefs make me cry. Any thoughts on how to improve that for later CLs?

http://codereview.chromium.org/268036/diff/1/2
File printing/print_settings.cc (right):

http://codereview.chromium.org/268036/diff/1/2#newcode84
Line 84: UInt32 resolution_count = 0;
comment that this is 1-based?

http://codereview.chromium.org/268036/diff/1/2#newcode86
Line 86: OSStatus status = PMPrinterGetPrinterResolutionCount(printer,
&resolution_count);
80cols

http://codereview.chromium.org/268036/diff/1/2#newcode90
Line 90: ::PMPrinterGetIndexedPrinterResolution(printer, i, &resolution);
we don't really do this sort of scoping in chromium. Mark had me remove it from
some other code.

http://codereview.chromium.org/268036/diff/1/4
File printing/printed_document.cc (right):

http://codereview.chromium.org/268036/diff/1/4#newcode224
Line 224: CFMutableDataRef pdf_data = CFDataCreateMutable(kCFAllocatorDefault,
who takes ownership of pdf_data? does it need a release?

http://codereview.chromium.org/268036/diff/1/4#newcode246
Line 246: #else  // OS_WIN
this comment isn't quite correct anymore.

http://codereview.chromium.org/268036/diff/1/9
File printing/printing_context_mac.mm (right):

http://codereview.chromium.org/268036/diff/1/9#newcode30
Line 30: PrintingContext::Result PrintingContext::AskUserForSettings(
do you want to add any DCHECKs to ensure we're on the main thread?

[Amanda Walker, 2009-10-12 20:14:20]

Overall, LGTM with pink's suggestions.  When I've refactored stuff from using
HDC to using gfx::NativeDrawingContext, I've tried to change member names from
things like "hdc_" to "context_" and the like, just to keep them meaningful for
readers who don't know windows types.  Not a blocker for this checkin, but nice
to do if possible.

[TVL, 2009-10-12 20:19:55]

http://codereview.chromium.org/268036/diff/1/2
File printing/print_settings.cc (right):

http://codereview.chromium.org/268036/diff/1/2#newcode91
Line 91: if (best_resolution.hRes > best_resolution.hRes)
um....

should i make fun of you or pink/awalker?

[stuartmorgan, 2009-10-12 23:15:13]

http://codereview.chromium.org/268036/diff/1/2
File printing/print_settings.cc (right):

http://codereview.chromium.org/268036/diff/1/2#newcode84
Line 84: UInt32 resolution_count = 0;
On 2009/10/12 19:21:21, pink wrote:
> comment that this is 1-based?

Done (at the loop itself, where it's most relevant).

http://codereview.chromium.org/268036/diff/1/2#newcode86
Line 86: OSStatus status = PMPrinterGetPrinterResolutionCount(printer,
&resolution_count);
On 2009/10/12 19:21:21, pink wrote:
> 80cols

Done.

http://codereview.chromium.org/268036/diff/1/2#newcode90
Line 90: ::PMPrinterGetIndexedPrinterResolution(printer, i, &resolution);
On 2009/10/12 19:21:21, pink wrote:
> we don't really do this sort of scoping in chromium. Mark had me remove it
from
> some other code.

Done.

http://codereview.chromium.org/268036/diff/1/2#newcode91
Line 91: if (best_resolution.hRes > best_resolution.hRes)
On 2009/10/12 20:19:55, TVL wrote:
> should i make fun of you or pink/awalker?

Definitely me. Fixed.

http://codereview.chromium.org/268036/diff/1/4
File printing/printed_document.cc (right):

http://codereview.chromium.org/268036/diff/1/4#newcode224
Line 224: CFMutableDataRef pdf_data = CFDataCreateMutable(kCFAllocatorDefault,
On 2009/10/12 19:21:21, pink wrote:
> who takes ownership of pdf_data? does it need a release?

Good catch; that should have been a scoped_cftyperef. Fixed.

http://codereview.chromium.org/268036/diff/1/4#newcode246
Line 246: #else  // OS_WIN
On 2009/10/12 19:21:21, pink wrote:
> this comment isn't quite correct anymore.

Yeah, I wasn't happy as I did this, but it was the closest to file style I could
come up with. Since there's a lot of Windows code above, and a couple of other
places that will need platform branching, I decided to just split the file.

http://codereview.chromium.org/268036/diff/1/9
File printing/printing_context_mac.mm (right):

http://codereview.chromium.org/268036/diff/1/9#newcode30
Line 30: PrintingContext::Result PrintingContext::AskUserForSettings(
On 2009/10/12 19:21:21, pink wrote:
> do you want to add any DCHECKs to ensure we're on the main thread?

Done; the only reason I didn't before is that the usual checks require things
from chrome/--but since this is a .mm file I can use NSThread.

[Amanda Walker, 2009-10-13 16:45:24]

LGTM

[Mark Mentovai, 2009-10-13 21:13:24]

http://codereview.chromium.org/268036/diff/5002/6006
File printing/printed_document_mac.cc (right):

http://codereview.chromium.org/268036/diff/5002/6006#newcode48
Line 48: CGContextDrawPDFDocument(context, target_rect, pdf_doc, page_number);
Deprecated.  The replacement is CGContextDrawPDFPage.

http://developer.apple.com/mac/library/DOCUMENTATION/GraphicsImaging/Referenc...

This causes errors with the 10.6 SDK:

.../src/printing/printed_document_mac.cc: In member function 'void
printing::PrintedDocument::RenderPrintedPage(const printing::PrintedPage&,
CGContext*) const':
.../src/printing/printed_document_mac.cc:48: warning: 'CGContextDrawPDFDocument'
is deprecated (declared at
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/CoreGraphics.framework/Headers/CGContext.h:864)