Skip to content

Use the worker created in the child actor (bug 1966721)#19935

Merged
calixteman merged 1 commit intomozilla:masterfrom
calixteman:bug1966721
May 15, 2025
Merged

Use the worker created in the child actor (bug 1966721)#19935
calixteman merged 1 commit intomozilla:masterfrom
calixteman:bug1966721

Conversation

@calixteman
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

Using a global pdfjsWorker property seems problematic here, since we're using the existence of globalThis.pdfjsWorker as an indication that fake-workers are being used.

@calixteman
Copy link
Contributor Author

Using a global pdfjsWorker property seems problematic here, since we're using the existence of globalThis.pdfjsWorker as an indication that fake-workers are being used.

Correct. What do you think about pdfjsParserWorker ?

@Snuffleupagus
Copy link
Collaborator

Using a global pdfjsWorker property seems problematic here, since we're using the existence of globalThis.pdfjsWorker as an indication that fake-workers are being used.

Correct. What do you think about pdfjsParserWorker ?

Naming things is hard, but maybe pdfjsPreloadedWorker to try and explain "more" what this is?

Another thing that just occurred to me is that we already have the workerPort option, hence rather than "hacking" this into the API it might be possible to just do the following. (Which might also be a tiny bit more efficient, since it avoids some checking during worker-setup.)

diff --git a/web/app_options.js b/web/app_options.js
index 461336dc1..34615b8fc 100644
--- a/web/app_options.js
+++ b/web/app_options.js
@@ -502,7 +502,10 @@ const defaultOptions = {

   workerPort: {
     /** @type {Object} */
-    value: null,
+    value:
+      typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")
+        ? globalThis.pdfjsPreloadedWorker
+        : null,
     kind: OptionKind.WORKER,
   },
   workerSrc: {

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

Nit: In the commit message (and PR title) there seems to be a missing word.
Use the worker created when the child actor has been {word missing here} (bug 1966721)

r=me, thank you.

@calixteman calixteman changed the title Use the worker created when the child actor has been (bug 1966721) Use the worker created in the child actor (bug 1966721) May 15, 2025
@calixteman calixteman merged commit f4a3e47 into mozilla:master May 15, 2025
7 checks passed
@calixteman calixteman deleted the bug1966721 branch May 15, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants