From 34952b732e33ce8310cb17fae798b238d44344e7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 20 Apr 2019 12:36:49 +0200 Subject: [PATCH] Add a `getDocId` method to the `idFactory`, in `Page` instances, to avoid passing around `PDFManager` instances unnecessarily (PR 7941 follow-up) This way we can avoid manually building a "document id" in multiple places in `evaluator.js`, and it also let's us avoid passing in an otherwise unnecessary `PDFManager` instance when creating a `PartialEvaluator`. --- src/core/annotation.js | 2 +- src/core/document.js | 8 ++++---- src/core/evaluator.js | 13 ++++++------- test/unit/annotation_spec.js | 22 +++------------------- test/unit/document_spec.js | 17 ++++++----------- test/unit/evaluator_spec.js | 6 ++---- test/unit/test_utils.js | 14 ++++++++++++++ 7 files changed, 36 insertions(+), 46 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index b49500e75..742adaf43 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -50,7 +50,7 @@ class AnnotationFactory { if (!isDict(dict)) { return; } - let id = isRef(ref) ? ref.toString() : 'annot_' + idFactory.createObjId(); + let id = isRef(ref) ? ref.toString() : `annot_${idFactory.createObjId()}`; // Determine the annotation's subtype. let subtype = dict.get('Subtype'); diff --git a/src/core/document.js b/src/core/document.js index 228f505e8..525f70312 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -54,13 +54,15 @@ class Page { this.evaluatorOptions = pdfManager.evaluatorOptions; this.resourcesPromise = null; - const uniquePrefix = `p${this.pageIndex}_`; const idCounters = { obj: 0, }; this.idFactory = { createObjId() { - return uniquePrefix + (++idCounters.obj); + return `p${pageIndex}_${++idCounters.obj}`; + }, + getDocId() { + return `g_${pdfManager.docId}`; }, }; } @@ -195,7 +197,6 @@ class Page { ]); const partialEvaluator = new PartialEvaluator({ - pdfManager: this.pdfManager, xref: this.xref, handler, pageIndex: this.pageIndex, @@ -270,7 +271,6 @@ class Page { const dataPromises = Promise.all([contentStreamPromise, resourcesPromise]); return dataPromises.then(([contentStream]) => { const partialEvaluator = new PartialEvaluator({ - pdfManager: this.pdfManager, xref: this.xref, handler, pageIndex: this.pageIndex, diff --git a/src/core/evaluator.js b/src/core/evaluator.js index d21e27c0b..8f7917f77 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -61,10 +61,9 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { isEvalSupported: true, }; - function PartialEvaluator({ pdfManager, xref, handler, pageIndex, idFactory, - fontCache, builtInCMapCache, options = null, + function PartialEvaluator({ xref, handler, pageIndex, idFactory, fontCache, + builtInCMapCache, options = null, pdfFunctionFactory, }) { - this.pdfManager = pdfManager; this.xref = xref; this.handler = handler; this.pageIndex = pageIndex; @@ -372,13 +371,13 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { NativeImageDecoding.NONE : this.options.nativeImageDecoderSupport; // If there is no imageMask, create the PDFImage and a lot // of image processing can be done here. - let objId = 'img_' + this.idFactory.createObjId(); + let objId = `img_${this.idFactory.createObjId()}`; if (this.parsingType3Font) { assert(nativeImageDecoderSupport === NativeImageDecoding.NONE, 'Type3 image resources should be completely decoded in the worker.'); - objId = `g_${this.pdfManager.docId}_type3res_${objId}`; + objId = `${this.idFactory.getDocId()}_type3res_${objId}`; } if (nativeImageDecoderSupport !== NativeImageDecoding.NONE && @@ -774,13 +773,13 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { if (!fontID) { fontID = this.idFactory.createObjId(); } - this.fontCache.put('id_' + fontID, fontCapability.promise); + this.fontCache.put(`id_${fontID}`, fontCapability.promise); } assert(fontID, 'The "fontID" must be defined.'); // Keep track of each font we translated so the caller can // load them asynchronously before calling display on a page. - font.loadedName = 'g_' + this.pdfManager.docId + '_f' + fontID; + font.loadedName = `${this.idFactory.getDocId()}_f${fontID}`; font.translated = fontCapability.promise; diff --git a/test/unit/annotation_spec.js b/test/unit/annotation_spec.js index 07c567972..9e1af94a2 100644 --- a/test/unit/annotation_spec.js +++ b/test/unit/annotation_spec.js @@ -20,10 +20,10 @@ import { AnnotationBorderStyleType, AnnotationFieldFlag, AnnotationFlag, AnnotationType, stringToBytes, stringToUTF8String } from '../../src/shared/util'; +import { createIdFactory, XRefMock } from './test_utils'; import { Dict, Name, Ref } from '../../src/core/primitives'; import { Lexer, Parser } from '../../src/core/parser'; import { StringStream } from '../../src/core/stream'; -import { XRefMock } from './test_utils'; describe('annotation', function() { class PDFManagerMock { @@ -43,26 +43,13 @@ describe('annotation', function() { } } - class IdFactoryMock { - constructor(params) { - this.uniquePrefix = params.prefix || 'p0_'; - this.idCounters = { - obj: params.startObjId || 0, - }; - } - - createObjId() { - return this.uniquePrefix + (++this.idCounters.obj); - } - } - let pdfManagerMock, idFactoryMock; beforeAll(function(done) { pdfManagerMock = new PDFManagerMock({ docBaseUrl: null, }); - idFactoryMock = new IdFactoryMock({ }); + idFactoryMock = createIdFactory(/* pageIndex = */ 0); done(); }); @@ -97,10 +84,7 @@ describe('annotation', function() { annotationDict.set('Subtype', Name.get('Link')); const xref = new XRefMock(); - const idFactory = new IdFactoryMock({ - prefix: 'p0_', - startObjId: 0, - }); + const idFactory = createIdFactory(/* pageIndex = */ 0); const annotation1 = AnnotationFactory.create(xref, annotationDict, pdfManagerMock, idFactory).then(({ data, }) => { diff --git a/test/unit/document_spec.js b/test/unit/document_spec.js index bb6a84875..b737a23bb 100644 --- a/test/unit/document_spec.js +++ b/test/unit/document_spec.js @@ -13,30 +13,25 @@ * limitations under the License. */ -import { Page } from '../../src/core/document'; +import { createIdFactory } from './test_utils'; describe('document', function () { describe('Page', function () { it('should create correct objId using the idFactory', function () { - var page1 = new Page({ - pdfManager: { }, - pageIndex: 0, - }); - var page2 = new Page({ - pdfManager: { }, - pageIndex: 1, - }); - - var idFactory1 = page1.idFactory, idFactory2 = page2.idFactory; + const idFactory1 = createIdFactory(/* pageIndex = */ 0); + const idFactory2 = createIdFactory(/* pageIndex = */ 1); expect(idFactory1.createObjId()).toEqual('p0_1'); expect(idFactory1.createObjId()).toEqual('p0_2'); + expect(idFactory1.getDocId()).toEqual('g_d0'); expect(idFactory2.createObjId()).toEqual('p1_1'); expect(idFactory2.createObjId()).toEqual('p1_2'); + expect(idFactory2.getDocId()).toEqual('g_d0'); expect(idFactory1.createObjId()).toEqual('p0_3'); expect(idFactory1.createObjId()).toEqual('p0_4'); + expect(idFactory1.getDocId()).toEqual('g_d0'); }); }); }); diff --git a/test/unit/evaluator_spec.js b/test/unit/evaluator_spec.js index 748e749e8..8ec1cc871 100644 --- a/test/unit/evaluator_spec.js +++ b/test/unit/evaluator_spec.js @@ -13,13 +13,13 @@ * limitations under the License. */ +import { createIdFactory, XRefMock } from './test_utils'; import { Dict, Name } from '../../src/core/primitives'; import { FormatError, OPS } from '../../src/shared/util'; import { Stream, StringStream } from '../../src/core/stream'; import { OperatorList } from '../../src/core/operator_list'; import { PartialEvaluator } from '../../src/core/evaluator'; import { WorkerTask } from '../../src/core/worker'; -import { XRefMock } from './test_utils'; describe('evaluator', function() { function HandlerMock() { @@ -37,8 +37,6 @@ describe('evaluator', function() { }, }; - function PdfManagerMock() { } - function runOperatorListCheck(evaluator, stream, resources, callback) { var result = new OperatorList(); var task = new WorkerTask('OperatorListCheck'); @@ -58,10 +56,10 @@ describe('evaluator', function() { beforeAll(function(done) { partialEvaluator = new PartialEvaluator({ - pdfManager: new PdfManagerMock(), xref: new XRefMock(), handler: new HandlerMock(), pageIndex: 0, + idFactory: createIdFactory(/* pageIndex = */ 0), }); done(); }); diff --git a/test/unit/test_utils.js b/test/unit/test_utils.js index 32c43f682..e931e7230 100644 --- a/test/unit/test_utils.js +++ b/test/unit/test_utils.js @@ -16,6 +16,7 @@ import { assert, CMapCompressionType } from '../../src/shared/util'; import isNodeJS from '../../src/shared/is_node'; import { isRef } from '../../src/core/primitives'; +import { Page } from '../../src/core/document'; class DOMFileReaderFactory { static async fetch(params) { @@ -158,6 +159,18 @@ class XRefMock { } } +function createIdFactory(pageIndex) { + const page = new Page({ + pdfManager: { + get docId() { + return 'd0'; + }, + }, + pageIndex, + }); + return page.idFactory; +} + export { DOMFileReaderFactory, NodeFileReaderFactory, @@ -166,4 +179,5 @@ export { XRefMock, buildGetDocumentParams, TEST_PDFS_PATH, + createIdFactory, };