Merge pull request #10754 from Snuffleupagus/idFactory-getDocId

Add a `getDocId` method to the `idFactory`, in `Page` instances, to avoid passing around `PDFManager` instances unnecessarily (PR 7941 follow-up)

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');

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,

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;
 

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, }) => {

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');
     });
   });
 });

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();
   });

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,
 };