Issues with patch that removed disableWorker

[tonyjin]

Thanks for all the great work!

I'm running into a slight issue with my particular setup. The recent patch to remove PDFJS.disableWorker (56a8c93) updated one of the checks for a fake worker to use

 if (typeof Worker !== 'undefined' && !isWorkerDisabled &&
          !getMainThreadWorkerMessageHandler()) {

I prefetch the worker file while users are not on a page that requires PDF preview, and when they do come to a document preview, I point the workerSrc to that same file that I prefetched earlier. Because of browser caching, the worker can be loaded from the cache. With this patch, getMainThreadWorkerMessageHandler() will return a handler because I prefetched the worker file at an earlier time and then this block returns false and goes into _setupFakeWorker(). Can this last handler check be entirely removed?

On a related, but slightly different note, would you folks be open to a PR that swaps these two in

pdf.js/src/display/api.js

Line 1204 in 7fbeeeb

function getWorkerSrc() {

?

if (typeof workerSrc !== 'undefined') {
      return workerSrc;
}
if (getDefaultSetting('workerSrc')) {
     return getDefaultSetting('workerSrc');
}

The library I build can sometimes be run in an application that uses RequireJS (in which workerSrc will be set to requirejs.url(...something)). I'd explicitly like to keep using the workerSrc I define using PDFJS.workerSrc = 'someUrl'.

[Snuffleupagus]

Can this last handler check be entirely removed?

If what you're suggesting is to completely remove line

pdf.js/src/display/api.js

Line 1336 in 6b7e2cb

!getMainThreadWorkerMessageHandler()) {

then that would make it totally impossible, rather than the intended difficult, for users to disable workers.
This would also imply that it's no longer possible to simulate the functionality of the (now removed) pdf.combined.js file, by manually loading pdf.worker.js into the main-thread.

Please note that, as part of the upcoming version 2.0 of PDF.js, we wanted to make it more difficult (but not impossible) to disable workers. Keep in mind that using PDF.js without workers will result in worse performance.
Unfortunately, since the pdf.combined.js file appeares to be somewhat popular in practice we still wanted a way to support that use-case; hence why this code was re-factored in PR #9385.

I prefetch the worker file while users are not on a page that requires PDF preview, and when they do come to a document preview, I point the workerSrc to that same file that I prefetched earlier.

In that case, why don't you simply use workerPort instead to load the worker file?

pdf.js/src/display/global.js

Lines 141 to 144 in 6b7e2cb

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

[tonyjin]

Ah perfect, let me try workerPort, looks like what I need :)

Edit: Unfortunately it looks like SharedWorkers (for the exposed port) aren't supported in IE11, which we have to support for the foreseeable future. I'm going to look into using preload instead of prefetch for the worker script, which should prevent execution.

Do you foresee any issue with swapping the order of:

if (typeof workerSrc !== 'undefined') {
      return workerSrc;
}
if (getDefaultSetting('workerSrc')) {
     return getDefaultSetting('workerSrc');
}

So if workerSrc is manually set, we always use that instead of letting the RequireJS check override it? (from

pdf.js/src/display/api.js

Line 64 in 6b7e2cb

workerSrc = requirejs.toUrl('pdfjs-dist/build/pdf.worker.js');

)

[Snuffleupagus]

Do you foresee any issue with swapping the order of:

That seems somewhat orthogonal to the rest of the issue, but I suppose that would be OK.