From 9da6bcc63386b4fc77bfc83d57f2c7b795aff303 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Wed, 29 Jan 2020 08:29:00 -0800
Subject: [PATCH] address review feedback

---
 shell/browser/api/atom_api_web_contents.cc | 14 ++++++++------
 spec-main/api-web-contents-spec.ts         |  1 -
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/shell/browser/api/atom_api_web_contents.cc b/shell/browser/api/atom_api_web_contents.cc
index 58b2e2f2d9288..f079a2f964853 100644
--- a/shell/browser/api/atom_api_web_contents.cc
+++ b/shell/browser/api/atom_api_web_contents.cc
@@ -354,14 +354,17 @@ base::Optional<base::TimeDelta> GetCursorBlinkInterval() {
 // This will return false if no printer with the provided device_name can be
 // found on the network. We need to check this because Chromium does not do
 // sanity checking of device_name validity and so will crash on invalid names.
-bool DeviceNameValid(const std::string& device_name) {
+bool IsDeviceNameValid(const base::string16& device_name) {
 #if defined(OS_MACOSX)
   base::ScopedCFTypeRef<CFStringRef> new_printer_id(
-      base::SysUTF8ToCFStringRef(device_name));
-  return PMPrinterCreateFromPrinterID(new_printer_id.get());
+      base::SysUTF16ToCFStringRef(device_name));
+  PMPrinter new_printer = PMPrinterCreateFromPrinterID(new_printer_id.get());
+  bool printer_exists = new_printer != nullptr;
+  PMRelease(new_printer);
+  return printer_exists;
 #elif defined(OS_WIN)
   printing::ScopedPrinterHandle printer;
-  return printer.OpenPrinterWithName(base::UTF8ToUTF16(device_name).c_str());
+  return printer.OpenPrinterWithName(device_name.c_str());
 #endif
   return true;
 }
@@ -1804,8 +1807,7 @@ void WebContents::Print(gin_helper::Arguments* args) {
   // Printer device name as opened by the OS.
   base::string16 device_name;
   options.Get("deviceName", &device_name);
-  if (!device_name.empty() &&
-      !DeviceNameValid(base::UTF16ToUTF8(device_name))) {
+  if (!device_name.empty() && !IsDeviceNameValid(device_name)) {
     args->ThrowError("webContents.print(): Invalid deviceName provided.");
     return;
   }
diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts
index e1347d99f5f48..b619653b41b03 100644
--- a/spec-main/api-web-contents-spec.ts
+++ b/spec-main/api-web-contents-spec.ts
@@ -128,7 +128,6 @@ describe('webContents module', () => {
     })
 
     it('does not crash', () => {
-      const w = new BrowserWindow({ show: false })
       expect(() => {
         w.webContents.print({ silent: true })
       }).to.not.throw()