Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api/web-contents.md
Expand Up @@ -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`.
Expand Down
18 changes: 0 additions & 18 deletions patches/chromium/printing.patch
Expand Up @@ -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
Expand Down
85 changes: 67 additions & 18 deletions shell/browser/api/atom_api_web_contents.cc
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down
11 changes: 11 additions & 0 deletions shell/browser/api/atom_api_web_contents.h
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 17 additions & 3 deletions spec-main/api-web-contents-spec.ts
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down