From 1c869906c8d1dcb2d7e389b1b1dd5a0645fe445a Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Thu, 13 Oct 2016 10:47:11 +0200 Subject: [PATCH 1/4] Strictly manage lifetime of PDFPrintService Make sure that the print service is stopped as soon as possible when aborted, and that it is not possible for a (slow) promise to accidentally wipe the state of a print job that was started later. --- web/pdf_print_service.js | 74 ++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js index 4ddac428d..b8eba0800 100644 --- a/web/pdf_print_service.js +++ b/web/pdf_print_service.js @@ -39,6 +39,7 @@ var scratchCanvas = null; function renderPage(pdfDocument, pageNumber, size, wrapper) { + var activeServiceOnEntry = activeService; if (!scratchCanvas) { scratchCanvas = document.createElement('canvas'); } @@ -69,9 +70,7 @@ }; return pdfPage.render(renderContext).promise; }).then(function() { - if (!activeService) { - return Promise.reject(new Error('cancelled')); - } + activeServiceOnEntry.throwIfInactive(); if (('toBlob' in scratchCanvas) && !pdfjsLib.PDFJS.disableCreateObjectURL) { scratchCanvas.toBlob(function (blob) { @@ -98,6 +97,8 @@ PDFPrintService.prototype = { layout: function () { + this.throwIfInactive(); + var pdfDocument = this.pdfDocument; var printContainer = this.printContainer; var body = document.querySelector('body'); @@ -139,15 +140,18 @@ }, destroy: function () { + if (activeService !== this) { + // |activeService| cannot be replaced without calling destroy() first, + // so if it differs then an external consumer has a stale reference to + // us. + return; + } this.printContainer.textContent = ''; this.wrappers = null; if (this.pageStyleSheet && this.pageStyleSheet.parentNode) { this.pageStyleSheet.parentNode.removeChild(this.pageStyleSheet); this.pageStyleSheet = null; } - if (activeService !== this) { - return; // no need to clean up shared resources - } activeService = null; if (scratchCanvas) { scratchCanvas.width = scratchCanvas.height = 0; @@ -164,10 +168,7 @@ renderPages: function () { var pageCount = this.pagesOverview.length; var renderNextPage = function (resolve, reject) { - if (activeService !== this) { - reject(new Error('cancelled')); - return; - } + this.throwIfInactive(); if (++this.currentPage >= pageCount) { renderProgress(pageCount, pageCount); resolve(); @@ -181,6 +182,16 @@ }.bind(this); return new Promise(renderNextPage); }, + + get active() { + return this === activeService; + }, + + throwIfInactive: function () { + if (!this.active) { + throw new Error('This print request was cancelled or completed.'); + } + }, }; @@ -200,7 +211,22 @@ if (!activeService) { console.error('Expected print service to be initialized.'); } - activeService.renderPages().then(startPrint, abort); + var activeServiceOnEntry = activeService; + activeService.renderPages().then(function () { + activeServiceOnEntry.throwIfInactive(); + return startPrint(activeServiceOnEntry); + }).catch(function () { + // Ignore any error messages. + }).then(function () { + // aborts acts on the "active" print request, so we need to check + // whether the print request (activeServiceOnEntry) is still active. + // Without the check, an unrelated print request (created after aborting + // this print request while the pages were being generated) would be + // aborted. + if (activeServiceOnEntry.active) { + abort(); + } + }); } }; @@ -210,17 +236,21 @@ window.dispatchEvent(event); } - function startPrint() { - // Push window.print in the macrotask queue to avoid being affected by - // the deprecation of running print() code in a microtask, see - // https://github.com/mozilla/pdf.js/issues/7547. - setTimeout(function() { - if (!activeService) { - return; // Print task cancelled by user. - } - print.call(window); - setTimeout(abort, 20); // Tidy-up - }, 0); + function startPrint(activeServiceOnEntry) { + return new Promise(function (resolve) { + // Push window.print in the macrotask queue to avoid being affected by + // the deprecation of running print() code in a microtask, see + // https://github.com/mozilla/pdf.js/issues/7547. + setTimeout(function () { + if (!activeServiceOnEntry.active) { + resolve(); + return; + } + print.call(window); + // Delay promise resolution in case print() was not synchronous. + setTimeout(resolve, 20); // Tidy-up. + }, 0); + }); } function abort() { From 0c21ebf9f3f957f1126ef5220af58e697005fd09 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Thu, 13 Oct 2016 23:36:57 +0200 Subject: [PATCH 2/4] Close overlay if print service was not initialized Fixes #7720 --- web/pdf_print_service.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js index b8eba0800..8d0da3bd3 100644 --- a/web/pdf_print_service.js +++ b/web/pdf_print_service.js @@ -202,7 +202,9 @@ return; } ensureOverlay().then(function () { - OverlayManager.open('printServiceOverlay'); + if (activeService) { + OverlayManager.open('printServiceOverlay'); + } }); try { @@ -210,6 +212,10 @@ } finally { if (!activeService) { console.error('Expected print service to be initialized.'); + if (OverlayManager.active === 'printServiceOverlay') { + OverlayManager.close('printServiceOverlay'); + } + return; } var activeServiceOnEntry = activeService; activeService.renderPages().then(function () { From d3b13e36d32a7e4f4f4df4a603a97e6f3c162213 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Wed, 19 Oct 2016 11:39:02 +0200 Subject: [PATCH 3/4] Refactor page printing logic on the web - Move the global scratchCanvas to PDFPrintService. This is mainly to make it easier to reason about the state of scratchCanvas. In practice there is no difference because only one PDFPrintService instance can be instantiated at any given time. - Move all logic of using the rendered page to one location. This makes it easier to replace the printing logic later, when I add special handling to out-of-process frames in the Chrome extension. --- web/pdf_print_service.js | 88 +++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js index 8d0da3bd3..1f28b2596 100644 --- a/web/pdf_print_service.js +++ b/web/pdf_print_service.js @@ -35,14 +35,10 @@ var activeService = null; - // Using one canvas for all paint operations -- painting one canvas at a time. - var scratchCanvas = null; - - function renderPage(pdfDocument, pageNumber, size, wrapper) { - var activeServiceOnEntry = activeService; - if (!scratchCanvas) { - scratchCanvas = document.createElement('canvas'); - } + // Renders the page to the canvas of the given print service, and returns + // the suggested dimensions of the output page. + function renderPage(activeServiceOnEntry, pdfDocument, pageNumber, size) { + var scratchCanvas = activeService.scratchCanvas; // The size of the canvas in pixels for printing. var PRINT_RESOLUTION = 150; @@ -51,9 +47,8 @@ scratchCanvas.height = Math.floor(size.height * PRINT_UNITS); // The physical size of the img as specified by the PDF document. - var img = document.createElement('img'); - img.style.width = Math.floor(size.width * CSS_UNITS) + 'px'; - img.style.height = Math.floor(size.height * CSS_UNITS) + 'px'; + var width = Math.floor(size.width * CSS_UNITS) + 'px'; + var height = Math.floor(size.height * CSS_UNITS) + 'px'; var ctx = scratchCanvas.getContext('2d'); ctx.save(); @@ -69,21 +64,11 @@ intent: 'print' }; return pdfPage.render(renderContext).promise; - }).then(function() { - activeServiceOnEntry.throwIfInactive(); - if (('toBlob' in scratchCanvas) && - !pdfjsLib.PDFJS.disableCreateObjectURL) { - scratchCanvas.toBlob(function (blob) { - img.src = URL.createObjectURL(blob); - }); - } else { - img.src = scratchCanvas.toDataURL(); - } - wrapper.appendChild(img); - return new Promise(function(resolve, reject) { - img.onload = resolve; - img.onerror = reject; - }); + }).then(function () { + return { + width: width, + height: height, + }; }); } @@ -91,8 +76,9 @@ this.pdfDocument = pdfDocument; this.pagesOverview = pagesOverview; this.printContainer = printContainer; - this.wrappers = []; this.currentPage = -1; + // The temporary canvas where renderPage paints one page at a time. + this.scratchCanvas = document.createElement('canvas'); } PDFPrintService.prototype = { @@ -100,7 +86,6 @@ this.throwIfInactive(); var pdfDocument = this.pdfDocument; - var printContainer = this.printContainer; var body = document.querySelector('body'); body.setAttribute('data-pdfjsprinting', true); @@ -131,12 +116,6 @@ '@page { size: ' + pageSize.width + 'pt ' + pageSize.height + 'pt;}' + '}'; body.appendChild(this.pageStyleSheet); - - for (var i = 0, ii = this.pagesOverview.length; i < ii; ++i) { - var wrapper = document.createElement('div'); - printContainer.appendChild(wrapper); - this.wrappers[i] = wrapper; - } }, destroy: function () { @@ -147,16 +126,13 @@ return; } this.printContainer.textContent = ''; - this.wrappers = null; if (this.pageStyleSheet && this.pageStyleSheet.parentNode) { this.pageStyleSheet.parentNode.removeChild(this.pageStyleSheet); this.pageStyleSheet = null; } + this.scratchCanvas.width = this.scratchCanvas.height = 0; + this.scratchCanvas = null; activeService = null; - if (scratchCanvas) { - scratchCanvas.width = scratchCanvas.height = 0; - scratchCanvas = null; - } ensureOverlay().then(function () { if (OverlayManager.active !== 'printServiceOverlay') { return; // overlay was already closed @@ -176,13 +152,41 @@ } var index = this.currentPage; renderProgress(index, pageCount); - renderPage(this.pdfDocument, index + 1, - this.pagesOverview[index], this.wrappers[index]).then( - function () { renderNextPage(resolve, reject); }, reject); + renderPage(this, this.pdfDocument, index + 1, this.pagesOverview[index]) + .then(this.useRenderedPage.bind(this)) + .then(function () { + renderNextPage(resolve, reject); + }, reject); }.bind(this); return new Promise(renderNextPage); }, + useRenderedPage: function (printItem) { + this.throwIfInactive(); + var img = document.createElement('img'); + img.style.width = printItem.width; + img.style.height = printItem.height; + + var scratchCanvas = this.scratchCanvas; + if (('toBlob' in scratchCanvas) && + !pdfjsLib.PDFJS.disableCreateObjectURL) { + scratchCanvas.toBlob(function (blob) { + img.src = URL.createObjectURL(blob); + }); + } else { + img.src = scratchCanvas.toDataURL(); + } + + var wrapper = document.createElement('div'); + wrapper.appendChild(img); + this.printContainer.appendChild(wrapper); + + return new Promise(function (resolve, reject) { + img.onload = resolve; + img.onerror = reject; + }); + }, + get active() { return this === activeService; }, From 594592216c9b6c33d48a972c6427a8233fbe5bd3 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Wed, 19 Oct 2016 11:47:54 +0200 Subject: [PATCH 4/4] Refactor printing: startPrint -> performPrint - Renamed startPrint to performPrint to emphasize that the method does not start the print process (preparing pages for the printer), but that it does the actual printing (sending pages off to the printer). - Put performPrint in the PDFPrintService, so that it can be overridden if needed. --- web/pdf_print_service.js | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js index 1f28b2596..392cdaf91 100644 --- a/web/pdf_print_service.js +++ b/web/pdf_print_service.js @@ -187,6 +187,24 @@ }); }, + performPrint: function () { + this.throwIfInactive(); + return new Promise(function (resolve) { + // Push window.print in the macrotask queue to avoid being affected by + // the deprecation of running print() code in a microtask, see + // https://github.com/mozilla/pdf.js/issues/7547. + setTimeout(function () { + if (!this.active) { + resolve(); + return; + } + print.call(window); + // Delay promise resolution in case print() was not synchronous. + setTimeout(resolve, 20); // Tidy-up. + }.bind(this), 0); + }.bind(this)); + }, + get active() { return this === activeService; }, @@ -223,8 +241,7 @@ } var activeServiceOnEntry = activeService; activeService.renderPages().then(function () { - activeServiceOnEntry.throwIfInactive(); - return startPrint(activeServiceOnEntry); + return activeServiceOnEntry.performPrint(); }).catch(function () { // Ignore any error messages. }).then(function () { @@ -246,23 +263,6 @@ window.dispatchEvent(event); } - function startPrint(activeServiceOnEntry) { - return new Promise(function (resolve) { - // Push window.print in the macrotask queue to avoid being affected by - // the deprecation of running print() code in a microtask, see - // https://github.com/mozilla/pdf.js/issues/7547. - setTimeout(function () { - if (!activeServiceOnEntry.active) { - resolve(); - return; - } - print.call(window); - // Delay promise resolution in case print() was not synchronous. - setTimeout(resolve, 20); // Tidy-up. - }, 0); - }); - } - function abort() { if (activeService) { activeService.destroy();