Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Small clean-up of the PDFDocumentProxy.destroy method and related code

Browse Source 
Note how `PDFDocumentProxy.destroy` is a nothing more than an alias for `PDFDocumentLoadingTask.destroy`. While removing the latter method would be a breaking API change, there's still room for at least some clean-up here.

The main changes in this patch are:
 - Stop providing a `PDFDocumentLoadingTask` instance *separately* when creating a `PDFDocumentProxy`, since the loadingTask is already available through the `WorkerTransport` instance.
 - Stop tracking the `PDFDocumentProxy` instance on the `WorkerTransport`, since that property is completely unused.
 - Simplify the 'Multiple `getDocument` instances' unit-tests by only destroying *once*, rather than twice, for each document.
This commit is contained in:
Jonas Jenwald 2019-03-11 12:43:44 +01:00
parent d587abbceb
commit 24fc4f83ca
2 changed files with 14 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/display/api.js
View File

@ -574,9 +574,7 @@ class PDFDataRangeTransport {
* properties that can be read synchronously.
*/
class PDFDocumentProxy {
constructor(pdfInfo, transport, loadingTask) {
this.loadingTask = loadingTask;
constructor(pdfInfo, transport) {
this._pdfInfo = pdfInfo;
this._transport = transport;
}
@ -761,6 +759,13 @@ class PDFDocumentProxy {
get loadingParams() {
return this._transport.loadingParams;
}
/**
* @return {PDFDocumentLoadingTask} The loadingTask for the current document.
*/
get loadingTask() {
return this._transport.loadingTask;
}
}
/**
@ -1827,9 +1832,8 @@ class WorkerTransport {
}, this);
messageHandler.on('GetDoc', function({ pdfInfo, }) {
this.numPages = pdfInfo.numPages;
this.pdfDocument = new PDFDocumentProxy(pdfInfo, this, loadingTask);
loadingTask._capability.resolve(this.pdfDocument);
this._numPages = pdfInfo.numPages;
loadingTask._capability.resolve(new PDFDocumentProxy(pdfInfo, this));
}, this);
messageHandler.on('PasswordRequest', function(exception) {
@ -2130,7 +2134,7 @@ class WorkerTransport {
getPage(pageNumber) {
if (!Number.isInteger(pageNumber) ||
pageNumber <= 0 || pageNumber > this.numPages) {
pageNumber <= 0 || pageNumber > this._numPages) {
return Promise.reject(new Error('Invalid page request'));
}

16
test/unit/api_spec.js
View File

@ -954,7 +954,7 @@ describe('api', function() {
return _checkCanLoad(false, filename, options);
}
afterEach(function(done) {
if (loadingTask) {
if (loadingTask && !loadingTask.destroyed) {
loadingTask.destroy().then(done);
} else {
done();
@ -1385,7 +1385,6 @@ describe('api', function() {
// A PDF using the Arial font.
var pdf3 = buildGetDocumentParams('issue6068.pdf');
var loadingTasks = [];
var pdfDocuments = [];
// Render the first page of the given PDF file.
// Fulfills the promise with the base64-encoded version of the PDF.
@ -1393,7 +1392,6 @@ describe('api', function() {
const loadingTask = getDocument(filename);
loadingTasks.push(loadingTask);
const pdf = await loadingTask.promise;
pdfDocuments.push(pdf);
const page = await pdf.getPage(1);
const viewport = page.getViewport({ scale: 1.2, });
const canvasAndCtx = CanvasFactory.create(viewport.width,
@ -1413,18 +1411,10 @@ describe('api', function() {
// Issue 6205 reported an issue with font rendering, so clear the loaded
// fonts so that we can see whether loading PDFs in parallel does not
// cause any issues with the rendered fonts.
var destroyPromises = pdfDocuments.map(function(pdfDocument) {
return pdfDocument.destroy();
});
// Destroy the workers.
var destroyPromises2 = loadingTasks.map(function(loadingTask) {
const destroyPromises = loadingTasks.map(function(loadingTask) {
return loadingTask.destroy();
});
Promise.all(destroyPromises.concat(destroyPromises2)).then(function() {
done();
});
Promise.all(destroyPromises).then(done);
});
it('should correctly render PDFs in parallel', function(done) {