Merge pull request #11985 from Snuffleupagus/rm-isEmptyObj

Convert some `Object`s to `Map`s in `ChunkedStreamManager`, and move the `isEmptyObj` helper function to the test utils

src/core/chunked_stream.js

@@ -18,7 +18,6 @@ import {
   arrayByteLength,
   arraysToBytes,
   createPromiseCapability,
-  isEmptyObj,
 } from "../shared/util.js";
 import { MissingDataException } from "./core_utils.js";
 
@@ -319,9 +318,9 @@ class ChunkedStreamManager {
 
     this.currRequestId = 0;
 
-    this.chunksNeededByRequest = Object.create(null);
-    this.requestsByChunk = Object.create(null);
-    this.promisesByRequest = Object.create(null);
+    this._chunksNeededByRequest = new Map();
+    this._requestsByChunk = new Map();
+    this._promisesByRequest = new Map();
     this.progressiveDataLength = 0;
     this.aborted = false;
 
@@ -384,29 +383,31 @@ class ChunkedStreamManager {
   _requestChunks(chunks) {
     const requestId = this.currRequestId++;
 
-    const chunksNeeded = Object.create(null);
-    this.chunksNeededByRequest[requestId] = chunksNeeded;
+    const chunksNeeded = new Set();
+    this._chunksNeededByRequest.set(requestId, chunksNeeded);
     for (const chunk of chunks) {
       if (!this.stream.hasChunk(chunk)) {
-        chunksNeeded[chunk] = true;
+        chunksNeeded.add(chunk);
       }
     }
 
-    if (isEmptyObj(chunksNeeded)) {
+    if (chunksNeeded.size === 0) {
       return Promise.resolve();
     }
 
     const capability = createPromiseCapability();
-    this.promisesByRequest[requestId] = capability;
+    this._promisesByRequest.set(requestId, capability);
 
     const chunksToRequest = [];
-    for (let chunk in chunksNeeded) {
-      chunk = chunk | 0;
-      if (!(chunk in this.requestsByChunk)) {
-        this.requestsByChunk[chunk] = [];
+    for (const chunk of chunksNeeded) {
+      let requestIds = this._requestsByChunk.get(chunk);
+      if (!requestIds) {
+        requestIds = [];
+        this._requestsByChunk.set(chunk, requestIds);
+
         chunksToRequest.push(chunk);
       }
-      this.requestsByChunk[chunk].push(requestId);
+      requestIds.push(requestId);
     }
 
     if (!chunksToRequest.length) {
@@ -522,16 +523,19 @@ class ChunkedStreamManager {
     const loadedRequests = [];
     for (let curChunk = beginChunk; curChunk < endChunk; ++curChunk) {
       // The server might return more chunks than requested.
-      const requestIds = this.requestsByChunk[curChunk] || [];
-      delete this.requestsByChunk[curChunk];
+      const requestIds = this._requestsByChunk.get(curChunk);
+      if (!requestIds) {
+        continue;
+      }
+      this._requestsByChunk.delete(curChunk);
 
       for (const requestId of requestIds) {
-        const chunksNeeded = this.chunksNeededByRequest[requestId];
-        if (curChunk in chunksNeeded) {
-          delete chunksNeeded[curChunk];
+        const chunksNeeded = this._chunksNeededByRequest.get(requestId);
+        if (chunksNeeded.has(curChunk)) {
+          chunksNeeded.delete(curChunk);
         }
 
-        if (!isEmptyObj(chunksNeeded)) {
+        if (chunksNeeded.size > 0) {
           continue;
         }
         loadedRequests.push(requestId);
@@ -540,7 +544,7 @@ class ChunkedStreamManager {
 
     // If there are no pending requests, automatically fetch the next
     // unfetched chunk of the PDF file.
-    if (!this.disableAutoFetch && isEmptyObj(this.requestsByChunk)) {
+    if (!this.disableAutoFetch && this._requestsByChunk.size === 0) {
       let nextEmptyChunk;
       if (this.stream.numChunksLoaded === 1) {
         // This is a special optimization so that after fetching the first
@@ -559,8 +563,8 @@ class ChunkedStreamManager {
     }
 
     for (const requestId of loadedRequests) {
-      const capability = this.promisesByRequest[requestId];
-      delete this.promisesByRequest[requestId];
+      const capability = this._promisesByRequest.get(requestId);
+      this._promisesByRequest.delete(requestId);
       capability.resolve();
     }
 
@@ -587,8 +591,8 @@ class ChunkedStreamManager {
     if (this.pdfNetworkStream) {
       this.pdfNetworkStream.cancelAllRequests(reason);
     }
-    for (const requestId in this.promisesByRequest) {
-      this.promisesByRequest[requestId].reject(reason);
+    for (const [, capability] of this._promisesByRequest) {
+      capability.reject(reason);
     }
   }
 }

src/shared/util.js

@@ -793,13 +793,6 @@ function utf8StringToString(str) {
   return unescape(encodeURIComponent(str));
 }
 
-function isEmptyObj(obj) {
-  for (const key in obj) {
-    return false;
-  }
-  return true;
-}
-
 function isBool(v) {
   return typeof v === "boolean";
 }
@@ -931,7 +924,6 @@ export {
   isArrayBuffer,
   isArrayEqual,
   isBool,
-  isEmptyObj,
   isNum,
   isString,
   isSameOrigin,

test/unit/metadata_spec.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { isEmptyObj } from "../../src/shared/util.js";
+import { isEmptyObj } from "./test_utils.js";
 import { Metadata } from "../../src/display/metadata.js";
 
 describe("metadata", function () {

test/unit/test_utils.js

@@ -175,6 +175,14 @@ function createIdFactory(pageIndex) {
   return page.idFactory;
 }
 
+function isEmptyObj(obj) {
+  assert(
+    typeof obj === "object" && obj !== null,
+    "isEmptyObj - invalid argument."
+  );
+  return Object.keys(obj).length === 0;
+}
+
 export {
   DOMFileReaderFactory,
   NodeFileReaderFactory,
@@ -184,4 +192,5 @@ export {
   buildGetDocumentParams,
   TEST_PDFS_PATH,
   createIdFactory,
+  isEmptyObj,
 };

test/unit/util_spec.js

@@ -19,7 +19,6 @@ import {
   createValidAbsoluteUrl,
   isArrayBuffer,
   isBool,
-  isEmptyObj,
   isNum,
   isSameOrigin,
   isString,
@@ -89,16 +88,6 @@ describe("util", function () {
     });
   });
 
-  describe("isEmptyObj", function () {
-    it("handles empty objects", function () {
-      expect(isEmptyObj({})).toEqual(true);
-    });
-
-    it("handles non-empty objects", function () {
-      expect(isEmptyObj({ foo: "bar" })).toEqual(false);
-    });
-  });
-
   describe("isNum", function () {
     it("handles numeric values", function () {
       expect(isNum(1)).toEqual(true);