From 715b8aa3898abb550b3df361c303cebdca87e5a1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 4 Dec 2020 12:34:09 +0100 Subject: [PATCH 01/12] Move, and rename, the `src/scripting_api/quickjs-sandbox.js` file to `src/pdf.sandbox.js` The current location feels somewhat strange, and also inconsistent with the existing way that bundling is done. Finally, add the version/build numbers at the top of the *built* `pdf.sandbox.js` files, since all other built files include that information given that it's often helpful to be able to easily determine the *exact* version. --- gulpfile.js | 3 ++- src/{scripting_api/quickjs-sandbox.js => pdf.sandbox.js} | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) rename src/{scripting_api/quickjs-sandbox.js => pdf.sandbox.js} (91%) diff --git a/gulpfile.js b/gulpfile.js index 8e95a5257..b1bf1e5d3 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -349,6 +349,7 @@ function createScriptingBundle(defines) { function createSandboxBundle(defines, code) { var sandboxAMDName = "pdfjs-dist/build/pdf.sandbox"; var sandboxOutputName = "pdf.sandbox.js"; + var sandboxFileConfig = createWebpackConfig(defines, { filename: sandboxOutputName, library: sandboxAMDName, @@ -372,7 +373,7 @@ function createSandboxBundle(defines, code) { }); return ( gulp - .src("./src/scripting_api/quickjs-sandbox.js") + .src("./src/pdf.sandbox.js") .pipe(webpack2Stream(sandboxFileConfig)) .pipe(replaceWebpackRequire()) .pipe(replaceJSRootName(sandboxAMDName, "pdfjsSandbox")) diff --git a/src/scripting_api/quickjs-sandbox.js b/src/pdf.sandbox.js similarity index 91% rename from src/scripting_api/quickjs-sandbox.js rename to src/pdf.sandbox.js index cbf6e4e87..8bb6b37b8 100644 --- a/src/scripting_api/quickjs-sandbox.js +++ b/src/pdf.sandbox.js @@ -13,7 +13,12 @@ * limitations under the License. */ -import ModuleLoader from "../../external/quickjs/quickjs-eval.js"; +import ModuleLoader from "../external/quickjs/quickjs-eval.js"; + +/* eslint-disable-next-line no-unused-vars */ +const pdfjsVersion = PDFJSDev.eval("BUNDLE_VERSION"); +/* eslint-disable-next-line no-unused-vars */ +const pdfjsBuild = PDFJSDev.eval("BUNDLE_BUILD"); class Sandbox { constructor(module, testMode) { From d742e3cde82c9dd9f57df87c76d7b15617db16cf Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 4 Dec 2020 13:03:24 +0100 Subject: [PATCH 02/12] Actually utilize the PDF.js build-system fully when bundling the `pdf.sandbox.js` file There's no good reason, as far as I can tell, to use search-and-replace to include the *stringified* `pdf.scripting.js` file in the built `pdf.sandbox.js` file. Instead we could, and even should, utilize the existing `PDFJSDev.eval(...)`-functionality, which is not only simpler but will also be more efficient as well (no need for a regular expression). --- gulpfile.js | 36 +++++++++++------------------------- src/pdf.sandbox.js | 2 +- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index b1bf1e5d3..d2145cb93 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -98,6 +98,7 @@ const DEFINES = Object.freeze({ PRODUCTION: true, SKIP_BABEL: true, TESTING: false, + NO_SOURCE_MAP: false, // The main build targets: GENERIC: false, MOZCENTRAL: false, @@ -106,7 +107,6 @@ const DEFINES = Object.freeze({ COMPONENTS: false, LIB: false, IMAGE_DECODERS: false, - NO_SOURCE_MAP: false, }); function transform(charEncoding, transformFunction) { @@ -350,36 +350,22 @@ function createSandboxBundle(defines, code) { var sandboxAMDName = "pdfjs-dist/build/pdf.sandbox"; var sandboxOutputName = "pdf.sandbox.js"; - var sandboxFileConfig = createWebpackConfig(defines, { + // Insert the code as a string to be `eval`-ed in the sandbox. + const sandboxDefines = builder.merge(defines, { + PDF_SCRIPTING_JS_SOURCE: code, + }); + var sandboxFileConfig = createWebpackConfig(sandboxDefines, { filename: sandboxOutputName, library: sandboxAMDName, libraryTarget: "umd", umdNamedDefine: true, }); - // The code is the one from the bundle pdf.scripting.js - // so in order to have it in a string (which will be eval-ed - // in the sandbox) we must escape some chars. - // This way we've all the code (initialization+sandbox) in - // the same bundle. - code = code.replace(/["\\\n\t]/g, match => { - if (match === "\n") { - return "\\n"; - } - if (match === "\t") { - return "\\t"; - } - return `\\${match}`; - }); - return ( - gulp - .src("./src/pdf.sandbox.js") - .pipe(webpack2Stream(sandboxFileConfig)) - .pipe(replaceWebpackRequire()) - .pipe(replaceJSRootName(sandboxAMDName, "pdfjsSandbox")) - // put the code in a string to be eval-ed in the sandbox - .pipe(replace("/* INITIALIZATION_CODE */", `${code}`)) - ); + return gulp + .src("./src/pdf.sandbox.js") + .pipe(webpack2Stream(sandboxFileConfig)) + .pipe(replaceWebpackRequire()) + .pipe(replaceJSRootName(sandboxAMDName, "pdfjsSandbox")); } function buildSandbox(defines, dir) { diff --git a/src/pdf.sandbox.js b/src/pdf.sandbox.js index 8bb6b37b8..fe1f93dfc 100644 --- a/src/pdf.sandbox.js +++ b/src/pdf.sandbox.js @@ -48,7 +48,7 @@ class Sandbox { "module = Object.create(null);", // Next line is replaced by code from initialization.js // when we create the bundle for the sandbox. - "/* INITIALIZATION_CODE */", + PDFJSDev.eval("PDF_SCRIPTING_JS_SOURCE"), `data = ${sandboxData};`, `module.exports.initSandbox({ data, extra: {${extraStr}}, out: this});`, "delete exports;", From 1f29d274743ba19b0770b522ef7a6c893947eaf9 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 4 Dec 2020 15:49:13 +0100 Subject: [PATCH 03/12] Change how we're passing `pdf.sandbox.js`-specific options to `createWebpackConfig` in `gulpfile.js` Given the somewhat "specialized" nature of the `pdf.sandbox.js` building, it ought to be possible to re-factor how some of the options are handled. Note in particular that the `gulp-strip-comments` dependency seems somewhat unncessary, since the *main* source of comments are just the default license header. Hence I seems much more reasonable to simply not include that to begin with, rather than removing it after the fact (the few remaining Webpack-related should be few/small enough to not really matter much in practice). This way we're able to further reduce the special-casing related to the `pdf.sandbox.js`-building, which will make future changes/maintenance easier by bringing this code more in-line with existing patterns in `gulpfile.js`. (If we really want to reduce the filesize, we might want to consider always minifying the `GENERIC`-build of the `pdf.sandbox.js` file.) --- gulpfile.js | 46 +++++++++++++-------- package-lock.json | 100 ---------------------------------------------- package.json | 1 - 3 files changed, 29 insertions(+), 118 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index d2145cb93..50edc8979 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -33,7 +33,6 @@ var stream = require("stream"); var exec = require("child_process").exec; var spawn = require("child_process").spawn; var spawnSync = require("child_process").spawnSync; -var stripComments = require("gulp-strip-comments"); var streamqueue = require("streamqueue"); var merge = require("merge-stream"); var zip = require("gulp-zip"); @@ -98,7 +97,6 @@ const DEFINES = Object.freeze({ PRODUCTION: true, SKIP_BABEL: true, TESTING: false, - NO_SOURCE_MAP: false, // The main build targets: GENERIC: false, MOZCENTRAL: false, @@ -171,7 +169,11 @@ function createStringSource(filename, content) { return source; } -function createWebpackConfig(defines, output) { +function createWebpackConfig( + defines, + output, + { disableSourceMaps = false, disableLicenseHeader = false } = {} +) { var versionInfo = getVersionJSON(); var bundleDefines = builder.merge(defines, { BUNDLE_VERSION: versionInfo.version, @@ -185,7 +187,7 @@ function createWebpackConfig(defines, output) { !bundleDefines.MOZCENTRAL && !bundleDefines.CHROME && !bundleDefines.TESTING && - !bundleDefines.NO_SOURCE_MAP; + !disableSourceMaps; var skipBabel = bundleDefines.SKIP_BABEL; // `core-js` (see https://github.com/zloirock/core-js/issues/514), @@ -201,6 +203,13 @@ function createWebpackConfig(defines, output) { } const babelExcludeRegExp = new RegExp(`(${babelExcludes.join("|")})`); + const plugins = []; + if (!disableLicenseHeader) { + plugins.push( + new webpack2.BannerPlugin({ banner: licenseHeaderLibre, raw: true }) + ); + } + // Required to expose e.g., the `window` object. output.globalObject = "this"; @@ -210,9 +219,7 @@ function createWebpackConfig(defines, output) { performance: { hints: false, // Disable messages about larger file sizes. }, - plugins: [ - new webpack2.BannerPlugin({ banner: licenseHeaderLibre, raw: true }), - ], + plugins, resolve: { alias: { pdfjs: path.join(__dirname, "src"), @@ -329,16 +336,20 @@ function createMainBundle(defines) { .pipe(replaceJSRootName(mainAMDName, "pdfjsLib")); } -function createScriptingBundle(defines) { +function createScriptingBundle(defines, extraOptions = undefined) { var scriptingAMDName = "pdfjs-dist/build/pdf.scripting"; var scriptingOutputName = "pdf.scripting.js"; - var scriptingFileConfig = createWebpackConfig(defines, { - filename: scriptingOutputName, - library: scriptingAMDName, - libraryTarget: "umd", - umdNamedDefine: true, - }); + var scriptingFileConfig = createWebpackConfig( + defines, + { + filename: scriptingOutputName, + library: scriptingAMDName, + libraryTarget: "umd", + umdNamedDefine: true, + }, + extraOptions + ); return gulp .src("./src/pdf.scripting.js") .pipe(webpack2Stream(scriptingFileConfig)) @@ -369,9 +380,10 @@ function createSandboxBundle(defines, code) { } function buildSandbox(defines, dir) { - const scriptingDefines = builder.merge(defines, { NO_SOURCE_MAP: true }); - return createScriptingBundle(scriptingDefines) - .pipe(stripComments()) + return createScriptingBundle(defines, { + disableSourceMaps: true, + disableLicenseHeader: true, + }) .pipe(gulp.dest(dir + "build")) .on("data", file => { const content = file.contents.toString(); diff --git a/package-lock.json b/package-lock.json index c3f80c08a..973b04ba7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2178,15 +2178,6 @@ "ansi-wrap": "^0.1.0" } }, - "ansi-cyan": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/ansi-cyan/-/ansi-cyan-0.1.1.tgz", - "integrity": "sha1-U4rlKK+JgvKK4w2G8vF0VtJgmHM=", - "dev": true, - "requires": { - "ansi-wrap": "0.1.0" - } - }, "ansi-gray": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/ansi-gray/-/ansi-gray-0.1.1.tgz", @@ -2196,15 +2187,6 @@ "ansi-wrap": "0.1.0" } }, - "ansi-red": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/ansi-red/-/ansi-red-0.1.1.tgz", - "integrity": "sha1-jGOPnRCAgAo1PJwoyKgcpHBdlGw=", - "dev": true, - "requires": { - "ansi-wrap": "0.1.0" - } - }, "ansi-regex": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz", @@ -4028,15 +4010,6 @@ "integrity": "sha1-6zkTMzRYd1y4TNGh+uBiEGu4dUU=", "dev": true }, - "decomment": { - "version": "0.9.3", - "resolved": "https://registry.npmjs.org/decomment/-/decomment-0.9.3.tgz", - "integrity": "sha512-5skH5BfUL3n09RDmMVaHS1QGCiZRnl2nArUwmsE9JRY93Ueh3tihYl5wIrDdAuXnoFhxVis/DmRWREO2c6DG3w==", - "dev": true, - "requires": { - "esprima": "4.0.1" - } - }, "decompress-response": { "version": "4.2.1", "resolved": "https://registry.npmjs.org/decompress-response/-/decompress-response-4.2.1.tgz", @@ -7073,79 +7046,6 @@ "replacestream": "^4.0.0" } }, - "gulp-strip-comments": { - "version": "2.5.2", - "resolved": "https://registry.npmjs.org/gulp-strip-comments/-/gulp-strip-comments-2.5.2.tgz", - "integrity": "sha512-lb1bW7rsPWDD8f4ZPSguDvmCdjKmjr5HR4yZb9ros3sLl5AfW7oUj8KzY9/VRisT7dG8dL7hVHzNpQEVxfwZGQ==", - "dev": true, - "requires": { - "decomment": "^0.9.0", - "plugin-error": "^0.1.2", - "through2": "^2.0.3" - }, - "dependencies": { - "arr-diff": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/arr-diff/-/arr-diff-1.1.0.tgz", - "integrity": "sha1-aHwydYFjWI/vfeezb6vklesaOZo=", - "dev": true, - "requires": { - "arr-flatten": "^1.0.1", - "array-slice": "^0.2.3" - } - }, - "arr-union": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/arr-union/-/arr-union-2.1.0.tgz", - "integrity": "sha1-IPnqtexw9cfSFbEHexw5Fh0pLH0=", - "dev": true - }, - "array-slice": { - "version": "0.2.3", - "resolved": "https://registry.npmjs.org/array-slice/-/array-slice-0.2.3.tgz", - "integrity": "sha1-3Tz7gO15c6dRF82sabC5nshhhvU=", - "dev": true - }, - "extend-shallow": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/extend-shallow/-/extend-shallow-1.1.4.tgz", - "integrity": "sha1-Gda/lN/AnXa6cR85uHLSH/TdkHE=", - "dev": true, - "requires": { - "kind-of": "^1.1.0" - } - }, - "kind-of": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-1.1.0.tgz", - "integrity": "sha1-FAo9LUGjbS78+pN3tiwk+ElaXEQ=", - "dev": true - }, - "plugin-error": { - "version": "0.1.2", - "resolved": "https://registry.npmjs.org/plugin-error/-/plugin-error-0.1.2.tgz", - "integrity": "sha1-O5uzM1zPAPQl4HQ34ZJ2ln2kes4=", - "dev": true, - "requires": { - "ansi-cyan": "^0.1.1", - "ansi-red": "^0.1.1", - "arr-diff": "^1.0.1", - "arr-union": "^2.0.1", - "extend-shallow": "^1.1.2" - } - }, - "through2": { - "version": "2.0.5", - "resolved": "https://registry.npmjs.org/through2/-/through2-2.0.5.tgz", - "integrity": "sha512-/mrRod8xqpA+IHSLyGCQ2s8SPHiCDEeQJSep1jqLYeEUClOFG2Qsh+4FU6G9VeqpZnGW/Su8LQGc4YKni5rYSQ==", - "dev": true, - "requires": { - "readable-stream": "~2.3.6", - "xtend": "~4.0.1" - } - } - } - }, "gulp-zip": { "version": "5.0.2", "resolved": "https://registry.npmjs.org/gulp-zip/-/gulp-zip-5.0.2.tgz", diff --git a/package.json b/package.json index 0a58bbd5a..195f8f33b 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,6 @@ "gulp-postcss": "^9.0.0", "gulp-rename": "^2.0.0", "gulp-replace": "^1.0.0", - "gulp-strip-comments": "^2.5.2", "gulp-zip": "^5.0.2", "jasmine": "^3.6.3", "jsdoc": "^3.6.6", From 13d7244529873647d8e0122e744f496584d35023 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 5 Dec 2020 13:59:46 +0100 Subject: [PATCH 04/12] Re-factor how the `pdf.sandbox.js` file is built (PR 12604 follow-up) The way that the `pdf.sandbox.js` building was implemented feels all kinds of inconsistent/wrong, and it "sticks out" quite a bit when compared to the rest of the `gulpfile.js`. This patch thus attempts to improve the current situation slightly, to hopefully make future maintenance easier. One thing that strikes you, pretty immediately, when looking at PR 12604 is that the two new `gulp`-tasks added (i.e. `sandbox` and `watch-sandbox`) don't even work!? The reason for this is that they implicitly dependent upon the result of the `buildnumber`-task, which isn't listed as a dependency. (Try running `gulp clean` *first*, and invoking any of the new `gulp`-tasks will inevitably fail.) Furthermore, there's another (potentially big) problem with the implementation of e.g. the `gulp sandbox` task, since it doesn't actually wait for all building to complete before the task is considered as "done". This has the potential to cause all sorts of subtle bugs elsewhere, and the fact that things even "work" as-is can probably be attributed mostly to luck. Unfortunately there's no *perfect* way to improve things here, since the `pdf.sandbox.js` file depends on including the `pdf.scripting.js` file as a string, however I firmly believe that improvements are still possible here. To that end, this patch updates all relevant build-targets to create a *temporary* `pdf.scripting.js` file as part of the setup in the `gulp`-tasks, and then reads that file during the `pdf.sandbox.js` building. This at least allows us to bring all of this "sandbox"-build code much more in-line with the existing build-system. --- gulpfile.js | 263 ++++++++++++++++++++++++++-------------------------- 1 file changed, 129 insertions(+), 134 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index 50edc8979..9d3142c95 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -64,6 +64,7 @@ var SRC_DIR = "src/"; var LIB_DIR = BUILD_DIR + "lib/"; var DIST_DIR = BUILD_DIR + "dist/"; var TYPES_DIR = BUILD_DIR + "types/"; +const TMP_DIR = BUILD_DIR + "tmp/"; var TYPESTEST_DIR = BUILD_DIR + "typestest/"; var COMMON_WEB_FILES = ["web/images/*.{png,svg,gif,cur}", "web/debugger.js"]; var MOZCENTRAL_DIFF_FILE = "mozcentral.diff"; @@ -186,6 +187,7 @@ function createWebpackConfig( var enableSourceMaps = !bundleDefines.MOZCENTRAL && !bundleDefines.CHROME && + !bundleDefines.LIB && !bundleDefines.TESTING && !disableSourceMaps; var skipBabel = bundleDefines.SKIP_BABEL; @@ -357,20 +359,34 @@ function createScriptingBundle(defines, extraOptions = undefined) { .pipe(replaceJSRootName(scriptingAMDName, "pdfjsScripting")); } -function createSandboxBundle(defines, code) { +function createTemporaryScriptingBundle(defines) { + return createScriptingBundle(defines, { + disableSourceMaps: true, + disableLicenseHeader: true, + }).pipe(gulp.dest(TMP_DIR)); +} + +function createSandboxBundle(defines, extraOptions = undefined) { var sandboxAMDName = "pdfjs-dist/build/pdf.sandbox"; var sandboxOutputName = "pdf.sandbox.js"; - // Insert the code as a string to be `eval`-ed in the sandbox. + const scriptingPath = TMP_DIR + "pdf.scripting.js"; + // Insert the source as a string to be `eval`-ed in the sandbox. const sandboxDefines = builder.merge(defines, { - PDF_SCRIPTING_JS_SOURCE: code, - }); - var sandboxFileConfig = createWebpackConfig(sandboxDefines, { - filename: sandboxOutputName, - library: sandboxAMDName, - libraryTarget: "umd", - umdNamedDefine: true, + PDF_SCRIPTING_JS_SOURCE: fs.readFileSync(scriptingPath).toString(), }); + fs.unlinkSync(scriptingPath); + + var sandboxFileConfig = createWebpackConfig( + sandboxDefines, + { + filename: sandboxOutputName, + library: sandboxAMDName, + libraryTarget: "umd", + umdNamedDefine: true, + }, + extraOptions + ); return gulp .src("./src/pdf.sandbox.js") @@ -379,19 +395,6 @@ function createSandboxBundle(defines, code) { .pipe(replaceJSRootName(sandboxAMDName, "pdfjsSandbox")); } -function buildSandbox(defines, dir) { - return createScriptingBundle(defines, { - disableSourceMaps: true, - disableLicenseHeader: true, - }) - .pipe(gulp.dest(dir + "build")) - .on("data", file => { - const content = file.contents.toString(); - createSandboxBundle(defines, content).pipe(gulp.dest(dir + "build")); - fs.unlinkSync(dir + "build/pdf.scripting.js"); - }); -} - function createWorkerBundle(defines) { var workerAMDName = "pdfjs-dist/build/pdf.worker"; var workerOutputName = "pdf.worker.js"; @@ -543,25 +546,6 @@ function makeRef(done, bot) { }); } -gulp.task("sandbox", function (done) { - const defines = builder.merge(DEFINES, { GENERIC: true }); - buildSandbox(defines, GENERIC_DIR); - done(); -}); - -gulp.task("watch-sandbox", function (done) { - const defines = builder.merge(DEFINES, { GENERIC: true }); - buildSandbox(defines, GENERIC_DIR); - const watcher = gulp.watch([ - "src/scripting_api/*.js", - "external/quickjs/*.js", - ]); - watcher.on("change", function () { - buildSandbox(defines, GENERIC_DIR); - }); - done(); -}); - gulp.task("default", function (done) { console.log("Available tasks:"); var tasks = Object.keys(gulp.registry().tasks()); @@ -797,6 +781,7 @@ function buildGeneric(defines, dir) { return merge([ createMainBundle(defines).pipe(gulp.dest(dir + "build")), createWorkerBundle(defines).pipe(gulp.dest(dir + "build")), + createSandboxBundle(defines).pipe(gulp.dest(dir + "build")), createWebBundle(defines).pipe(gulp.dest(dir + "web")), gulp.src(COMMON_WEB_FILES, { base: "web/" }).pipe(gulp.dest(dir + "web")), gulp.src("LICENSE").pipe(gulp.dest(dir)), @@ -835,14 +820,17 @@ gulp.task( "buildnumber", "default_preferences", "locale", + function scripting() { + var defines = builder.merge(DEFINES, { GENERIC: true }); + return createTemporaryScriptingBundle(defines); + }, function () { console.log(); console.log("### Creating generic viewer"); var defines = builder.merge(DEFINES, { GENERIC: true }); return buildGeneric(defines, GENERIC_DIR); - }, - "sandbox" + } ) ); @@ -854,6 +842,13 @@ gulp.task( "buildnumber", "default_preferences", "locale", + function scripting() { + var defines = builder.merge(DEFINES, { + GENERIC: true, + SKIP_BABEL: false, + }); + return createTemporaryScriptingBundle(defines); + }, function () { console.log(); console.log("### Creating generic (ES5) viewer"); @@ -863,13 +858,6 @@ gulp.task( }); return buildGeneric(defines, GENERIC_ES5_DIR); - }, - function () { - const defines = builder.merge(DEFINES, { - GENERIC: true, - SKIP_BABEL: false, - }); - return buildSandbox(defines, GENERIC_ES5_DIR); } ) ); @@ -963,6 +951,7 @@ function buildMinified(defines, dir) { return merge([ createMainBundle(defines).pipe(gulp.dest(dir + "build")), createWorkerBundle(defines).pipe(gulp.dest(dir + "build")), + createSandboxBundle(defines).pipe(gulp.dest(dir + "build")), createWebBundle(defines).pipe(gulp.dest(dir + "web")), createImageDecodersBundle( builder.merge(defines, { IMAGE_DECODERS: true }) @@ -1002,16 +991,15 @@ gulp.task( "buildnumber", "default_preferences", "locale", + function scripting() { + var defines = builder.merge(DEFINES, { MINIFIED: true, GENERIC: true }); + return createTemporaryScriptingBundle(defines); + }, function () { console.log(); console.log("### Creating minified viewer"); var defines = builder.merge(DEFINES, { MINIFIED: true, GENERIC: true }); - return buildSandbox(defines, MINIFIED_DIR); - }, - function () { - var defines = builder.merge(DEFINES, { MINIFIED: true, GENERIC: true }); - return buildMinified(defines, MINIFIED_DIR); } ) @@ -1023,19 +1011,17 @@ gulp.task( "buildnumber", "default_preferences", "locale", - function () { - console.log(); - console.log("### Creating minified (ES5) viewer"); + function scripting() { var defines = builder.merge(DEFINES, { MINIFIED: true, GENERIC: true, SKIP_BABEL: false, }); - - return buildSandbox(defines, MINIFIED_ES5_DIR); + return createTemporaryScriptingBundle(defines); }, - function () { + console.log(); + console.log("### Creating minified (ES5) viewer"); var defines = builder.merge(DEFINES, { MINIFIED: true, GENERIC: true, @@ -1237,80 +1223,85 @@ gulp.task("mozcentral", gulp.series("mozcentral-pre")); gulp.task( "chromium-pre", - gulp.series("buildnumber", "default_preferences", "locale", function () { - console.log(); - console.log("### Building Chromium extension"); - var defines = builder.merge(DEFINES, { CHROME: true, SKIP_BABEL: false }); + gulp.series( + "buildnumber", + "default_preferences", + "locale", + function scripting() { + var defines = builder.merge(DEFINES, { CHROME: true, SKIP_BABEL: false }); + return createTemporaryScriptingBundle(defines); + }, + function () { + console.log(); + console.log("### Building Chromium extension"); + var defines = builder.merge(DEFINES, { CHROME: true, SKIP_BABEL: false }); - var CHROME_BUILD_DIR = BUILD_DIR + "/chromium/", - CHROME_BUILD_CONTENT_DIR = CHROME_BUILD_DIR + "/content/"; + var CHROME_BUILD_DIR = BUILD_DIR + "/chromium/", + CHROME_BUILD_CONTENT_DIR = CHROME_BUILD_DIR + "/content/"; - // Clear out everything in the chrome extension build directory - rimraf.sync(CHROME_BUILD_DIR); + // Clear out everything in the chrome extension build directory + rimraf.sync(CHROME_BUILD_DIR); - var version = getVersionJSON().version; + var version = getVersionJSON().version; - return merge([ - createMainBundle(defines).pipe( - gulp.dest(CHROME_BUILD_CONTENT_DIR + "build") - ), - createWorkerBundle(defines).pipe( - gulp.dest(CHROME_BUILD_CONTENT_DIR + "build") - ), - createWebBundle(defines).pipe( - gulp.dest(CHROME_BUILD_CONTENT_DIR + "web") - ), - gulp - .src(COMMON_WEB_FILES, { base: "web/" }) - .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), + return merge([ + createMainBundle(defines).pipe( + gulp.dest(CHROME_BUILD_CONTENT_DIR + "build") + ), + createWorkerBundle(defines).pipe( + gulp.dest(CHROME_BUILD_CONTENT_DIR + "build") + ), + createSandboxBundle(defines).pipe( + gulp.dest(CHROME_BUILD_CONTENT_DIR + "build") + ), + createWebBundle(defines).pipe( + gulp.dest(CHROME_BUILD_CONTENT_DIR + "web") + ), + gulp + .src(COMMON_WEB_FILES, { base: "web/" }) + .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), - gulp - .src( - ["web/locale/*/viewer.properties", "web/locale/locale.properties"], - { base: "web/" } - ) - .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), - gulp - .src(["external/bcmaps/*.bcmap", "external/bcmaps/LICENSE"], { - base: "external/bcmaps", - }) - .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web/cmaps")), + gulp + .src( + ["web/locale/*/viewer.properties", "web/locale/locale.properties"], + { base: "web/" } + ) + .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), + gulp + .src(["external/bcmaps/*.bcmap", "external/bcmaps/LICENSE"], { + base: "external/bcmaps", + }) + .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web/cmaps")), - preprocessHTML("web/viewer.html", defines).pipe( - gulp.dest(CHROME_BUILD_CONTENT_DIR + "web") - ), - preprocessCSS("web/viewer.css", "chrome", defines, true) - .pipe( - postcss([autoprefixer({ overrideBrowserslist: ["chrome >= 49"] })]) - ) - .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), + preprocessHTML("web/viewer.html", defines).pipe( + gulp.dest(CHROME_BUILD_CONTENT_DIR + "web") + ), + preprocessCSS("web/viewer.css", "chrome", defines, true) + .pipe( + postcss([autoprefixer({ overrideBrowserslist: ["chrome >= 49"] })]) + ) + .pipe(gulp.dest(CHROME_BUILD_CONTENT_DIR + "web")), - gulp.src("LICENSE").pipe(gulp.dest(CHROME_BUILD_DIR)), - gulp - .src("extensions/chromium/manifest.json") - .pipe(replace(/\bPDFJSSCRIPT_VERSION\b/g, version)) - .pipe(gulp.dest(CHROME_BUILD_DIR)), - gulp - .src( - [ - "extensions/chromium/**/*.{html,js,css,png}", - "extensions/chromium/preferences_schema.json", - ], - { base: "extensions/chromium/" } - ) - .pipe(gulp.dest(CHROME_BUILD_DIR)), - ]); - }) + gulp.src("LICENSE").pipe(gulp.dest(CHROME_BUILD_DIR)), + gulp + .src("extensions/chromium/manifest.json") + .pipe(replace(/\bPDFJSSCRIPT_VERSION\b/g, version)) + .pipe(gulp.dest(CHROME_BUILD_DIR)), + gulp + .src( + [ + "extensions/chromium/**/*.{html,js,css,png}", + "extensions/chromium/preferences_schema.json", + ], + { base: "extensions/chromium/" } + ) + .pipe(gulp.dest(CHROME_BUILD_DIR)), + ]); + } + ) ); -gulp.task( - "chromium", - gulp.series("chromium-pre", function () { - var defines = builder.merge(DEFINES, { CHROME: true, SKIP_BABEL: false }); - var CHROME_BUILD_CONTENT_DIR = BUILD_DIR + "/chromium/content/"; - return buildSandbox(defines, CHROME_BUILD_CONTENT_DIR); - }) -); +gulp.task("chromium", gulp.series("chromium-pre")); gulp.task("jsdoc", function (done) { console.log(); @@ -1431,15 +1422,17 @@ gulp.task( gulp.series( "buildnumber", "default_preferences", - function () { + function scripting() { var defines = builder.merge(DEFINES, { GENERIC: true, LIB: true }); - - return buildLib(defines, "build/lib/"); + return createTemporaryScriptingBundle(defines); }, function () { var defines = builder.merge(DEFINES, { GENERIC: true, LIB: true }); - return buildSandbox(defines, "build/lib/"); + return merge([ + buildLib(defines, "build/lib/"), + createSandboxBundle(defines).pipe(gulp.dest("build/lib/")), + ]); } ) ); @@ -1449,14 +1442,13 @@ gulp.task( gulp.series( "buildnumber", "default_preferences", - function () { + function scripting() { var defines = builder.merge(DEFINES, { GENERIC: true, LIB: true, SKIP_BABEL: false, }); - - return buildLib(defines, "build/lib-es5/"); + return createTemporaryScriptingBundle(defines); }, function () { var defines = builder.merge(DEFINES, { @@ -1465,7 +1457,10 @@ gulp.task( SKIP_BABEL: false, }); - return buildSandbox(defines, "build/lib-es5/"); + return merge([ + buildLib(defines, "build/lib-es5/"), + createSandboxBundle(defines).pipe(gulp.dest("build/lib-es5/")), + ]); } ) ); From c39f1aedb2e2405c91528c23c852b0b3a682f64f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 5 Dec 2020 15:51:39 +0100 Subject: [PATCH 05/12] Re-implement working `dev-sandbox`/`watch-dev-sandbox` gulp-tasks Compared to the, previously removed, `sandbox`/`watch-sandbox` gulp-tasks, these ones should work even when run against an non-existent/empty `build`-folder. Also, to ensure that the development viewer actually works out-of-the-box, `gulp server` will now also include `gulp watch-dev-sandbox` to remove the need to *manually* invoke the build-tasks. Finally, this patch also removes the `web/devcom.js` file since it shouldn't actually be needed, assuming that the "sandbox"-loading code in the `web/genericcom.js` file is actually *correctly* implemented. --- gulpfile.js | 68 +++++++++++++++++++++++++++++++++++++++------- web/app_options.js | 16 +++++------ web/devcom.js | 43 ----------------------------- web/genericcom.js | 2 +- web/viewer.js | 3 -- 5 files changed, 67 insertions(+), 65 deletions(-) delete mode 100644 web/devcom.js diff --git a/gulpfile.js b/gulpfile.js index 9d3142c95..056cf2293 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -173,9 +173,15 @@ function createStringSource(filename, content) { function createWebpackConfig( defines, output, - { disableSourceMaps = false, disableLicenseHeader = false } = {} + { + disableVersionInfo = false, + disableSourceMaps = false, + disableLicenseHeader = false, + } = {} ) { - var versionInfo = getVersionJSON(); + const versionInfo = !disableVersionInfo + ? getVersionJSON() + : { version: 0, commit: 0 }; var bundleDefines = builder.merge(defines, { BUNDLE_VERSION: versionInfo.version, BUNDLE_BUILD: versionInfo.commit, @@ -359,8 +365,9 @@ function createScriptingBundle(defines, extraOptions = undefined) { .pipe(replaceJSRootName(scriptingAMDName, "pdfjsScripting")); } -function createTemporaryScriptingBundle(defines) { +function createTemporaryScriptingBundle(defines, extraOptions = undefined) { return createScriptingBundle(defines, { + disableVersionInfo: !!(extraOptions && extraOptions.disableVersionInfo), disableSourceMaps: true, disableLicenseHeader: true, }).pipe(gulp.dest(TMP_DIR)); @@ -1711,16 +1718,57 @@ gulp.task( }) ); -gulp.task("server", function () { - console.log(); - console.log("### Starting local server"); +gulp.task( + "dev-sandbox", + gulp.series( + function scripting() { + const defines = builder.merge(DEFINES, { GENERIC: true, TESTING: true }); + return createTemporaryScriptingBundle(defines, { + disableVersionInfo: true, + }); + }, + function () { + console.log(); + console.log("### Building development sandbox"); - var WebServer = require("./test/webserver.js").WebServer; - var server = new WebServer(); - server.port = 8888; - server.start(); + const defines = builder.merge(DEFINES, { GENERIC: true, TESTING: true }); + const sandboxDir = BUILD_DIR + "dev-sandbox/"; + + rimraf.sync(sandboxDir); + + return createSandboxBundle(defines, { + disableVersionInfo: true, + }).pipe(gulp.dest(sandboxDir)); + } + ) +); + +gulp.task("watch-dev-sandbox", function () { + gulp.watch( + [ + "src/pdf.{sandbox,scripting}.js", + "src/scripting_api/*.js", + "src/shared/scripting_utils.js", + "external/quickjs/*.js", + ], + { ignoreInitial: false }, + gulp.series("dev-sandbox") + ); }); +gulp.task( + "server", + gulp.parallel("watch-dev-sandbox", function () { + console.log(); + console.log("### Starting local server"); + + var WebServer = require("./test/webserver.js").WebServer; + var server = new WebServer(); + server.port = 8888; + server.start(); + }) +); + gulp.task("clean", function (done) { console.log(); console.log("### Cleaning up project builds"); diff --git a/web/app_options.js b/web/app_options.js index d13f2026d..caf2455ca 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -223,14 +223,6 @@ const defaultOptions = { value: false, kind: OptionKind.API, }, - scriptingSrc: { - /** @type {string} */ - value: - typeof PDFJSDev === "undefined" || !PDFJSDev.test("PRODUCTION") - ? "../build/generic/build/pdf.sandbox.js" - : "../build/pdf.sandbox.js", - kind: OptionKind.VIEWER, - }, verbosity: { /** @type {number} */ value: 1, @@ -265,6 +257,14 @@ if ( value: typeof navigator !== "undefined" ? navigator.language : "en-US", kind: OptionKind.VIEWER, }; + defaultOptions.sandboxBundleSrc = { + /** @type {string} */ + value: + typeof PDFJSDev === "undefined" || !PDFJSDev.test("PRODUCTION") + ? "../build/dev-sandbox/pdf.sandbox.js" + : "../build/pdf.sandbox.js", + kind: OptionKind.VIEWER, + }; } const userOptions = Object.create(null); diff --git a/web/devcom.js b/web/devcom.js deleted file mode 100644 index 3df13f604..000000000 --- a/web/devcom.js +++ /dev/null @@ -1,43 +0,0 @@ -/* Copyright 2017 Mozilla Foundation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; -import { loadScript, shadow } from "pdfjs-lib"; - -const DevCom = {}; - -class DevExternalServices extends DefaultExternalServices { - static get scripting() { - const promise = loadScript("../build/pdf.sandbox.js").then(() => { - return window.pdfjsSandbox.QuickJSSandbox(); - }); - const sandbox = { - createSandbox(data) { - promise.then(sbx => sbx.create(data)); - }, - dispatchEventInSandbox(event) { - promise.then(sbx => sbx.dispatchEvent(event)); - }, - destroySandbox() { - promise.then(sbx => sbx.nukeSandbox()); - }, - }; - - return shadow(this, "scripting", sandbox); - } -} -PDFViewerApplication.externalServices = DevExternalServices; - -export { DevCom }; diff --git a/web/genericcom.js b/web/genericcom.js index f1f7dba38..73f78b045 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -53,7 +53,7 @@ class GenericExternalServices extends DefaultExternalServices { } static get scripting() { - const promise = loadScript(AppOptions.get("scriptingSrc")).then(() => { + const promise = loadScript(AppOptions.get("sandboxBundleSrc")).then(() => { return window.pdfjsSandbox.QuickJSSandbox(); }); const sandbox = { diff --git a/web/viewer.js b/web/viewer.js index 1bc2b35a8..d447c962a 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -56,9 +56,6 @@ if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("GENERIC")) { if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("CHROME")) { require("./chromecom.js"); } -if (typeof PDFJSDev === "undefined") { - import("./devcom.js"); -} if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("CHROME || GENERIC")) { require("./pdf_print_service.js"); } From c549069ebdac524de182eafef5bf0e5e5589a6fa Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 5 Dec 2020 16:35:49 +0100 Subject: [PATCH 06/12] Replace the `testMode` parameter in `src/pdf.sandbox.js` with a constant, set using the pre-processor This simplifies not just this code, but the unit-tests as well, and should be sufficient as far as I can tell. Note also that currently, in the *built* `pdf.sandbox.js` file, there's even a line reading `testMode = testMode && false;` because of an accidentally flipped pre-processor statement. Finally, in the `scripting_spec.js` unit-test, defines `sandboxBundleSrc` at the top of the file to make it easier to find and/or change it when necessary. --- src/pdf.sandbox.js | 18 ++++++++---------- test/unit/scripting_spec.js | 10 +++++----- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/pdf.sandbox.js b/src/pdf.sandbox.js index fe1f93dfc..9dec2bc5f 100644 --- a/src/pdf.sandbox.js +++ b/src/pdf.sandbox.js @@ -20,15 +20,17 @@ const pdfjsVersion = PDFJSDev.eval("BUNDLE_VERSION"); /* eslint-disable-next-line no-unused-vars */ const pdfjsBuild = PDFJSDev.eval("BUNDLE_BUILD"); +const TESTING = + typeof PDFJSDev === "undefined" || PDFJSDev.test("!PRODUCTION || TESTING"); + class Sandbox { - constructor(module, testMode) { + constructor(module) { this._evalInSandbox = module.cwrap("evalInSandbox", null, [ "string", "int", ]); this._dispatchEventName = null; this._module = module; - this._testMode = testMode; this._alertOnError = 1; } @@ -55,7 +57,7 @@ class Sandbox { "delete module;", "delete data;", ]; - if (!this._testMode) { + if (!TESTING) { code = code.concat(extra.map(name => `delete ${name};`)); code.push("delete debugMe;"); } @@ -86,7 +88,7 @@ class Sandbox { } evalForTesting(code, key) { - if (this._testMode) { + if (TESTING) { this._evalInSandbox( `try { send({ id: "${key}", result: ${code} }); @@ -99,13 +101,9 @@ class Sandbox { } } -function QuickJSSandbox(testMode = false) { - testMode = - testMode && - (typeof PDFJSDev === "undefined" || - PDFJSDev.test("!PRODUCTION || TESTING")); +function QuickJSSandbox() { return ModuleLoader().then(module => { - return new Sandbox(module, testMode); + return new Sandbox(module); }); } diff --git a/test/unit/scripting_spec.js b/test/unit/scripting_spec.js index e4d6b1077..c8062b097 100644 --- a/test/unit/scripting_spec.js +++ b/test/unit/scripting_spec.js @@ -15,6 +15,8 @@ import { loadScript } from "../../src/display/display_utils.js"; +const sandboxBundleSrc = "../../build/generic/build/pdf.sandbox.js"; + describe("Scripting", function () { let sandbox, send_queue, test_id, ref; @@ -44,11 +46,9 @@ describe("Scripting", function () { send_queue.set(event.detail.id, event.detail); } }; - const promise = loadScript("../../build/generic/build/pdf.sandbox.js").then( - () => { - return window.pdfjsSandbox.QuickJSSandbox(true); - } - ); + const promise = loadScript(sandboxBundleSrc).then(() => { + return window.pdfjsSandbox.QuickJSSandbox(); + }); sandbox = { createSandbox(data) { promise.then(sbx => sbx.create(data)); From f8ea83609f8277b15bedeb1549f185dd2f596a75 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 12:30:33 +0100 Subject: [PATCH 07/12] Dispatch a `metadataloaded` event once the `PDFViewerApplication._initializeMetadata` method is done This will be useful in the following patch, and note that there's also an old issue (see 5765) which asked for such an event. However, given that the use-case wasn't *clearly* specified, and that we didn't have an internal use for it at the time it wasn't implemented. Also, ensure that all of the metadata-related properties are actually reset when the document is closed. --- web/app.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web/app.js b/web/app.js index 620c9c239..baacb1b0f 100644 --- a/web/app.js +++ b/web/app.js @@ -248,7 +248,9 @@ const PDFViewerApplication = { url: "", baseUrl: "", externalServices: DefaultExternalServices, - _boundEvents: {}, + _boundEvents: Object.create(null), + documentInfo: null, + metadata: null, contentDispositionFilename: null, triggerDelayedFallback: null, _saveInProgress: false, @@ -789,6 +791,8 @@ const PDFViewerApplication = { this.downloadComplete = false; this.url = ""; this.baseUrl = ""; + this.documentInfo = null; + this.metadata = null; this.contentDispositionFilename = null; this.triggerDelayedFallback = null; this._saveInProgress = false; @@ -1652,6 +1656,8 @@ const PDFViewerApplication = { generator: generatorId, formType, }); + + this.eventBus.dispatch("metadataloaded", { source: this }); }, /** From bfdb39a1e67447442d861feaf69e404084d24226 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 12:42:56 +0100 Subject: [PATCH 08/12] Stop re-fetching the `metadata` unconditionally in `PDFViewerApplication._initializeJavaScript` We can easily avoid unnecessary API-calls here, since most of the time the `metadata` will already be available here. In the *rare* case that it's not available, we can simply wait for the existing `getMetadata`-call to resolve. --- web/app.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/web/app.js b/web/app.js index baacb1b0f..67a6ff944 100644 --- a/web/app.js +++ b/web/app.js @@ -1417,11 +1417,21 @@ const PDFViewerApplication = { } const calculationOrder = await pdfDocument.getCalculationOrderIds(); const scripting = this.externalServices.scripting; - const { - info, - metadata, - contentDispositionFilename, - } = await pdfDocument.getMetadata(); + + if (!this.documentInfo) { + // It should be *extremely* rare for metadata to not have been resolved + // when this code runs, but ensure that we handle that case here. + await new Promise(resolve => { + const metadataLoaded = () => { + this.eventBus._off("metadataloaded", metadataLoaded); + resolve(); + }; + this.eventBus._on("metadataloaded", metadataLoaded); + }); + if (pdfDocument !== this.pdfDocument) { + return; // The document was closed while the metadata resolved. + } + } window.addEventListener("updateFromSandbox", event => { const detail = event.detail; @@ -1480,7 +1490,7 @@ const PDFViewerApplication = { const dispatchEventName = generateRandomStringForSandbox(objects); const { length } = await pdfDocument.getDownloadInfo(); const filename = - contentDispositionFilename || getPDFFileNameFromURL(this.url); + this.contentDispositionFilename || getPDFFileNameFromURL(this.url); scripting.createSandbox({ objects, dispatchEventName, @@ -1490,11 +1500,11 @@ const PDFViewerApplication = { language: navigator.language, }, docInfo: { - ...info, + ...this.documentInfo, baseURL: this.baseUrl, filesize: length, filename, - metadata, + metadata: this.metadata, numPages: pdfDocument.numPages, URL: this.url, }, From 1f2f8c907b193907a3f3e6ffe175ebc7dcc037cd Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 13:00:34 +0100 Subject: [PATCH 09/12] Tweak the "filesize" handling in `PDFViewerApplication._initializeJavaScript` Another possible option here could be to use the `contentLength`, when it exists, and then using e.g. a custom event to always update the "filesize" in the sandbox "after the fact" with the result of the `getDownloadInfo`-call. --- web/app.js | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/web/app.js b/web/app.js index 67a6ff944..8a78d128d 100644 --- a/web/app.js +++ b/web/app.js @@ -251,7 +251,8 @@ const PDFViewerApplication = { _boundEvents: Object.create(null), documentInfo: null, metadata: null, - contentDispositionFilename: null, + _contentDispositionFilename: null, + _contentLength: null, triggerDelayedFallback: null, _saveInProgress: false, _wheelUnusedTicks: 0, @@ -793,7 +794,8 @@ const PDFViewerApplication = { this.baseUrl = ""; this.documentInfo = null; this.metadata = null; - this.contentDispositionFilename = null; + this._contentDispositionFilename = null; + this._contentLength = null; this.triggerDelayedFallback = null; this._saveInProgress = false; for (const callback of this._idleCallbacks) { @@ -946,7 +948,7 @@ const PDFViewerApplication = { // Use this.url instead of this.baseUrl to perform filename detection based // on the reference fragment as ultimate fallback if needed. const filename = - this.contentDispositionFilename || getPDFFileNameFromURL(this.url); + this._contentDispositionFilename || getPDFFileNameFromURL(this.url); const downloadManager = this.downloadManager; downloadManager.onerror = err => { // This error won't really be helpful because it's likely the @@ -979,7 +981,7 @@ const PDFViewerApplication = { // Use this.url instead of this.baseUrl to perform filename detection based // on the reference fragment as ultimate fallback if needed. const filename = - this.contentDispositionFilename || getPDFFileNameFromURL(this.url); + this._contentDispositionFilename || getPDFFileNameFromURL(this.url); const downloadManager = this.downloadManager; downloadManager.onerror = err => { // This error won't really be helpful because it's likely the @@ -1488,9 +1490,23 @@ const PDFViewerApplication = { }); const dispatchEventName = generateRandomStringForSandbox(objects); - const { length } = await pdfDocument.getDownloadInfo(); + + if (!this._contentLength) { + // Always waiting for the entire PDF document to be loaded will, most + // likely, delay sandbox-creation too much in the general case for all + // PDF documents which are not provided as binary data to the API. + // Hence we'll simply have to trust that the `contentLength` (as provided + // by the server), when it exists, is accurate enough here. + const { length } = await pdfDocument.getDownloadInfo(); + + if (pdfDocument !== this.pdfDocument) { + return; // The document was closed while the download info resolved. + } + this._contentLength = length; + } const filename = - this.contentDispositionFilename || getPDFFileNameFromURL(this.url); + this._contentDispositionFilename || getPDFFileNameFromURL(this.url); + scripting.createSandbox({ objects, dispatchEventName, @@ -1502,7 +1518,7 @@ const PDFViewerApplication = { docInfo: { ...this.documentInfo, baseURL: this.baseUrl, - filesize: length, + filesize: this._contentLength, filename, metadata: this.metadata, numPages: pdfDocument.numPages, @@ -1582,6 +1598,7 @@ const PDFViewerApplication = { info, metadata, contentDispositionFilename, + contentLength, } = await pdfDocument.getMetadata(); if (pdfDocument !== this.pdfDocument) { @@ -1589,7 +1606,8 @@ const PDFViewerApplication = { } this.documentInfo = info; this.metadata = metadata; - this.contentDispositionFilename = contentDispositionFilename; + this._contentDispositionFilename = contentDispositionFilename; + this._contentLength = contentLength; // Provides some basic debug information console.log( From f4d8a427f0358790f123ce5823be6b049de47b2d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 13:19:14 +0100 Subject: [PATCH 10/12] Ensure that the *correct* PDF document is still active after *every* asynchronous API-call in `PDFViewerApplication._initializeJavaScript` This patch also changes the method to skip *all* data fetching when "enableScripting" isn't active. Finally, simplifies some event-data accesses in the "updateFromSandbox" listener. --- web/app.js | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/web/app.js b/web/app.js index 8a78d128d..d62cbf632 100644 --- a/web/app.js +++ b/web/app.js @@ -1409,16 +1409,20 @@ const PDFViewerApplication = { * @private */ async _initializeJavaScript(pdfDocument) { - const objects = await pdfDocument.getFieldObjects(); - - if (pdfDocument !== this.pdfDocument) { - return; // The document was closed while the JavaScript data resolved. - } - if (!objects || !AppOptions.get("enableScripting")) { + if (!AppOptions.get("enableScripting")) { return; } - const calculationOrder = await pdfDocument.getCalculationOrderIds(); - const scripting = this.externalServices.scripting; + const [objects, calculationOrder] = await Promise.all([ + pdfDocument.getFieldObjects(), + pdfDocument.getCalculationOrderIds(), + ]); + + if (!objects || pdfDocument !== this.pdfDocument) { + // No FieldObjects were found in the document, + // or the document was closed while the data resolved. + return; + } + const { scripting } = this.externalServices; if (!this.documentInfo) { // It should be *extremely* rare for metadata to not have been resolved @@ -1436,37 +1440,37 @@ const PDFViewerApplication = { } window.addEventListener("updateFromSandbox", event => { - const detail = event.detail; - const id = detail.id; + const { detail } = event; + const { id, command, value } = detail; if (!id) { - switch (detail.command) { + switch (command) { case "alert": // eslint-disable-next-line no-alert - window.alert(detail.value); + window.alert(value); break; case "clear": console.clear(); break; case "error": - console.error(detail.value); + console.error(value); break; case "layout": - this.pdfViewer.spreadMode = apiPageLayoutToSpreadMode(detail.value); + this.pdfViewer.spreadMode = apiPageLayoutToSpreadMode(value); return; case "page-num": - this.pdfViewer.currentPageNumber = detail.value + 1; + this.pdfViewer.currentPageNumber = value + 1; return; case "print": this.triggerPrinting(); return; case "println": - console.log(detail.value); + console.log(value); break; case "zoom": - if (typeof detail.value === "string") { - this.pdfViewer.currentScaleValue = detail.value; + if (typeof value === "string") { + this.pdfViewer.currentScaleValue = value; } else { - this.pdfViewer.currentScale = detail.value; + this.pdfViewer.currentScale = value; } return; } @@ -1477,10 +1481,9 @@ const PDFViewerApplication = { if (element) { element.dispatchEvent(new CustomEvent("updateFromSandbox", { detail })); } else { - const value = detail.value; if (value !== undefined && value !== null) { - // the element hasn't been rendered yet so use annotation storage - pdfDocument.annotationStorage.setValue(id, detail.value); + // The element hasn't been rendered yet, use the AnnotationStorage. + pdfDocument.annotationStorage.setValue(id, value); } } }); From 1e007f9285ad1ded6696baa4da86e5d3fbfecbb4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 13:57:24 +0100 Subject: [PATCH 11/12] Actually remove a `scripting`-instance, and its global events, upon document closing I was actually quite surprised to find that, despite the various `scripting`-getters implementing `destroySandbox` methods, there were no attempts at actually cleaning-up either the "sandbox" or removing the globally registered event listeners. --- web/app.js | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/web/app.js b/web/app.js index d62cbf632..ff7aa9fb8 100644 --- a/web/app.js +++ b/web/app.js @@ -257,6 +257,7 @@ const PDFViewerApplication = { _saveInProgress: false, _wheelUnusedTicks: 0, _idleCallbacks: new Set(), + _scriptingInstance: null, // Called once when the document is loaded. async initialize(appConfig) { @@ -803,6 +804,18 @@ const PDFViewerApplication = { } this._idleCallbacks.clear(); + if (this._scriptingInstance) { + const { scripting, events } = this._scriptingInstance; + try { + scripting.destroySandbox(); + } catch (ex) {} + + for (const [name, listener] of events) { + window.removeEventListener(name, listener); + } + this._scriptingInstance = null; + } + this.pdfSidebar.reset(); this.pdfOutlineViewer.reset(); this.pdfAttachmentViewer.reset(); @@ -1423,6 +1436,9 @@ const PDFViewerApplication = { return; } const { scripting } = this.externalServices; + // Store a reference to the current scripting-instance, to allow destruction + // of the sandbox and removal of the event listeners at document closing. + this._scriptingInstance = { scripting, events: new Map() }; if (!this.documentInfo) { // It should be *extremely* rare for metadata to not have been resolved @@ -1439,7 +1455,7 @@ const PDFViewerApplication = { } } - window.addEventListener("updateFromSandbox", event => { + const updateFromSandbox = event => { const { detail } = event; const { id, command, value } = detail; if (!id) { @@ -1486,11 +1502,20 @@ const PDFViewerApplication = { pdfDocument.annotationStorage.setValue(id, value); } } - }); + }; + window.addEventListener("updateFromSandbox", updateFromSandbox); + // Ensure that the event listener can be removed at document closing. + this._scriptingInstance.events.set("updateFromSandbox", updateFromSandbox); - window.addEventListener("dispatchEventInSandbox", function (event) { + const dispatchEventInSandbox = event => { scripting.dispatchEventInSandbox(event.detail); - }); + }; + window.addEventListener("dispatchEventInSandbox", dispatchEventInSandbox); + // Ensure that the event listener can be removed at document closing. + this._scriptingInstance.events.set( + "dispatchEventInSandbox", + dispatchEventInSandbox + ); const dispatchEventName = generateRandomStringForSandbox(objects); From b2dfb5513692321307d85d5d20e985ae3a3fb22e Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Dec 2020 14:11:06 +0100 Subject: [PATCH 12/12] Re-factor the `scripting` getter in GENERIC-builds, since using the same sandbox for *multiple* PDF documents seems highly questionable Similar to the previous patch, the GENERIC default viewer is capable of opening more than *one* PDF document and we should ensure that we handle that case correctly. --- web/genericcom.js | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/web/genericcom.js b/web/genericcom.js index 73f78b045..2b83f0dfe 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -14,11 +14,11 @@ */ import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; -import { loadScript, shadow } from "pdfjs-lib"; import { AppOptions } from "./app_options.js"; import { BasePreferences } from "./preferences.js"; import { DownloadManager } from "./download_manager.js"; import { GenericL10n } from "./genericl10n.js"; +import { loadScript } from "pdfjs-lib"; if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("GENERIC")) { throw new Error( @@ -39,6 +39,29 @@ class GenericPreferences extends BasePreferences { } } +class GenericScripting { + constructor() { + this._ready = loadScript(AppOptions.get("sandboxBundleSrc")).then(() => { + return window.pdfjsSandbox.QuickJSSandbox(); + }); + } + + async createSandbox(data) { + const sandbox = await this._ready; + sandbox.create(data); + } + + async dispatchEventInSandbox(event) { + const sandbox = await this._ready; + sandbox.dispatchEvent(event); + } + + async destroySandbox() { + const sandbox = await this._ready; + sandbox.nukeSandbox(); + } +} + class GenericExternalServices extends DefaultExternalServices { static createDownloadManager(options) { return new DownloadManager(); @@ -53,22 +76,7 @@ class GenericExternalServices extends DefaultExternalServices { } static get scripting() { - const promise = loadScript(AppOptions.get("sandboxBundleSrc")).then(() => { - return window.pdfjsSandbox.QuickJSSandbox(); - }); - const sandbox = { - createSandbox(data) { - promise.then(sbx => sbx.create(data)); - }, - dispatchEventInSandbox(event) { - promise.then(sbx => sbx.dispatchEvent(event)); - }, - destroySandbox() { - promise.then(sbx => sbx.nukeSandbox()); - }, - }; - - return shadow(this, "scripting", sandbox); + return new GenericScripting(); } } PDFViewerApplication.externalServices = GenericExternalServices;