Match paper by both PPD name and size.

printing/printing_context_mac.mm

Index: printing/printing_context_mac.mm
diff --git a/printing/printing_context_mac.mm b/printing/printing_context_mac.mm
index 36971c87e249d74f4b25224081b2208b87329d15..bcde2c66f38b29be25562cd825aca47bcc4ac760 100644
--- a/printing/printing_context_mac.mm
+++ b/printing/printing_context_mac.mm
@@ -7,6 +7,9 @@
 #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"
@@ -17,6 +20,32 @@
 
 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));
@@ -227,12 +256,16 @@ bool PrintingContextMac::UpdatePageFormatWithPaperInfo() {
   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)
@@ -245,22 +278,26 @@ bool PrintingContextMac::UpdatePageFormatWithPaperInfo() {
   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) {
-      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) {
+    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;
+      best_match = new_score;
     }
   }
 
@@ -284,6 +321,7 @@ bool PrintingContextMac::UpdatePageFormatWithPaperInfo() {
     [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;