Browse Source

chore: default to sys printer and prevent bad default crash (#22012)

Shelley Vohr 5 years ago
parent
commit
f0d872947d

+ 1 - 1
docs/api/web-contents.md

@@ -1244,7 +1244,7 @@ Returns [`PrinterInfo[]`](structures/printer-info.md)
   * `silent` Boolean (optional) - Don't ask user for print settings. Default is `false`.
   * `printBackground` Boolean (optional) - Prints the background color and image of
     the web page. Default is `false`.
-  * `deviceName` String (optional) - Set the printer device name to use. Default is `''`.
+  * `deviceName` String (optional) - Set the printer device name to use. Must be the system-defined name and not the 'friendly' name, e.g 'Brother_QL_820NWB' and not 'Brother QL-820NWB'.
   * `color` Boolean (optional) - Set whether the printed web page will be in color or grayscale. Default is `true`.
   * `margins` Object (optional)
     * `marginType` String (optional) - Can be `default`, `none`, `printableArea`, or `custom`. If `custom` is chosen, you will also need to specify `top`, `bottom`, `left`, and `right`.

+ 0 - 18
patches/chromium/printing.patch

@@ -600,24 +600,6 @@ index 71c0c15217b62cd7a6087c6d9ae50481f9041d5f..18d853d7f808aaf816de86e8c5b82317
  
  #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
    // Set options for print preset from source PDF document.
-diff --git a/printing/print_settings_conversion.cc b/printing/print_settings_conversion.cc
-index 17c363ff9aa2e2262cacd0c9baea3820334bf67b..5b02461c2e9afe254405ddacd904e4bdbddd0b8b 100644
---- a/printing/print_settings_conversion.cc
-+++ b/printing/print_settings_conversion.cc
-@@ -184,11 +184,12 @@ bool PrintSettingsFromJobSettings(const base::Value& job_settings,
- 
-   settings->set_dpi_xy(dpi_horizontal.value(), dpi_vertical.value());
- #endif
-+  if (!device_name->empty())
-+    settings->set_device_name(base::UTF8ToUTF16(*device_name));
- 
-   settings->set_collate(collate.value());
-   settings->set_copies(copies.value());
-   settings->SetOrientation(landscape.value());
--  settings->set_device_name(base::UTF8ToUTF16(*device_name));
-   settings->set_duplex_mode(static_cast<DuplexMode>(duplex_mode.value()));
-   settings->set_color(static_cast<ColorModel>(color.value()));
-   settings->set_scale_factor(static_cast<double>(scale_factor.value()) / 100.0);
 diff --git a/printing/printing_context.cc b/printing/printing_context.cc
 index cd5c27c87df175676504a06b4e1904f6b836dc90..c4f6acf66bc69f1e7db633aa5b3b03a913ffb666 100644
 --- a/printing/printing_context.cc

+ 67 - 18
shell/browser/api/atom_api_web_contents.cc

@@ -113,11 +113,6 @@
 #include "ui/gfx/font_render_params.h"
 #endif
 
-#if BUILDFLAG(ENABLE_PRINTING)
-#include "chrome/browser/printing/print_view_manager_basic.h"
-#include "components/printing/common/print_messages.h"
-#endif
-
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
 #include "shell/browser/extensions/atom_extension_web_contents_observer.h"
 #endif
@@ -313,6 +308,35 @@ void OnCapturePageDone(util::Promise promise, const SkBitmap& bitmap) {
   promise.Resolve(gfx::Image::CreateFrom1xBitmap(bitmap));
 }
 
+#if BUILDFLAG(ENABLE_PRINTING)
+// 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 IsDeviceNameValid(const base::string16& device_name) {
+#if defined(OS_MACOSX)
+  base::ScopedCFTypeRef<CFStringRef> new_printer_id(
+      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(device_name.c_str());
+#endif
+  return true;
+}
+
+base::string16 GetDefaultPrinterAsync() {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
+
+  scoped_refptr<printing::PrintBackend> backend =
+      printing::PrintBackend::CreateInstance(nullptr);
+  std::string printer_name = backend->GetDefaultPrinterName();
+  return base::UTF8ToUTF16(printer_name);
+}
+#endif
 }  // namespace
 
 WebContents::WebContents(v8::Isolate* isolate,
@@ -1647,6 +1671,30 @@ bool WebContents::IsCurrentlyAudible() {
 }
 
 #if BUILDFLAG(ENABLE_PRINTING)
+void WebContents::OnGetDefaultPrinter(
+    base::DictionaryValue print_settings,
+    printing::CompletionCallback print_callback,
+    base::string16 device_name,
+    bool silent,
+    base::string16 default_printer) {
+  base::string16 printer_name =
+      device_name.empty() ? default_printer : device_name;
+  print_settings.SetStringKey(printing::kSettingDeviceName, printer_name);
+
+  auto* print_view_manager =
+      printing::PrintViewManagerBasic::FromWebContents(web_contents());
+  auto* focused_frame = web_contents()->GetFocusedFrame();
+  auto* rfh = focused_frame && focused_frame->HasSelection()
+                  ? focused_frame
+                  : web_contents()->GetMainFrame();
+
+  print_view_manager->PrintNow(
+      rfh,
+      std::make_unique<PrintMsg_PrintPages>(rfh->GetRoutingID(), silent,
+                                            std::move(print_settings)),
+      std::move(print_callback));
+}
+
 void WebContents::Print(mate::Arguments* args) {
   mate::Dictionary options = mate::Dictionary::CreateEmpty(args->isolate());
   base::DictionaryValue settings;
@@ -1710,11 +1758,15 @@ void WebContents::Print(mate::Arguments* args) {
   options.Get("landscape", &landscape);
   settings.SetBoolean(printing::kSettingLandscape, landscape);
 
-  // We set the default to empty string here and only update
-  // if at the Chromium level if it's non-empty
+  // We set the default to the system's default printer and only update
+  // if at the Chromium level if the user overrides.
+  // Printer device name as opened by the OS.
   base::string16 device_name;
   options.Get("deviceName", &device_name);
-  settings.SetString(printing::kSettingDeviceName, device_name);
+  if (!device_name.empty() && !IsDeviceNameValid(device_name)) {
+    args->ThrowError("webContents.print(): Invalid deviceName provided.");
+    return;
+  }
 
   int scale_factor = 100;
   options.Get("scaleFactor", &scale_factor);
@@ -1781,16 +1833,13 @@ void WebContents::Print(mate::Arguments* args) {
     settings.SetInteger(printing::kSettingDpiVertical, dpi);
   }
 
-  auto* print_view_manager =
-      printing::PrintViewManagerBasic::FromWebContents(web_contents());
-  auto* focused_frame = web_contents()->GetFocusedFrame();
-  auto* rfh = focused_frame && focused_frame->HasSelection()
-                  ? focused_frame
-                  : web_contents()->GetMainFrame();
-  print_view_manager->PrintNow(rfh,
-                               std::make_unique<PrintMsg_PrintPages>(
-                                   rfh->GetRoutingID(), silent, settings),
-                               std::move(callback));
+  base::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_BLOCKING},
+      base::BindOnce(&GetDefaultPrinterAsync),
+      base::BindOnce(&WebContents::OnGetDefaultPrinter,
+                     weak_factory_.GetWeakPtr(), std::move(settings),
+                     std::move(callback), device_name, silent));
 }
 
 std::vector<printing::PrinterBasicInfo> WebContents::GetPrinterList() {

+ 11 - 0
shell/browser/api/atom_api_web_contents.h

@@ -31,8 +31,14 @@
 #include "ui/gfx/image/image.h"
 
 #if BUILDFLAG(ENABLE_PRINTING)
+#include "chrome/browser/printing/print_view_manager_basic.h"
+#include "components/printing/common/print_messages.h"
 #include "printing/backend/print_backend.h"
 #include "shell/browser/printing/print_preview_message_handler.h"
+
+#if defined(OS_WIN)
+#include "printing/backend/win_helper.h"
+#endif
 #endif
 
 namespace blink {
@@ -181,6 +187,11 @@ class WebContents : public mate::TrackableObject<WebContents>,
   v8::Local<v8::Value> GetNativeView() const;
 
 #if BUILDFLAG(ENABLE_PRINTING)
+  void OnGetDefaultPrinter(base::DictionaryValue print_settings,
+                           printing::CompletionCallback print_callback,
+                           base::string16 device_name,
+                           bool silent,
+                           base::string16 default_printer);
   void Print(mate::Arguments* args);
   std::vector<printing::PrinterBasicInfo> GetPrinterList();
   // Print current page as PDF.

+ 17 - 3
spec-main/api-web-contents-spec.ts

@@ -6,7 +6,7 @@ import * as http from 'http'
 import { BrowserWindow, ipcMain, webContents, session } from 'electron'
 import { emittedOnce } from './events-helpers';
 import { closeAllWindows } from './window-helpers';
-import { ifdescribe } from './spec-helpers';
+import { ifdescribe, ifit } from './spec-helpers';
 
 const { expect } = chai
 
@@ -102,21 +102,35 @@ describe('webContents module', () => {
   })
 
   ifdescribe(features.isPrintingEnabled())('webContents.print()', () => {
+    let w: BrowserWindow
+
+    beforeEach(() => {
+      w = new BrowserWindow({ show: false })
+    })
+
+    afterEach(closeAllWindows)
+
     it('throws when invalid settings are passed', () => {
-      const w = new BrowserWindow({ show: false })
       expect(() => {
         // @ts-ignore this line is intentionally incorrect
         w.webContents.print(true)
       }).to.throw('webContents.print(): Invalid print settings specified.')
+    })
 
+    it('throws when an invalid callback is passed', () => {
       expect(() => {
         // @ts-ignore this line is intentionally incorrect
         w.webContents.print({}, true)
       }).to.throw('webContents.print(): Invalid optional callback provided.')
     })
 
+    ifit(process.platform !== 'linux')('throws when an invalid deviceName is passed', () => {
+      expect(() => {
+        w.webContents.print({ deviceName: 'i-am-a-nonexistent-printer' }, () => {})
+      }).to.throw('webContents.print(): Invalid deviceName provided.')
+    })
+
     it('does not crash', () => {
-      const w = new BrowserWindow({ show: false })
       expect(() => {
         w.webContents.print({ silent: true })
       }).to.not.throw()