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

fix: default printer if none is provided #21956

Merged
merged 2 commits into from Jan 31, 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
21 changes: 0 additions & 21 deletions patches/chromium/printing.patch
Expand Up @@ -616,27 +616,6 @@ index 51cfabd4dbeaa63538f4483c7a1948afaa665de1..0850b96b7511c494692a24026734d757

#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 e01d195261a4c993dddb77d7a355481bf02067cc..2e533a6981c86387f892f8a3aaed99478f6079ef 100644
--- a/printing/print_settings_conversion.cc
+++ b/printing/print_settings_conversion.cc
@@ -186,12 +186,14 @@ bool PrintSettingsFromJobSettings(const base::Value& job_settings,

settings->set_dpi_xy(dpi_horizontal.value(), dpi_vertical.value());
#endif
+ const std::string* device_name =
+ job_settings.FindStringKey(kSettingDeviceName);
+ 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(*job_settings.FindStringKey(kSettingDeviceName)));
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 73940192472b1576a701cad3abbb92f2d72aa77e..bc0d39ccd113306691ae532e9fbc5b64c9aa0a33 100644
--- a/printing/printing_context.cc
Expand Down
61 changes: 41 additions & 20 deletions shell/browser/api/atom_api_web_contents.cc
Expand Up @@ -113,15 +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"

#if defined(OS_WIN)
#include "printing/backend/win_helper.h"
#endif
#endif

#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
#include "shell/browser/extensions/atom_extension_web_contents_observer.h"
#endif
Expand Down Expand Up @@ -368,6 +359,17 @@ bool IsDeviceNameValid(const base::string16& device_name) {
#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, g_browser_process->GetApplicationLocale());
std::string printer_name = backend->GetDefaultPrinterName();
return base::UTF8ToUTF16(printer_name);
}
#endif

} // namespace
Expand Down Expand Up @@ -1737,6 +1739,27 @@ bool WebContents::IsCurrentlyAudible() {
}

#if BUILDFLAG(ENABLE_PRINTING)
void WebContents::OnGetDefaultPrinter(
base::Value 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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing discussion about #21986 (comment) , the crash is caused by this line.

This function is registered as a weak callback to the task runner below, so that its not invoked when the api::WebContents is not available. Based on the crash it seems content::WebContents is not available in the callback, @zcbenz is it possible for api::WebContents to be alive while the content::WebContents is not ?

auto* focused_frame = web_contents()->GetFocusedFrame();
auto* rfh = focused_frame && focused_frame->HasSelection()
? focused_frame
: web_contents()->GetMainFrame();

print_view_manager->PrintNow(rfh, silent, std::move(print_settings),
std::move(print_callback));
}

void WebContents::Print(gin_helper::Arguments* args) {
gin_helper::Dictionary options =
gin::Dictionary::CreateEmpty(args->isolate());
Expand Down Expand Up @@ -1801,16 +1824,15 @@ void WebContents::Print(gin_helper::Arguments* args) {
options.Get("landscape", &landscape);
settings.SetBoolKey(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);
if (!device_name.empty() && !IsDeviceNameValid(device_name)) {
args->ThrowError("webContents.print(): Invalid deviceName provided.");
return;
}
settings.SetStringKey(printing::kSettingDeviceName, device_name);

int scale_factor = 100;
options.Get("scaleFactor", &scale_factor);
Expand Down Expand Up @@ -1890,14 +1912,13 @@ void WebContents::Print(gin_helper::Arguments* args) {
settings.SetIntKey(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, silent, std::move(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 @@ -32,8 +32,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

#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
Expand Down Expand Up @@ -190,6 +196,11 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
bool IsBeingCaptured();

#if BUILDFLAG(ENABLE_PRINTING)
void OnGetDefaultPrinter(base::Value print_settings,
printing::CompletionCallback print_callback,
base::string16 device_name,
bool silent,
base::string16 default_printer);
void Print(gin_helper::Arguments* args);
std::vector<printing::PrinterBasicInfo> GetPrinterList();
// Print current page as PDF.
Expand Down