From aa8621f1006fd8dc2bc87ddd532810499cbb5139 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 17 Jan 2018 18:20:00 +0100 Subject: [PATCH 1/4] [api-major] Remove the `SINGLE_FILE` build target Please note that this build target, and the resulting `build/pdf.combined.js` file, is equivalent to setting the `PDFJS.disableWorker` option to `true` which is a performance footgun. --- gulpfile.js | 27 +-------------------------- src/display/api.js | 12 +++--------- src/display/svg.js | 3 +-- src/pdf.js | 5 ++--- 4 files changed, 7 insertions(+), 40 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index 8300332095466..4d2bdc808d7b3 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -48,7 +48,6 @@ var BASELINE_DIR = BUILD_DIR + 'baseline/'; var MOZCENTRAL_BASELINE_DIR = BUILD_DIR + 'mozcentral.baseline/'; var GENERIC_DIR = BUILD_DIR + 'generic/'; var COMPONENTS_DIR = BUILD_DIR + 'components/'; -var SINGLE_FILE_DIR = BUILD_DIR + 'singlefile/'; var MINIFIED_DIR = BUILD_DIR + 'minified/'; var FIREFOX_BUILD_DIR = BUILD_DIR + 'firefox/'; var CHROME_BUILD_DIR = BUILD_DIR + 'chromium/'; @@ -79,7 +78,6 @@ var DEFINES = { MOZCENTRAL: false, CHROME: false, MINIFIED: false, - SINGLE_FILE: false, COMPONENTS: false, LIB: false, SKIP_BABEL: false, @@ -235,10 +233,6 @@ function createBundle(defines) { var mainAMDName = 'pdfjs-dist/build/pdf'; var mainOutputName = 'pdf.js'; - if (defines.SINGLE_FILE) { - mainAMDName = 'pdfjs-dist/build/pdf.combined'; - mainOutputName = 'pdf.combined.js'; - } var mainFileConfig = createWebpackConfig(defines, { filename: mainOutputName, @@ -250,9 +244,6 @@ function createBundle(defines) { .pipe(webpack2Stream(mainFileConfig)) .pipe(replaceWebpackRequire()) .pipe(replaceJSRootName(mainAMDName)); - if (defines.SINGLE_FILE) { - return mainOutput; // don't need a worker file. - } var workerAMDName = 'pdfjs-dist/build/pdf.worker'; var workerOutputName = 'pdf.worker.js'; @@ -634,18 +625,6 @@ gulp.task('components', ['buildnumber'], function () { ]); }); -gulp.task('singlefile', ['buildnumber'], function () { - console.log(); - console.log('### Creating singlefile build'); - var defines = builder.merge(DEFINES, { SINGLE_FILE: true, }); - - var SINGLE_FILE_BUILD_DIR = SINGLE_FILE_DIR + 'build/'; - - rimraf.sync(SINGLE_FILE_DIR); - - return createBundle(defines).pipe(gulp.dest(SINGLE_FILE_BUILD_DIR)); -}); - gulp.task('minified-pre', ['buildnumber', 'locale'], function () { console.log(); console.log('### Creating minified viewer'); @@ -1275,9 +1254,7 @@ gulp.task('gh-pages-git', ['gh-pages-prepare', 'wintersmith'], function () { gulp.task('web', ['gh-pages-prepare', 'wintersmith', 'gh-pages-git']); -gulp.task('dist-pre', - ['generic', 'singlefile', 'components', 'lib', 'minified'], - function () { +gulp.task('dist-pre', ['generic', 'components', 'lib', 'minified'], function() { var VERSION = getVersionJSON().version; console.log(); @@ -1359,8 +1336,6 @@ gulp.task('dist-pre', GENERIC_DIR + 'build/pdf.js.map', GENERIC_DIR + 'build/pdf.worker.js', GENERIC_DIR + 'build/pdf.worker.js.map', - SINGLE_FILE_DIR + 'build/pdf.combined.js', - SINGLE_FILE_DIR + 'build/pdf.combined.js.map', SRC_DIR + 'pdf.worker.entry.js', ]).pipe(gulp.dest(DIST_DIR + 'build/')), gulp.src(MINIFIED_DIR + 'build/pdf.js') diff --git a/src/display/api.js b/src/display/api.js index d70014d73a693..d53e9db3e546b 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -46,8 +46,7 @@ var pdfjsFilePath = var fakeWorkerFilesLoader = null; var useRequireEnsure = false; -if (typeof PDFJSDev !== 'undefined' && - PDFJSDev.test('GENERIC && !SINGLE_FILE')) { +if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) { // For GENERIC build we need add support of different fake file loaders // for different frameworks. if (typeof window === 'undefined') { @@ -1243,10 +1242,6 @@ var PDFWorker = (function PDFWorkerClosure() { throw new Error( 'SystemJS or CommonJS must be used to load fake worker.'); } - } else if (PDFJSDev.test('SINGLE_FILE')) { - var pdfjsCoreWorker = require('../core/worker.js'); - WorkerMessageHandler = pdfjsCoreWorker.WorkerMessageHandler; - fakeWorkerFilesLoadedCapability.resolve(WorkerMessageHandler); } else { var loader = fakeWorkerFilesLoader || function (callback) { Util.loadScript(getWorkerSrc(), function () { @@ -1320,9 +1315,8 @@ var PDFWorker = (function PDFWorkerClosure() { // all requirements to run parts of pdf.js in a web worker. // Right now, the requirement is, that an Uint8Array is still an // Uint8Array as it arrives on the worker. (Chrome added this with v.15.) - if ((typeof PDFJSDev === 'undefined' || !PDFJSDev.test('SINGLE_FILE')) && - !isWorkerDisabled && !getDefaultSetting('disableWorker') && - typeof Worker !== 'undefined') { + if (typeof Worker !== 'undefined' && !isWorkerDisabled && + !getDefaultSetting('disableWorker')) { var workerSrc = getWorkerSrc(); try { diff --git a/src/display/svg.js b/src/display/svg.js index 1322114b28f34..a9a7a0ca92c62 100644 --- a/src/display/svg.js +++ b/src/display/svg.js @@ -25,8 +25,7 @@ var SVGGraphics = function() { throw new Error('Not implemented: SVGGraphics'); }; -if (typeof PDFJSDev === 'undefined' || - PDFJSDev.test('GENERIC || SINGLE_FILE')) { +if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { var SVG_DEFAULTS = { fontStyle: 'normal', diff --git a/src/pdf.js b/src/pdf.js index 8ef2263dce644..38e5e458ca280 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -29,8 +29,7 @@ var pdfjsDisplayAnnotationLayer = require('./display/annotation_layer.js'); var pdfjsDisplayDOMUtils = require('./display/dom_utils.js'); var pdfjsDisplaySVG = require('./display/svg.js'); -if (typeof PDFJSDev === 'undefined' || - !PDFJSDev.test('FIREFOX || MOZCENTRAL || CHROME')) { +if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { const isNodeJS = require('./shared/is_node.js'); if (isNodeJS()) { let PDFNodeStream = require('./display/node_stream.js').PDFNodeStream; @@ -43,7 +42,7 @@ if (typeof PDFJSDev === 'undefined' || pdfjsDisplayAPI.setPDFNetworkStreamFactory((params) => { return new PDFFetchStream(params); }); - } else { + } else { let PDFNetworkStream = require('./display/network.js').PDFNetworkStream; pdfjsDisplayAPI.setPDFNetworkStreamFactory((params) => { return new PDFNetworkStream(params); From 5c712e446cbf35d6161a92737a7abb2a6e5891eb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 29 Jan 2018 15:58:40 +0100 Subject: [PATCH 2/4] [api-minor] Add a (static) `PDFWorker.getWorkerSrc` method that returns the current `workerSrc` This method returns the currently used `workerSrc`, which thus allows obtaining the fallback `workerSrc` value (e.g. when the option wasn't set by the user). --- src/display/api.js | 4 ++++ test/unit/api_spec.js | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/display/api.js b/src/display/api.js index d53e9db3e546b..afc08c047dab0 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1485,6 +1485,10 @@ var PDFWorker = (function PDFWorkerClosure() { return new PDFWorker(null, port); }; + PDFWorker.getWorkerSrc = function() { + return getWorkerSrc(); + }; + return PDFWorker; })(); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 04e543bdf919b..955200effd477 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -24,7 +24,7 @@ import { DOMCanvasFactory, RenderingCancelledException } from '../../src/display/dom_utils'; import { - getDocument, PDFDocumentProxy, PDFPageProxy + getDocument, PDFDocumentProxy, PDFPageProxy, PDFWorker } from '../../src/display/api'; import isNodeJS from '../../src/shared/is_node'; import { PDFJS } from '../../src/display/global'; @@ -405,6 +405,11 @@ describe('api', function() { done.fail(reason); }); }); + it('gets current workerSrc', function() { + let workerSrc = PDFWorker.getWorkerSrc(); + expect(typeof workerSrc).toEqual('string'); + expect(workerSrc).toEqual(PDFJS.workerSrc); + }); }); describe('PDFDocument', function() { var loadingTask; From 3f3992c2622ab8cef2527f36d1597eb68f08aa84 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 19 Jan 2018 18:16:17 +0100 Subject: [PATCH 3/4] [api-major] Remove the `PDFJS.disableWorker` option Despite this patch removing the `disableWorker` option itself, please note that we'll still fallback to loading the worker file(s) on the main-thread when running in environments without proper Web Worker support. Furthermore it's still possible, even with this patch, to force the use of fake workers by manually loading the necessary file using a `` to the HTML (obviously with the path adjusted as needed). Finally note that the `disableWorker` option is a performance footgun, and unfortunately many existing third-party examples actually use it without providing any sort of warning/justification. --- [1] This approach is used in the default viewer, since certain kind of debugging may be easier if the code is running directly on the main-thread. --- examples/browserify/main.js | 3 --- examples/learning/helloworld.html | 7 ------ examples/learning/helloworld64.html | 6 ----- examples/learning/prevnext.html | 8 ------ examples/webpack/main.js | 3 --- external/dist/webpack.js | 2 -- src/display/api.js | 24 ++++++++++++++++-- src/display/dom_utils.js | 2 -- src/display/global.js | 12 +-------- test/unit/api_spec.js | 16 ++++++------ web/app.js | 38 ++++++++++++++++++++++++++--- 11 files changed, 64 insertions(+), 57 deletions(-) diff --git a/examples/browserify/main.js b/examples/browserify/main.js index 46a921a591863..42c640ef09027 100644 --- a/examples/browserify/main.js +++ b/examples/browserify/main.js @@ -10,9 +10,6 @@ var pdfPath = '../helloworld/helloworld.pdf'; // Setting worker path to worker bundle. PDFJS.workerSrc = '../../build/browserify/pdf.worker.bundle.js'; -// It is also possible to disable workers via `PDFJS.disableWorker = true`, -// however that might degrade the UI performance in web browsers. - // Loading a document. var loadingTask = PDFJS.getDocument(pdfPath); loadingTask.promise.then(function (pdfDocument) { diff --git a/examples/learning/helloworld.html b/examples/learning/helloworld.html index 4142ef6f5a81e..c2988c28707a4 100644 --- a/examples/learning/helloworld.html +++ b/examples/learning/helloworld.html @@ -19,13 +19,6 @@

'Hello, world!' example

// var url = './helloworld.pdf'; - // - // Disable workers to avoid yet another cross-origin issue (workers need - // the URL of the script to be loaded, and dynamically loading a cross-origin - // script does not work). - // - // PDFJS.disableWorker = true; - // // The workerSrc property shall be specified. // diff --git a/examples/learning/helloworld64.html b/examples/learning/helloworld64.html index 58890fe87a065..69420495a5340 100644 --- a/examples/learning/helloworld64.html +++ b/examples/learning/helloworld64.html @@ -31,12 +31,6 @@

'Hello, world!' example

'MDAwIG4gCjAwMDAwMDAzODAgMDAwMDAgbiAKdHJhaWxlcgo8PAogIC9TaXplIDYKICAvUm9v' + 'dCAxIDAgUgo+PgpzdGFydHhyZWYKNDkyCiUlRU9G'); - // Disable workers to avoid yet another cross-origin issue (workers need - // the URL of the script to be loaded, and dynamically loading a cross-origin - // script does not work). - // - // PDFJS.disableWorker = true; - // // The workerSrc property shall be specified. // diff --git a/examples/learning/prevnext.html b/examples/learning/prevnext.html index 463527a3df1bb..f7e68f2b5d42d 100644 --- a/examples/learning/prevnext.html +++ b/examples/learning/prevnext.html @@ -28,14 +28,6 @@

'Previous/Next' example

// var url = '../../web/compressed.tracemonkey-pldi-09.pdf'; - - // - // Disable workers to avoid yet another cross-origin issue (workers need - // the URL of the script to be loaded, and dynamically loading a cross-origin - // script does not work). - // - // PDFJS.disableWorker = true; - // // In cases when the pdf.worker.js is located at the different folder than the // pdf.js's one, or the pdf.js is executed via eval(), the workerSrc property diff --git a/examples/webpack/main.js b/examples/webpack/main.js index d0466b86d79d8..d1beb101cb1a4 100644 --- a/examples/webpack/main.js +++ b/examples/webpack/main.js @@ -10,9 +10,6 @@ var pdfPath = '../helloworld/helloworld.pdf'; // Setting worker path to worker bundle. pdfjsLib.PDFJS.workerSrc = '../../build/webpack/pdf.worker.bundle.js'; -// It is also possible to disable workers via `PDFJS.disableWorker = true`, -// however that might degrade the UI performance in web browsers. - // Loading a document. var loadingTask = pdfjsLib.getDocument(pdfPath); loadingTask.promise.then(function (pdfDocument) { diff --git a/external/dist/webpack.js b/external/dist/webpack.js index f2c4a6400898c..af397943940ea 100644 --- a/external/dist/webpack.js +++ b/external/dist/webpack.js @@ -19,8 +19,6 @@ var PdfjsWorker = require('worker-loader!./build/pdf.worker.js'); if (typeof window !== 'undefined' && 'Worker' in window) { pdfjs.PDFJS.workerPort = new PdfjsWorker(); -} else { - pdfjs.PDFJS.disableWorker = true; } module.exports = pdfjs; diff --git a/src/display/api.js b/src/display/api.js index afc08c047dab0..c62c3e7c1bca2 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1216,6 +1216,19 @@ var PDFWorker = (function PDFWorkerClosure() { throw new Error('No PDFJS.workerSrc specified'); } + function getMainThreadWorkerMessageHandler() { + if (typeof window === 'undefined') { + return null; + } + if (typeof PDFJSDev === 'undefined' || !PDFJSDev.test('PRODUCTION')) { + return (window.pdfjsNonProductionPdfWorker && + window.pdfjsNonProductionPdfWorker.WorkerMessageHandler); + } + // PRODUCTION + return (window.pdfjsDistBuildPdfWorker && + window.pdfjsDistBuildPdfWorker.WorkerMessageHandler); + } + let fakeWorkerFilesLoadedCapability; // Loads worker code into main thread. @@ -1225,6 +1238,13 @@ var PDFWorker = (function PDFWorkerClosure() { return fakeWorkerFilesLoadedCapability.promise; } fakeWorkerFilesLoadedCapability = createPromiseCapability(); + + let mainWorkerMessageHandler = getMainThreadWorkerMessageHandler(); + if (mainWorkerMessageHandler) { + // The worker was already loaded using a `