Match paper by both PPD name and size.

printing/printing_context_mac.mm

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "printing/printing_context_mac.h"
 
 #import <ApplicationServices/ApplicationServices.h>
 #import <AppKit/AppKit.h>
 
+#import <iomanip>
+#import <numeric>
+
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/mac/scoped_nsexception_enabler.h"
 #include "base/sys_string_conversions.h"
 #include "base/values.h"
 #include "printing/print_settings_initializer_mac.h"
 
 namespace printing {
 
+namespace {
+
+std::string PaperToString(const PMPaper paper) {

[kmadhusu, 2012/09/08 01:06:16]

Document this function.

[kmadhusu, 2012/09/08 01:06:16]

const PMPaper -> const PMPaper&

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Don't need if remove logging.

On 2012/09/08 01:06:16, kmadhusu wrote:

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Done.

+  CFStringRef name = NULL;
+  if (PMPaperGetPPDPaperName(paper, &name) != noErr)
+    return std::string();
+  std::string result(CFStringGetLength(name) + 1, ' ');

[kmadhusu, 2012/09/08 01:06:16]

Use base::mac::ScopedCFTypeRef<CFStringRef>

+  if (result.empty() || !CFStringGetCString(name, &result[0], result.size(),
+                                            kCFStringEncodingASCII)) {
+    return std::string();
+  }
+  result.resize(result.size() - 1);
+  return result;
+}
+
+bool IsPaperNameEqual(const PMPaper paper1, const PMPaper paper2) {

[kmadhusu, 2012/09/08 01:06:16]

nit: Document this function.

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Done.

+  CFStringRef name1 = NULL;

[kmadhusu, 2012/09/08 01:06:16]

Use base::mac::ScopedCFTypeRef<CFStringRef>

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

I am not experienced with MAC dev, but it seems we don't need Retain/Release
here
On 2012/09/08 01:06:16, kmadhusu wrote:

[kmadhusu, 2012/09/08 02:26:18]

csilv@: Do we need to use base::mac::ScopedCFTypeRef<CFStringRef> here?

[csilv, 2012/09/10 03:27:47]

kmadhusu@, no reason to use ScopedCFTypeRef here.  PMPaperGetPPDPaperName
retains ownership of the returned object.  So there is no reason for the object
to be released and therefore no real benefit of ScopedCFTypeRef.

+  CFStringRef name2 = NULL;
+  return PMPaperGetPPDPaperName(paper1, &name1) == noErr &&

[kmadhusu, 2012/09/08 01:06:16]

nit: Please add parantheses.
return (a == b) && ( e == b) && (c == d);

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Done.

+         PMPaperGetPPDPaperName(paper2, &name2) == noErr &&
+         CFStringCompare(name1, name2,
+                         kCFCompareCaseInsensitive) == kCFCompareEqualTo;
+}
+
+}  // namespace
+
 // static
 PrintingContext* PrintingContext::Create(const std::string& app_locale) {
   return static_cast<PrintingContext*>(new PrintingContextMac(app_locale));
 }
 
 PrintingContextMac::PrintingContextMac(const std::string& app_locale)
     : PrintingContext(app_locale),
       print_info_([[NSPrintInfo sharedPrintInfo] copy]),
       context_(NULL) {
 }
@@ skipping 190 matching lines @@
   PMPrintSession print_session =
       static_cast<PMPrintSession>([print_info_.get() PMPrintSession]);
 
   PMPageFormat default_page_format =
       static_cast<PMPageFormat>([print_info_.get() PMPageFormat]);
 
   PMPaper default_paper;
   if (PMGetPageFormatPaper(default_page_format, &default_paper) != noErr)
     return false;
 
-  double default_page_width, default_page_height;
+  double default_page_width = 0.0;
+  double default_page_height = 0.0;
   if (PMPaperGetWidth(default_paper, &default_page_width) != noErr)
     return false;
 
   if (PMPaperGetHeight(default_paper, &default_page_height) != noErr)
     return false;
+  VLOG(1) << "default_paper: " << PaperToString(default_paper) << " " <<
+      std::setprecision(10) << default_page_width << "x" <<

[kmadhusu, 2012/09/08 01:06:16]

Do you need these VLOG here and at 290 and 324?

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Done.

+      std::setprecision(10) << default_page_height;
 
   PMPrinter current_printer = NULL;
   if (PMSessionGetCurrentPrinter(print_session, &current_printer) != noErr)
     return false;
 
   if (current_printer == nil)
     return false;
 
   CFArrayRef paper_list = NULL;
   if (PMPrinterGetPaperList(current_printer, &paper_list) != noErr)
     return false;
 
+  double best_match = std::numeric_limits<double>::max();
   PMPaper best_matching_paper = kPMNoData;
   int num_papers = CFArrayGetCount(paper_list);
   for (int i = 0; i < num_papers; ++i) {
     PMPaper paper = (PMPaper) [(NSArray* ) paper_list objectAtIndex: i];
-    double paper_width, paper_height;
+    double paper_width = 0.0;
+    double paper_height = 0.0;
     PMPaperGetWidth(paper, &paper_width);
     PMPaperGetHeight(paper, &paper_height);
-    if (default_page_width == paper_width &&
-        default_page_height == paper_height) {
+    VLOG(1) << "paper_" << i << ": " << PaperToString(paper) << " " <<
+        std::setprecision(10) << paper_width << "x" <<
+        std::setprecision(10) << paper_height;
+    double new_score = std::max(fabs(default_page_width - paper_width),

[kmadhusu, 2012/09/08 01:06:16]

nit: Can you rename new_score to current_match or something similar?

[kmadhusu, 2012/09/08 01:06:16]

Please re-add the comments that explain the paper order priority.

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

Done.

[Vitaly Buka (NO REVIEWS), 2012/09/08 01:54:19]

It's not relevant here. I removed the check for 0.0001
Now it's basically your function with "+= IsPaperNameEqual()"

On 2012/09/08 01:06:16, kmadhusu wrote:

+                                fabs(default_page_height - paper_height));
+    if (new_score > 2)
+      continue;
+    new_score += IsPaperNameEqual(paper, default_paper) ? 0 : 1;
+    if (new_score < best_match) {
       best_matching_paper = paper;
-      break;
-    }
-    // Trying to find the best matching paper.
-    if (fabs(default_page_width - paper_width) < 2 &&
-        fabs(default_page_height - paper_height) < 2) {
-      best_matching_paper = paper;
+      best_match = new_score;
     }
   }
 
   if (best_matching_paper == kPMNoData) {
     PMPaper paper = kPMNoData;
     // Create a custom paper for the specified default page size.
     PMPaperMargins default_margins;
     if (PMPaperGetMargins(default_paper, &default_margins) != noErr)
       return false;
 
     const PMPaperMargins margins =
         {default_margins.top, default_margins.left, default_margins.bottom,
          default_margins.right};
     CFStringRef paper_id = CFSTR("Custom paper ID");
     CFStringRef paper_name = CFSTR("Custom paper");
     if (PMPaperCreateCustom(current_printer, paper_id, paper_name,
             default_page_width, default_page_height, &margins, &paper) !=
             noErr) {
       return false;
     }
     [print_info_.get() updateFromPMPageFormat];
     PMRelease(paper);
   } else {
+    VLOG(1) << "best_paper_name: " << PaperToString(best_matching_paper);
     PMPageFormat chosen_page_format = NULL;
     if (PMCreatePageFormat((PMPageFormat*) &chosen_page_format) != noErr)
       return false;
 
     // Create page format from that paper.
     if (PMCreatePageFormatWithPMPaper(&chosen_page_format,
             best_matching_paper) != noErr) {
       PMRelease(chosen_page_format);
       return false;
     }
@@ skipping 193 matching lines @@
 void PrintingContextMac::ReleaseContext() {
   print_info_.reset();
   context_ = NULL;
 }
 
 gfx::NativeDrawingContext PrintingContextMac::context() const {
   return context_;
 }
 
 }  // namespace printing