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
Snuffleupagus committed Apr 1, 2024
1 parent 55db439 commit e4d0e84
Show file tree
Hide file tree
Showing 28 changed files with 159 additions and 252 deletions.
26 changes: 14 additions & 12 deletions gulpfile.mjs
View file
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",
{ ...BABEL_PRESET_ENV_OPTS, loose: false, modules: false },
],
],
plugins: [[babelPluginPDFJSPreprocessor, ctx]],
targets: BABEL_TARGETS,
}).code;
Expand Down
6 changes: 3 additions & 3 deletions src/core/chunked_stream.js
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

1 comment on commit e4d0e84

@PatricheloM
Copy link

@PatricheloM PatricheloM commented on e4d0e84 Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just received a message from one of my PHP webapp's user that the page that uses the PDF.JS library does not show the PDF that it supposed to show on PROD.

Turns out, since this change in this commit is not in a release but has been rolled out to https://mozilla.github.io/pdf.js/build/pdf.mjs and I have been using this kind of import in my code: <script src="//mozilla.github.io/pdf.js/build/pdf.mjs" type="module"></script>, it broke the page for all my iOS Safari users who didn't update his or hers iOS to 17.4 (or can not update his phone since it is an old iPhone) because the Promise.withResolvers() function is only supported since iOS 17.4 (released March 5th. 2024). Function compatibility chart.

You can say it is my mistake that I didn't use a CDN URL for the source of the script since those only have official releases but I would like to point out that the examples page has the same example I used and copied above. (Interactive examples section).

I resolved the problem by downloading and self hosting an older version after 5 hours of investigation for the root of the issue.

Two suggestions:

  1. Modify the examples page to encourage users to use CDNs to prevent further problems like this.
  2. This change is also present in the LEGACY BUILD! (Legacy build) I would like to suggest the removal of the Promise.withResolvers() function from the legacy build since it is only supported from eg. iOS 17.4 and eg. Chrome 119. Edit: Just read the footnote in the commit message about polyfilling the function.

Please sign in to comment.