Skip to content

Commit

Permalink
Merge pull request #11985 from Snuffleupagus/rm-isEmptyObj
Browse files Browse the repository at this point in the history
Convert some `Object`s to `Map`s in `ChunkedStreamManager`, and move the `isEmptyObj` helper function to the test utils
  • Loading branch information
timvandermeij committed Jun 9, 2020
2 parents a4fa455 + 88fdb48 commit a327f38
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 45 deletions.
54 changes: 29 additions & 25 deletions src/core/chunked_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {
arrayByteLength,
arraysToBytes,
createPromiseCapability,
isEmptyObj,
} from "../shared/util.js";
import { MissingDataException } from "./core_utils.js";

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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();
}

Expand All @@ -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);
}
}
}
Expand Down
8 changes: 0 additions & 8 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down Expand Up @@ -931,7 +924,6 @@ export {
isArrayBuffer,
isArrayEqual,
isBool,
isEmptyObj,
isNum,
isString,
isSameOrigin,
Expand Down
2 changes: 1 addition & 1 deletion test/unit/metadata_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
9 changes: 9 additions & 0 deletions test/unit/test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -184,4 +192,5 @@ export {
buildGetDocumentParams,
TEST_PDFS_PATH,
createIdFactory,
isEmptyObj,
};
11 changes: 0 additions & 11 deletions test/unit/util_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
createValidAbsoluteUrl,
isArrayBuffer,
isBool,
isEmptyObj,
isNum,
isSameOrigin,
isString,
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a327f38

Please sign in to comment.