Skip to content

Commit

Permalink
[api-minor] Replace the PromiseCapability with `Promise.withResolve…
Browse files Browse the repository at this point in the history
…rs()`

This replaces our custom `PromiseCapability`-class with the new native `Promise.withResolvers()` functionality, which does *almost* the same thing[1]; please see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers

The only difference is that `PromiseCapability` also had a `settled`-getter, which was however not widely used and the call-sites can either be removed or re-factored to avoid it. In particular:
 - In `src/display/api.js` we can tweak the `PDFObjects`-class to use a "special" initial data-value and just compare against that, in order to replace the `settled`-state.
 - In `web/app.js` we change the only case to manually track the `settled`-state, which should hopefully be OK given how this is being used.
 - In `web/pdf_outline_viewer.js` we can remove the `settled`-checks, since the code should work just fine without it. The only thing that could potentially happen is that we try to `resolve` a Promise multiple times, which is however *not* a problem since the value of a Promise cannot be changed once fulfilled or rejected.
 - In `web/pdf_viewer.js` we can remove the `settled`-checks, since the code should work fine without them:
     - For the `_onePageRenderedCapability` case the `settled`-check is used in a `EventBus`-listener which is *removed* on its first (valid) invocation.
     - For the `_pagesCapability` case the `settled`-check is used in a print-related helper that works just fine with "only" the other checks.
 - In `test/unit/api_spec.js` we can change the few relevant cases to manually track the `settled`-state, since this is both simple and *test-only* code.

---
[1] In browsers/environments that lack native support, note [the compatibility data](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers#browser_compatibility), it'll be polyfilled via the `core-js` library (but only in `legacy` builds).
  • Loading branch information
Snuffleupagus committed Mar 29, 2024
1 parent 55db439 commit d7f45fe
Show file tree
Hide file tree
Showing 27 changed files with 130 additions and 224 deletions.
26 changes: 14 additions & 12 deletions gulpfile.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ const AUTOPREFIXER_CONFIG = {
// Default Babel targets used for generic, components, minified-pre
const BABEL_TARGETS = ENV_TARGETS.join(", ");

const BABEL_PRESET_ENV_OPTS = Object.freeze({
corejs: "3.36.1",
exclude: ["web.structured-clone"],
shippedProposals: true,
useBuiltIns: "usage",
});

const DEFINES = Object.freeze({
SKIP_BABEL: true,
TESTING: undefined,
Expand Down Expand Up @@ -303,17 +310,7 @@ function createWebpackConfig(

const babelPresets = skipBabel
? undefined
: [
[
"@babel/preset-env",
{
corejs: "3.36.1",
exclude: ["web.structured-clone"],
shippedProposals: true,
useBuiltIns: "usage",
},
],
];
: [["@babel/preset-env", BABEL_PRESET_ENV_OPTS]];
const babelPlugins = [
[
babelPluginPDFJSPreprocessor,
Expand Down Expand Up @@ -1542,7 +1539,12 @@ function buildLibHelper(bundleDefines, inputStream, outputDir) {
sourceType: "module",
presets: skipBabel
? undefined
: [["@babel/preset-env", { loose: false, modules: false }]],
: [
[
"@babel/preset-env",
{ loose: false, modules: false, ...BABEL_PRESET_ENV_OPTS },
],
],
plugins: [[babelPluginPDFJSPreprocessor, ctx]],
targets: BABEL_TARGETS,
}).code;
Expand Down
6 changes: 3 additions & 3 deletions src/core/chunked_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/

import { arrayBuffersToBytes, MissingDataException } from "./core_utils.js";
import { assert, PromiseCapability } from "../shared/util.js";
import { assert } from "../shared/util.js";
import { Stream } from "./stream.js";

class ChunkedStream extends Stream {
Expand Down Expand Up @@ -273,7 +273,7 @@ class ChunkedStreamManager {
this.progressiveDataLength = 0;
this.aborted = false;

this._loadedStreamCapability = new PromiseCapability();
this._loadedStreamCapability = Promise.withResolvers();
}

sendRequest(begin, end) {
Expand Down Expand Up @@ -347,7 +347,7 @@ class ChunkedStreamManager {
return Promise.resolve();
}

const capability = new PromiseCapability();
const capability = Promise.withResolvers();
this._promisesByRequest.set(requestId, capability);

const chunksToRequest = [];
Expand Down
15 changes: 7 additions & 8 deletions src/core/evaluator.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
isArrayEqual,
normalizeUnicode,
OPS,
PromiseCapability,
shadow,
stringToPDFString,
TextRenderingMode,
Expand Down Expand Up @@ -1270,7 +1269,7 @@ class PartialEvaluator {
return this.fontCache.get(font.cacheKey);
}

const fontCapability = new PromiseCapability();
const { promise, resolve } = Promise.withResolvers();

let preEvaluatedFont;
try {
Expand Down Expand Up @@ -1328,10 +1327,10 @@ class PartialEvaluator {
// keys. Also, since `fontRef` is used when getting cached fonts,
// we'll not accidentally match fonts cached with the `fontID`.
if (fontRefIsRef) {
this.fontCache.put(fontRef, fontCapability.promise);
this.fontCache.put(fontRef, promise);
} else {
font.cacheKey = `cacheKey_${fontID}`;
this.fontCache.put(font.cacheKey, fontCapability.promise);
this.fontCache.put(font.cacheKey, promise);
}

// Keep track of each font we translated so the caller can
Expand All @@ -1340,7 +1339,7 @@ class PartialEvaluator {

this.translateFont(preEvaluatedFont)
.then(translatedFont => {
fontCapability.resolve(
resolve(
new TranslatedFont({
loadedName: font.loadedName,
font: translatedFont,
Expand All @@ -1350,10 +1349,10 @@ class PartialEvaluator {
);
})
.catch(reason => {
// TODO fontCapability.reject?
// TODO reject?
warn(`loadFont - translateFont failed: "${reason}".`);

fontCapability.resolve(
resolve(
new TranslatedFont({
loadedName: font.loadedName,
font: new ErrorFont(
Expand All @@ -1364,7 +1363,7 @@ class PartialEvaluator {
})
);
});
return fontCapability.promise;
return promise;
}

buildPath(operatorList, fn, args, parsingText = false) {
Expand Down
5 changes: 2 additions & 3 deletions src/core/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
isNodeJS,
MissingPDFException,
PasswordException,
PromiseCapability,
setVerbosityLevel,
stringToPDFString,
UnexpectedResponseException,
Expand All @@ -48,7 +47,7 @@ class WorkerTask {
constructor(name) {
this.name = name;
this.terminated = false;
this._capability = new PromiseCapability();
this._capability = Promise.withResolvers();
}

get finished() {
Expand Down Expand Up @@ -212,7 +211,7 @@ class WorkerMessageHandler {
password,
rangeChunkSize,
};
const pdfManagerCapability = new PromiseCapability();
const pdfManagerCapability = Promise.withResolvers();
let newPdfManager;

if (data) {
Expand Down
39 changes: 20 additions & 19 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
MAX_IMAGE_SIZE_TO_CACHE,
MissingPDFException,
PasswordException,
PromiseCapability,
RenderingIntentFlag,
setVerbosityLevel,
shadow,
Expand Down Expand Up @@ -577,7 +576,7 @@ class PDFDocumentLoadingTask {
static #docId = 0;

constructor() {
this._capability = new PromiseCapability();
this._capability = Promise.withResolvers();
this._transport = null;
this._worker = null;

Expand Down Expand Up @@ -674,7 +673,7 @@ class PDFDataRangeTransport {
this._progressListeners = [];
this._progressiveReadListeners = [];
this._progressiveDoneListeners = [];
this._readyCapability = new PromiseCapability();
this._readyCapability = Promise.withResolvers();
}

/**
Expand Down Expand Up @@ -1461,7 +1460,7 @@ class PDFPageProxy {
// If there's no displayReadyCapability yet, then the operatorList
// was never requested before. Make the request and create the promise.
if (!intentState.displayReadyCapability) {
intentState.displayReadyCapability = new PromiseCapability();
intentState.displayReadyCapability = Promise.withResolvers();
intentState.operatorList = {
fnArray: [],
argsArray: [],
Expand Down Expand Up @@ -1588,7 +1587,7 @@ class PDFPageProxy {
if (!intentState.opListReadCapability) {
opListTask = Object.create(null);
opListTask.operatorListChanged = operatorListChanged;
intentState.opListReadCapability = new PromiseCapability();
intentState.opListReadCapability = Promise.withResolvers();
(intentState.renderTasks ||= new Set()).add(opListTask);
intentState.operatorList = {
fnArray: [],
Expand Down Expand Up @@ -2049,7 +2048,7 @@ class PDFWorker {
this.destroyed = false;
this.verbosity = verbosity;

this._readyCapability = new PromiseCapability();
this._readyCapability = Promise.withResolvers();
this._port = null;
this._webWorker = null;
this._messageHandler = null;
Expand Down Expand Up @@ -2365,7 +2364,7 @@ class WorkerTransport {
this._networkStream = networkStream;
this._fullReader = null;
this._lastProgress = null;
this.downloadInfoCapability = new PromiseCapability();
this.downloadInfoCapability = Promise.withResolvers();

this.setupMessageHandler();

Expand Down Expand Up @@ -2471,7 +2470,7 @@ class WorkerTransport {
}

this.destroyed = true;
this.destroyCapability = new PromiseCapability();
this.destroyCapability = Promise.withResolvers();

this.#passwordCapability?.reject(
new Error("Worker was destroyed during onPassword callback")
Expand Down Expand Up @@ -2562,7 +2561,7 @@ class WorkerTransport {
});

messageHandler.on("ReaderHeadersReady", data => {
const headersCapability = new PromiseCapability();
const headersCapability = Promise.withResolvers();
const fullReader = this._fullReader;
fullReader.headersReady.then(() => {
// If stream or range are disabled, it's our only way to report
Expand Down Expand Up @@ -2677,7 +2676,7 @@ class WorkerTransport {
});

messageHandler.on("PasswordRequest", exception => {
this.#passwordCapability = new PromiseCapability();
this.#passwordCapability = Promise.withResolvers();

if (loadingTask.onPassword) {
const updatePassword = password => {
Expand Down Expand Up @@ -3089,6 +3088,8 @@ class WorkerTransport {
}
}

const INITIAL_DATA = Symbol("INITIAL_DATA");

/**
* A PDF document and page is built of many objects. E.g. there are objects for
* fonts, images, rendering code, etc. These objects may get processed inside of
Expand All @@ -3105,8 +3106,8 @@ class PDFObjects {
*/
#ensureObj(objId) {
return (this.#objs[objId] ||= {
capability: new PromiseCapability(),
data: null,
...Promise.withResolvers(),
data: INITIAL_DATA,
});
}

Expand All @@ -3127,15 +3128,15 @@ class PDFObjects {
// not required to be resolved right now.
if (callback) {
const obj = this.#ensureObj(objId);
obj.capability.promise.then(() => callback(obj.data));
obj.promise.then(() => callback(obj.data));
return null;
}
// If there isn't a callback, the user expects to get the resolved data
// directly.
const obj = this.#objs[objId];
// If there isn't an object yet or the object isn't resolved, then the
// data isn't ready yet!
if (!obj?.capability.settled) {
if (!obj || obj.data === INITIAL_DATA) {
throw new Error(`Requesting object that isn't resolved yet ${objId}.`);
}
return obj.data;
Expand All @@ -3147,7 +3148,7 @@ class PDFObjects {
*/
has(objId) {
const obj = this.#objs[objId];
return obj?.capability.settled ?? false;
return !!obj && obj.data !== INITIAL_DATA;
}

/**
Expand All @@ -3159,7 +3160,7 @@ class PDFObjects {
resolve(objId, data = null) {
const obj = this.#ensureObj(objId);
obj.data = data;
obj.capability.resolve();
obj.resolve();
}

clear() {
Expand All @@ -3172,9 +3173,9 @@ class PDFObjects {

*[Symbol.iterator]() {
for (const objId in this.#objs) {
const { capability, data } = this.#objs[objId];
const { data } = this.#objs[objId];

if (!capability.settled) {
if (data === INITIAL_DATA) {
continue;
}
yield [objId, data];
Expand Down Expand Up @@ -3283,7 +3284,7 @@ class InternalRenderTask {
this._useRequestAnimationFrame =
useRequestAnimationFrame === true && typeof window !== "undefined";
this.cancelled = false;
this.capability = new PromiseCapability();
this.capability = Promise.withResolvers();
this.task = new RenderTask(this);
// caching this-bound methods
this._cancelBound = this.cancel.bind(this);
Expand Down
11 changes: 3 additions & 8 deletions src/display/fetch_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,7 @@
* limitations under the License.
*/

import {
AbortException,
assert,
PromiseCapability,
warn,
} from "../shared/util.js";
import { AbortException, assert, warn } from "../shared/util.js";
import {
createResponseStatusError,
extractFilenameFromHeader,
Expand Down Expand Up @@ -118,7 +113,7 @@ class PDFFetchStreamReader {
const source = stream.source;
this._withCredentials = source.withCredentials || false;
this._contentLength = source.length;
this._headersCapability = new PromiseCapability();
this._headersCapability = Promise.withResolvers();
this._disableRange = source.disableRange || false;
this._rangeChunkSize = source.rangeChunkSize;
if (!this._rangeChunkSize && !this._disableRange) {
Expand Down Expand Up @@ -223,7 +218,7 @@ class PDFFetchStreamRangeReader {
this._loaded = 0;
const source = stream.source;
this._withCredentials = source.withCredentials || false;
this._readCapability = new PromiseCapability();
this._readCapability = Promise.withResolvers();
this._isStreamingSupported = !source.disableStream;

this._abortController = new AbortController();
Expand Down
8 changes: 4 additions & 4 deletions src/display/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

import { assert, PromiseCapability, stringToBytes } from "../shared/util.js";
import { assert, stringToBytes } from "../shared/util.js";
import {
createResponseStatusError,
extractFilenameFromHeader,
Expand Down Expand Up @@ -255,7 +255,7 @@ class PDFNetworkStreamFullRequestReader {
};
this._url = source.url;
this._fullRequestId = manager.requestFull(args);
this._headersReceivedCapability = new PromiseCapability();
this._headersReceivedCapability = Promise.withResolvers();
this._disableRange = source.disableRange || false;
this._contentLength = source.length; // Optional
this._rangeChunkSize = source.rangeChunkSize;
Expand Down Expand Up @@ -375,7 +375,7 @@ class PDFNetworkStreamFullRequestReader {
if (this._done) {
return { value: undefined, done: true };
}
const requestCapability = new PromiseCapability();
const requestCapability = Promise.withResolvers();
this._requests.push(requestCapability);
return requestCapability.promise;
}
Expand Down Expand Up @@ -466,7 +466,7 @@ class PDFNetworkStreamRangeRequestReader {
if (this._done) {
return { value: undefined, done: true };
}
const requestCapability = new PromiseCapability();
const requestCapability = Promise.withResolvers();
this._requests.push(requestCapability);
return requestCapability.promise;
}
Expand Down
Loading

0 comments on commit d7f45fe

Please sign in to comment.