Avoid crash in DocumentProperties

printing/backend/win_helper.cc

Index: printing/backend/win_helper.cc
diff --git a/printing/backend/win_helper.cc b/printing/backend/win_helper.cc
index 3527aeece5a455f0f63e92816532021bd4609d0f..af917a0528d3c82bfdcc536729c70cbdaaf1a332 100644
--- a/printing/backend/win_helper.cc
+++ b/printing/backend/win_helper.cc
@@ -471,12 +471,7 @@ std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevModeWithColor(
   return ticket;
 }
 
-bool IsPrinterRPCSOnly(HANDLE printer) {
-  PrinterInfo5 info_5;
-  if (!info_5.Init(printer))
-    return false;
-  const wchar_t* name = info_5.get()->pPrinterName;
-  const wchar_t* port = info_5.get()->pPortName;
+bool IsPrinterRPCSOnly(const wchar_t* name, const wchar_t* port) {
   int num_languages =
       DeviceCapabilities(name, port, DC_PERSONALITY, NULL, NULL);
   if (num_languages != 1)
@@ -487,6 +482,10 @@ bool IsPrinterRPCSOnly(HANDLE printer) {
   return wcscmp(buf.data(), kRPCSLanguage) == 0;
 }
 
+bool PrinterHasValidPaperSize(const wchar_t* name, const wchar_t* port) {
+  return DeviceCapabilities(name, port, DC_PAPERSIZE, NULL, NULL) > 0;

[Lei Zhang, 2017/06/02 21:29:54]

nit: Can we write nullptr in new code?

[rbpotter, 2017/06/03 01:23:24]

Done.

+}
+
 std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
                                                           DEVMODE* in) {
   LONG buffer_size = DocumentProperties(
@@ -502,10 +501,19 @@ std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
       reinterpret_cast<DEVMODE*>(calloc(buffer_size, 1)));
   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER;
 
+  PrinterInfo5 info_5;
+  if (!info_5.Init(printer))
+    return std::unique_ptr<DEVMODE, base::FreeDeleter>();

[Lei Zhang, 2017/06/02 21:29:54]

I think you can just return nullptr. If so, maybe convert all the returns below
too?

[rbpotter, 2017/06/03 01:23:24]

Done.

+  const wchar_t* name = info_5.get()->pPrinterName;
+  const wchar_t* port = info_5.get()->pPortName;
+
   // Check for RPCS drivers on Windows 8+ as DocumentProperties will crash if
-  // called on one of these printers. See crbug.com/679160
-  if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
-      IsPrinterRPCSOnly(printer)) {
+  // called on one of these printers. Also check that valid paper sizes exist;
+  // some old drivers return no paper sizes and crash in DocumentProperties if
+  // used with Win10. See crbug.com/679160

[Lei Zhang, 2017/06/02 21:29:54]

Append "Also ..." to the existing comment and reference the new bug, or just do
these as two separate checks.

[rbpotter, 2017/06/03 01:23:24]

Done.

+  if ((base::win::GetVersion() >= base::win::VERSION_WIN8 &&
+       IsPrinterRPCSOnly(name, port)) ||
+      !PrinterHasValidPaperSize(name, port)) {
     return std::unique_ptr<DEVMODE, base::FreeDeleter>();
   }