Merge pull request #20939 from Snuffleupagus/BaseCMapReaderFactory-filename

[api-minor] Simplify `BaseCMapReaderFactory` by having the worker-thread create the `filename`

Author: timvandermeij

src/core/evaluator.js

@@ -102,6 +102,7 @@ const DefaultPartialEvaluatorOptions = Object.freeze({
   useWasm: true,
   useWorkerFetch: true,
   cMapUrl: null,
+  cMapPacked: true,
   iccUrl: null,
   standardFontDataUrl: null,
   wasmUrl: null,
@@ -413,10 +414,13 @@ class PartialEvaluator {
         throw new Error("Only worker-thread fetching supported.");
       }
       // Get the data on the main-thread instead.
-      data = await this.handler.sendWithPromise("FetchBinaryData", {
-        type: "cMapReaderFactory",
-        name,
-      });
+      data = {
+        cMapData: await this.handler.sendWithPromise("FetchBinaryData", {
+          type: "cMapReaderFactory",
+          filename: `${name}${this.options.cMapPacked ? ".bcmap" : ""}`,
+        }),
+        isCompressed: this.options.cMapPacked,
+      };
     }
     // Cache the CMap data, to avoid fetching it repeatedly.
     this.builtInCMapCache.set(name, data);

src/display/api.js

@@ -366,6 +366,7 @@ function getDocument(src = {}) {
           StandardFontDataFactory === DOMStandardFontDataFactory &&
           WasmFactory === DOMWasmFactory &&
           cMapUrl &&
+          cMapPacked &&
           standardFontDataUrl &&
           wasmUrl &&
           isValidFetchUrl(cMapUrl, document.baseURI) &&
@@ -391,7 +392,7 @@ function getDocument(src = {}) {
       (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) ||
       useWorkerFetch
         ? null
-        : new CMapReaderFactory({ baseUrl: cMapUrl, isCompressed: cMapPacked }),
+        : new CMapReaderFactory({ baseUrl: cMapUrl }),
     standardFontDataFactory:
       (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) ||
       useWorkerFetch
@@ -439,6 +440,7 @@ function getDocument(src = {}) {
       useWasm,
       useWorkerFetch,
       cMapUrl,
+      cMapPacked,
       iccUrl,
       standardFontDataUrl,
       wasmUrl,

src/display/cmap_reader_factory.js

@@ -17,30 +17,25 @@ import { stringToBytes, unreachable } from "../shared/util.js";
 import { fetchData } from "./display_utils.js";
 
 class BaseCMapReaderFactory {
-  constructor({ baseUrl = null, isCompressed = true }) {
+  constructor({ baseUrl = null }) {
     if (
       (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) &&
       this.constructor === BaseCMapReaderFactory
     ) {
       unreachable("Cannot initialize BaseCMapReaderFactory.");
     }
     this.baseUrl = baseUrl;
-    this.isCompressed = isCompressed;
   }
 
-  async fetch({ name }) {
+  async fetch({ filename }) {
     if (!this.baseUrl) {
-      throw new Error(
-        "Ensure that the `cMapUrl` and `cMapPacked` API parameters are provided."
-      );
+      throw new Error("Ensure that the `cMapUrl` API parameter is provided.");
     }
-    const url = this.baseUrl + name + (this.isCompressed ? ".bcmap" : "");
+    const url = `${this.baseUrl}${filename}`;
 
-    return this._fetch(url)
-      .then(cMapData => ({ cMapData, isCompressed: this.isCompressed }))
-      .catch(reason => {
-        throw new Error(`Unable to load CMap data at: ${url}`);
-      });
+    return this._fetch(url).catch(reason => {
+      throw new Error(`Unable to load CMap data at: ${url}`);
+    });
   }
 
   /**
@@ -59,7 +54,7 @@ class DOMCMapReaderFactory extends BaseCMapReaderFactory {
   async _fetch(url) {
     const data = await fetchData(
       url,
-      /* type = */ this.isCompressed ? "bytes" : "text"
+      /* type = */ url.endsWith(".bcmap") ? "bytes" : "text"
     );
     return data instanceof Uint8Array ? data : stringToBytes(data);
   }

test/unit/annotation_spec.js

@@ -37,6 +37,7 @@ import {
   createIdFactory,
   DefaultCMapReaderFactory,
   DefaultStandardFontDataFactory,
+  fetchBuiltInCMapHelper,
   STANDARD_FONT_DATA_URL,
   XRefMock,
 } from "./test_utils.js";
@@ -116,16 +117,13 @@ describe("annotation", function () {
     const CMapReaderFactory = new DefaultCMapReaderFactory({
       baseUrl: CMAP_URL,
     });
+    const fetchBuiltInCMap = name =>
+      fetchBuiltInCMapHelper(CMapReaderFactory, /* cMapPacked = */ true, name);
 
     const builtInCMapCache = new Map();
-    builtInCMapCache.set(
-      "UniJIS-UTF16-H",
-      await CMapReaderFactory.fetch({ name: "UniJIS-UTF16-H" })
-    );
-    builtInCMapCache.set(
-      "Adobe-Japan1-UCS2",
-      await CMapReaderFactory.fetch({ name: "Adobe-Japan1-UCS2" })
-    );
+    for (const name of ["UniJIS-UTF16-H", "Adobe-Japan1-UCS2"]) {
+      builtInCMapCache.set(name, await fetchBuiltInCMap(name));
+    }
 
     idFactoryMock = createIdFactory(/* pageIndex = */ 0);
     partialEvaluator = new PartialEvaluator({

test/unit/cmap_spec.js

@@ -14,24 +14,24 @@
  */
 
 import { CMap, CMapFactory, IdentityCMap } from "../../src/core/cmap.js";
-import { CMAP_URL, DefaultCMapReaderFactory } from "./test_utils.js";
+import {
+  CMAP_URL,
+  DefaultCMapReaderFactory,
+  fetchBuiltInCMapHelper,
+} from "./test_utils.js";
 import { Name } from "../../src/core/primitives.js";
 import { StringStream } from "../../src/core/stream.js";
 
 describe("cmap", function () {
   let fetchBuiltInCMap;
 
   beforeAll(function () {
-    // Allow CMap testing in Node.js, e.g. for Travis.
     const CMapReaderFactory = new DefaultCMapReaderFactory({
       baseUrl: CMAP_URL,
     });
 
-    fetchBuiltInCMap = function (name) {
-      return CMapReaderFactory.fetch({
-        name,
-      });
-    };
+    fetchBuiltInCMap = name =>
+      fetchBuiltInCMapHelper(CMapReaderFactory, /* cMapPacked = */ true, name);
   });
 
   afterAll(function () {
@@ -206,7 +206,11 @@ describe("cmap", function () {
   it("attempts to load a built-in CMap without the necessary API parameters", async function () {
     function tmpFetchBuiltInCMap(name) {
       const CMapReaderFactory = new DefaultCMapReaderFactory({});
-      return CMapReaderFactory.fetch({ name });
+      return fetchBuiltInCMapHelper(
+        CMapReaderFactory,
+        /* cMapPacked = */ true,
+        name
+      );
     }
 
     try {
@@ -221,7 +225,7 @@ describe("cmap", function () {
     } catch (reason) {
       expect(reason).toBeInstanceOf(Error);
       expect(reason.message).toEqual(
-        "Ensure that the `cMapUrl` and `cMapPacked` API parameters are provided."
+        "Ensure that the `cMapUrl` API parameter is provided."
       );
     }
   });
@@ -230,9 +234,12 @@ describe("cmap", function () {
     function tmpFetchBuiltInCMap(name) {
       const CMapReaderFactory = new DefaultCMapReaderFactory({
         baseUrl: CMAP_URL,
-        isCompressed: false,
       });
-      return CMapReaderFactory.fetch({ name });
+      return fetchBuiltInCMapHelper(
+        CMapReaderFactory,
+        /* cMapPacked = */ false,
+        name
+      );
     }
 
     try {

test/unit/test_utils.js

@@ -55,6 +55,15 @@ const DefaultStandardFontDataFactory =
     ? NodeStandardFontDataFactory
     : DOMStandardFontDataFactory;
 
+async function fetchBuiltInCMapHelper(cMapReaderFactory, cMapPacked, name) {
+  return {
+    cMapData: await cMapReaderFactory.fetch({
+      filename: `${name}${cMapPacked ? ".bcmap" : ""}`,
+    }),
+    isCompressed: cMapPacked,
+  };
+}
+
 function buildGetDocumentParams(filename, options) {
   const params = Object.create(null);
   params.url = isNodeJS
@@ -252,6 +261,7 @@ export {
   DefaultCMapReaderFactory,
   DefaultFileReaderFactory,
   DefaultStandardFontDataFactory,
+  fetchBuiltInCMapHelper,
   getCrossOriginHostname,
   STANDARD_FONT_DATA_URL,
   TEST_PDFS_PATH,