Skip to content

Commit

Permalink
[api-major] Remove the PDFJS.disableWorker option
Browse files Browse the repository at this point in the history
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 `<script>` tag on the main-thread.[1]
That way, the functionality of the now removed `SINGLE_FILE` build target and the resulting `build/pdf.combined.js` file can still be achieved simply by adding e.g. `<script src="build/pdf.worker.js"></script>` 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.
  • Loading branch information
Snuffleupagus committed Jan 31, 2018
1 parent a5aaf62 commit 56a8c93
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 57 deletions.
3 changes: 0 additions & 3 deletions examples/browserify/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 0 additions & 7 deletions examples/learning/helloworld.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ <h1>'Hello, world!' example</h1>
//
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.
//
Expand Down
6 changes: 0 additions & 6 deletions examples/learning/helloworld64.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ <h1>'Hello, world!' example</h1>
'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.
//
Expand Down
8 changes: 0 additions & 8 deletions examples/learning/prevnext.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ <h1>'Previous/Next' example</h1>
//
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
Expand Down
3 changes: 0 additions & 3 deletions examples/webpack/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 0 additions & 2 deletions external/dist/webpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
24 changes: 22 additions & 2 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 `<script>` tag.
fakeWorkerFilesLoadedCapability.resolve(mainWorkerMessageHandler);
return fakeWorkerFilesLoadedCapability.promise;
}
// In the developer build load worker_loader which in turn loads all the
// other files and resolves the promise. In production only the
// pdf.worker.js file is needed.
Expand Down Expand Up @@ -1316,7 +1336,7 @@ var PDFWorker = (function PDFWorkerClosure() {
// 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 Worker !== 'undefined' && !isWorkerDisabled &&
!getDefaultSetting('disableWorker')) {
!getMainThreadWorkerMessageHandler()) {
var workerSrc = getWorkerSrc();

try {
Expand Down Expand Up @@ -1427,7 +1447,7 @@ var PDFWorker = (function PDFWorkerClosure() {
},

_setupFakeWorker: function PDFWorker_setupFakeWorker() {
if (!isWorkerDisabled && !getDefaultSetting('disableWorker')) {
if (!isWorkerDisabled) {
warn('Setting up fake worker.');
isWorkerDisabled = true;
}
Expand Down
2 changes: 0 additions & 2 deletions src/display/dom_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,6 @@ function getDefaultSetting(id) {
return globalSettings ? globalSettings.workerPort : null;
case 'workerSrc':
return globalSettings ? globalSettings.workerSrc : null;
case 'disableWorker':
return globalSettings ? globalSettings.disableWorker : false;
case 'maxImageSize':
return globalSettings ? globalSettings.maxImageSize : -1;
case 'imageResourcesPath':
Expand Down
12 changes: 1 addition & 11 deletions src/display/global.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,6 @@ PDFJS.disableFontFace = (PDFJS.disableFontFace === undefined ?
PDFJS.imageResourcesPath = (PDFJS.imageResourcesPath === undefined ?
'' : PDFJS.imageResourcesPath);

/**
* Disable the web worker and run all code on the main thread. This will
* happen automatically if the browser doesn't support workers or sending
* typed arrays to workers.
* @var {boolean}
*/
PDFJS.disableWorker = (PDFJS.disableWorker === undefined ?
false : PDFJS.disableWorker);

/**
* Path and filename of the worker file. Required when the worker is enabled
* in development mode. If unspecified in the production build, the worker
Expand All @@ -148,8 +139,7 @@ PDFJS.disableWorker = (PDFJS.disableWorker === undefined ?
PDFJS.workerSrc = (PDFJS.workerSrc === undefined ? null : PDFJS.workerSrc);

/**
* Defines global port for worker process. Overrides workerSrc and
* disableWorker setting.
* Defines global port for worker process. Overrides `workerSrc` setting.
*/
PDFJS.workerPort = (PDFJS.workerPort === undefined ? null : PDFJS.workerPort);

Expand Down
16 changes: 7 additions & 9 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,14 @@ describe('api', function() {
path: TEST_PDFS_PATH.node + basicApiFileName,
});
} else {
var nonBinaryRequest = PDFJS.disableWorker;
var request = new XMLHttpRequest();
let nonBinaryRequest = false;
let request = new XMLHttpRequest();
request.open('GET', TEST_PDFS_PATH.dom + basicApiFileName, false);
if (!nonBinaryRequest) {
try {
request.responseType = 'arraybuffer';
nonBinaryRequest = request.responseType !== 'arraybuffer';
} catch (e) {
nonBinaryRequest = true;
}
try {
request.responseType = 'arraybuffer';
nonBinaryRequest = request.responseType !== 'arraybuffer';
} catch (e) {
nonBinaryRequest = true;
}
if (nonBinaryRequest && request.overrideMimeType) {
request.overrideMimeType('text/plain; charset=x-user-defined');
Expand Down
38 changes: 34 additions & 4 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import {
} from './ui_utils';
import {
build, createBlob, getDocument, getFilenameFromUrl, InvalidPDFException,
MissingPDFException, OPS, PDFJS, shadow, UnexpectedResponseException,
UNSUPPORTED_FEATURES, version
MissingPDFException, OPS, PDFJS, PDFWorker, shadow,
UnexpectedResponseException, UNSUPPORTED_FEATURES, version
} from 'pdfjs-lib';
import { CursorTool, PDFCursorTools } from './pdf_cursor_tools';
import { PDFRenderingQueue, RenderingStates } from './pdf_rendering_queue';
Expand Down Expand Up @@ -285,8 +285,9 @@ let PDFViewerApplication = {
let hash = document.location.hash.substring(1);
let hashParams = parseQueryString(hash);

if ('disableworker' in hashParams) {
PDFJS.disableWorker = (hashParams['disableworker'] === 'true');
if ('disableworker' in hashParams &&
hashParams['disableworker'] === 'true') {
waitOn.push(loadFakeWorker());
}
if ('disablerange' in hashParams) {
PDFJS.disableRange = (hashParams['disablerange'] === 'true');
Expand Down Expand Up @@ -1512,6 +1513,35 @@ if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) {
};
}

function loadFakeWorker() {
return new Promise(function(resolve, reject) {
if (typeof PDFJSDev === 'undefined' || !PDFJSDev.test('PRODUCTION')) {
if (typeof SystemJS === 'object') {
SystemJS.import('pdfjs/core/worker').then((worker) => {
window.pdfjsNonProductionPdfWorker = worker;
resolve();
});
} else if (typeof require === 'function') {
window.pdfjsNonProductionPdfWorker = require('../src/core/worker.js');
resolve();
} else {
reject(new Error(
'SystemJS or CommonJS must be used to load fake worker.'));
}
} else {
let script = document.createElement('script');
script.src = PDFWorker.getWorkerSrc();
script.onload = function() {
resolve();
};
script.onerror = function() {
reject(new Error(`Cannot load fake worker at: ${script.src}`));
};
(document.head || document.documentElement).appendChild(script);
}
});
}

function loadAndEnablePDFBug(enabledTabs) {
return new Promise(function (resolve, reject) {
let appConfig = PDFViewerApplication.appConfig;
Expand Down

0 comments on commit 56a8c93

Please sign in to comment.