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

[api-minor] Add an option, in PDFDocumentProxy.cleanup, to allow fonts to remain attached to the DOM #13172

Merged
merged 3 commits into from
Apr 5, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 30 additions & 23 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -921,10 +921,13 @@ class PDFDocumentProxy {
* NOTE: Do not, under any circumstances, call this method when rendering is
* currently ongoing since that may lead to rendering errors.
*
* @param {boolean} [keepLoadedFonts] - Let fonts remain attached to the DOM.
* NOTE: This will increase persistent memory usage, hence don't use this
* option unless absolutely necessary. The default value is `false`.
* @returns {Promise} A promise that is resolved when clean-up has finished.
*/
cleanup() {
return this._transport.startCleanup();
cleanup(keepLoadedFonts = false) {
return this._transport.startCleanup(keepLoadedFonts || this.isPureXfa);
}

/**
Expand Down Expand Up @@ -1180,14 +1183,14 @@ class PDFPageProxy {
* {Array} of the annotation objects.
*/
getAnnotations({ intent = null } = {}) {
if (!this.annotationsPromise || this.annotationsIntent !== intent) {
this.annotationsPromise = this._transport.getAnnotations(
if (!this._annotationsPromise || this._annotationsIntent !== intent) {
this._annotationsPromise = this._transport.getAnnotations(
this._pageIndex,
intent
);
this.annotationsIntent = intent;
this._annotationsIntent = intent;
}
return this.annotationsPromise;
return this._annotationsPromise;
}

/**
Expand Down Expand Up @@ -1490,7 +1493,7 @@ class PDFPageProxy {
}
}
this.objs.clear();
this.annotationsPromise = null;
this._annotationsPromise = null;
this._jsActionsPromise = null;
this._xfaPromise = null;
this.pendingCleanup = false;
Expand Down Expand Up @@ -1525,7 +1528,7 @@ class PDFPageProxy {

this._intentStates.clear();
this.objs.clear();
this.annotationsPromise = null;
this._annotationsPromise = null;
this._jsActionsPromise = null;
this._xfaPromise = null;
if (resetStats && this._stats) {
Expand Down Expand Up @@ -2799,24 +2802,28 @@ class WorkerTransport {
return this.messageHandler.sendWithPromise("GetStats", null);
}

startCleanup() {
return this.messageHandler.sendWithPromise("Cleanup", null).then(() => {
for (let i = 0, ii = this.pageCache.length; i < ii; i++) {
const page = this.pageCache[i];
if (page) {
const cleanupSuccessful = page.cleanup();
async startCleanup(keepLoadedFonts = false) {
await this.messageHandler.sendWithPromise("Cleanup", null);

if (!cleanupSuccessful) {
throw new Error(
`startCleanup: Page ${i + 1} is currently rendering.`
);
}
}
if (this.destroyed) {
return; // No need to manually clean-up when destruction has started.
}
for (let i = 0, ii = this.pageCache.length; i < ii; i++) {
const page = this.pageCache[i];
if (!page) {
continue;
}
this.commonObjs.clear();
const cleanupSuccessful = page.cleanup();

if (!cleanupSuccessful) {
throw new Error(`startCleanup: Page ${i + 1} is currently rendering.`);
}
}
this.commonObjs.clear();
if (!keepLoadedFonts) {
this.fontLoader.clear();
this._hasJSActionsPromise = null;
});
}
this._hasJSActionsPromise = null;
}

get loadingParams() {
Expand Down
126 changes: 54 additions & 72 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1339,12 +1339,10 @@ describe("api", function () {
.catch(done.fail);
});

it("cleans up document resources", function (done) {
const promise = pdfDocument.cleanup();
promise.then(function () {
expect(true).toEqual(true);
done();
}, done.fail);
it("cleans up document resources", async function () {
await pdfDocument.cleanup();

expect(true).toEqual(true);
});

it("checks that fingerprints are unique", function (done) {
Expand Down Expand Up @@ -1982,85 +1980,69 @@ describe("api", function () {
]).then(done);
});

it("cleans up document resources after rendering of page", function (done) {
it("cleans up document resources after rendering of page", async function () {
const loadingTask = getDocument(buildGetDocumentParams(basicApiFileName));
let canvasAndCtx;
const pdfDoc = await loadingTask.promise;
const pdfPage = await pdfDoc.getPage(1);

loadingTask.promise
.then(pdfDoc => {
return pdfDoc.getPage(1).then(pdfPage => {
const viewport = pdfPage.getViewport({ scale: 1 });
canvasAndCtx = CanvasFactory.create(
viewport.width,
viewport.height
);
const viewport = pdfPage.getViewport({ scale: 1 });
const canvasAndCtx = CanvasFactory.create(
viewport.width,
viewport.height
);

const renderTask = pdfPage.render({
canvasContext: canvasAndCtx.context,
canvasFactory: CanvasFactory,
viewport,
});
return renderTask.promise.then(() => {
return pdfDoc.cleanup();
});
});
})
.then(() => {
expect(true).toEqual(true);
const renderTask = pdfPage.render({
canvasContext: canvasAndCtx.context,
canvasFactory: CanvasFactory,
viewport,
});
await renderTask.promise;

CanvasFactory.destroy(canvasAndCtx);
loadingTask.destroy().then(done);
}, done.fail);
await pdfDoc.cleanup();

expect(true).toEqual(true);

CanvasFactory.destroy(canvasAndCtx);
await loadingTask.destroy();
});

it("cleans up document resources during rendering of page", function (done) {
it("cleans up document resources during rendering of page", async function () {
const loadingTask = getDocument(
buildGetDocumentParams("tracemonkey.pdf")
);
let canvasAndCtx;
const pdfDoc = await loadingTask.promise;
const pdfPage = await pdfDoc.getPage(1);

loadingTask.promise
.then(pdfDoc => {
return pdfDoc.getPage(1).then(pdfPage => {
const viewport = pdfPage.getViewport({ scale: 1 });
canvasAndCtx = CanvasFactory.create(
viewport.width,
viewport.height
);
const viewport = pdfPage.getViewport({ scale: 1 });
const canvasAndCtx = CanvasFactory.create(
viewport.width,
viewport.height
);

const renderTask = pdfPage.render({
canvasContext: canvasAndCtx.context,
canvasFactory: CanvasFactory,
viewport,
});
const renderTask = pdfPage.render({
canvasContext: canvasAndCtx.context,
canvasFactory: CanvasFactory,
viewport,
});
// Ensure that clean-up runs during rendering.
renderTask.onContinue = function (cont) {
waitSome(cont);
};

renderTask.onContinue = function (cont) {
waitSome(cont);
};
try {
await pdfDoc.cleanup();

return pdfDoc
.cleanup()
.then(
() => {
throw new Error("shall fail cleanup");
},
reason => {
expect(reason instanceof Error).toEqual(true);
expect(reason.message).toEqual(
"startCleanup: Page 1 is currently rendering."
);
}
)
.then(() => {
return renderTask.promise;
})
.then(() => {
CanvasFactory.destroy(canvasAndCtx);
loadingTask.destroy().then(done);
});
});
})
.catch(done.fail);
throw new Error("shall fail cleanup");
} catch (reason) {
expect(reason instanceof Error).toEqual(true);
expect(reason.message).toEqual(
"startCleanup: Page 1 is currently rendering."
);
}
await renderTask.promise;

CanvasFactory.destroy(canvasAndCtx);
await loadingTask.destroy();
});

it("caches image resources at the document/page level as expected (issue 11878)", async function () {
Expand Down
13 changes: 8 additions & 5 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ const PDFViewerApplication = {
this.overlayManager = new OverlayManager();

const pdfRenderingQueue = new PDFRenderingQueue();
pdfRenderingQueue.onIdle = this.cleanup.bind(this);
pdfRenderingQueue.onIdle = this._cleanup.bind(this);
this.pdfRenderingQueue = pdfRenderingQueue;

const pdfLinkService = new PDFLinkService({
Expand Down Expand Up @@ -1767,17 +1767,20 @@ const PDFViewerApplication = {
}
},

cleanup() {
/**
* @private
*/
_cleanup() {
if (!this.pdfDocument) {
return; // run cleanup when document is loaded
}
this.pdfViewer.cleanup();
this.pdfThumbnailViewer.cleanup();

// We don't want to remove fonts used by active page SVGs.
if (this.pdfViewer.renderer !== RendererType.SVG) {
this.pdfDocument.cleanup();
}
this.pdfDocument.cleanup(
/* keepLoadedFonts = */ this.pdfViewer.renderer === RendererType.SVG
);
},

forceRendering() {
Expand Down