Commit 190d38b5 authored by rbpotter's avatar rbpotter Committed by Commit Bot

Fix Print Preview to System dialog bug

Update PrintJob::pdf_page_mapping_ so that PDF pages not in the
user's selection are ignored instead of causing the print job to be
cancelled.

Also remove some comments that are significantly outdated, and
caused confusion while investigating the bug.

Bug: 823876
Change-Id: Id6c1c3acb67c4d460cffa937f7a9b87eda402a88
Reviewed-on: https://chromium-review.googlesource.com/974042Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545173}
parent 38b5c184
...@@ -85,6 +85,23 @@ void PrintJob::Initialize(PrintJobWorkerOwner* job, ...@@ -85,6 +85,23 @@ void PrintJob::Initialize(PrintJobWorkerOwner* job,
content::Source<PrintJob>(this)); content::Source<PrintJob>(this));
} }
#if defined(OS_WIN)
void PrintJob::ResetPageMapping() {
std::vector<int> pages = PageRange::GetPages(settings_.ranges());
if (pages.empty())
return;
pdf_page_mapping_ = std::vector<int>(document_->page_count(), -1);
for (int page_number : pages) {
// Make sure the page is in range.
if (page_number >= 0 &&
page_number < static_cast<int>(pdf_page_mapping_.size())) {
pdf_page_mapping_[page_number] = page_number;
}
}
}
#endif
void PrintJob::Observe(int type, void PrintJob::Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
const content::NotificationDetails& details) { const content::NotificationDetails& details) {
...@@ -297,10 +314,14 @@ void PrintJob::OnPdfPageConverted(int page_number, ...@@ -297,10 +314,14 @@ void PrintJob::OnPdfPageConverted(int page_number,
return; return;
} }
// Update the rendered document. It will send notifications to the listener. // Add the page to the document if it is one of the pages requested by the
document_->SetPage(pdf_page_mapping_[page_number], std::move(metafile), // user. If it is not, ignore it.
scale_factor, pdf_conversion_state_->page_size(), if (pdf_page_mapping_[page_number] != -1) {
pdf_conversion_state_->content_area()); // Update the rendered document. It will send notifications to the listener.
document_->SetPage(pdf_page_mapping_[page_number], std::move(metafile),
scale_factor, pdf_conversion_state_->page_size(),
pdf_conversion_state_->content_area());
}
pdf_conversion_state_->GetMorePages( pdf_conversion_state_->GetMorePages(
base::Bind(&PrintJob::OnPdfPageConverted, this)); base::Bind(&PrintJob::OnPdfPageConverted, this));
......
...@@ -53,6 +53,15 @@ class PrintJob : public PrintJobWorkerOwner, ...@@ -53,6 +53,15 @@ class PrintJob : public PrintJobWorkerOwner,
const base::string16& name, const base::string16& name,
int page_count); int page_count);
#if defined(OS_WIN)
// Overwrites the PDF page mapping to fill in values of -1 for all indices
// that are not selected. This is needed when the user opens the system
// dialog from the link in Print Preview on Windows and then sets a selection
// of pages, because all PDF pages will be converted, but only the user's
// selected pages should be sent to the printer. See https://crbug.com/823876.
void ResetPageMapping();
#endif
// content::NotificationObserver implementation. // content::NotificationObserver implementation.
void Observe(int type, void Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
......
...@@ -322,8 +322,7 @@ void PrintJobWorker::StartPrinting(PrintedDocument* new_document) { ...@@ -322,8 +322,7 @@ void PrintJobWorker::StartPrinting(PrintedDocument* new_document) {
return; return;
} }
// Try to print already cached data. It may already have been generated for // This will start a loop to wait for the page data.
// the print preview.
OnNewPage(); OnNewPage();
// Don't touch this anymore since the instance could be destroyed. It happens // Don't touch this anymore since the instance could be destroyed. It happens
// if all the pages are printed a one sweep and the client doesn't have a // if all the pages are printed a one sweep and the client doesn't have a
......
...@@ -263,6 +263,10 @@ void PrintViewManagerBase::StartLocalPrintJob( ...@@ -263,6 +263,10 @@ void PrintViewManagerBase::StartLocalPrintJob(
return; return;
} }
#if defined(OS_WIN)
print_job_->ResetPageMapping();
#endif
const printing::PrintSettings& settings = printer_query->settings(); const printing::PrintSettings& settings = printer_query->settings();
gfx::Size page_size = settings.page_setup_device_units().physical_size(); gfx::Size page_size = settings.page_setup_device_units().physical_size();
gfx::Rect content_area = gfx::Rect content_area =
...@@ -578,8 +582,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(PrintJobWorkerOwner* job) { ...@@ -578,8 +582,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(PrintJobWorkerOwner* job) {
return false; return false;
} }
// Ask the renderer to generate the print preview, create the print preview
// view and switch to it, initialize the printer and show the print dialog.
DCHECK(!print_job_.get()); DCHECK(!print_job_.get());
DCHECK(job); DCHECK(job);
if (!job) if (!job)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment