Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean-up getMainThreadWorkerMessageHandler for non-PRODUCTION mode #9611

Merged
merged 1 commit into from
Mar 31, 2018
Merged

Clean-up getMainThreadWorkerMessageHandler for non-PRODUCTION mode #9611

merged 1 commit into from
Mar 31, 2018

Conversation

Snuffleupagus
Copy link
Collaborator

This is a final piece of clean-up of code that I recently wrote, after which I'm done :-)

When the getMainThreadWorkerMessageHandler function was added, in PR #9385, it did so by basically introducing a web/app.js dependency in src/display/api.js through the window.pdfjsNonProductionPdfWorker property[1]. Even though this is limited to non-PRODUCTION mode, i.e. gulp server, it still seems unfortunate to have that sort of viewer dependency in the API code itself.

With the new, much nicer and shorter, names introduced in PR #9565 we can remove this non-PRODUCTION hack and just use window.pdfjsWorker in both the viewer and the API regardless of the build mode.


[1] It didn't seem correct to piggy-back on the window.pdfjsDistBuildPdfWorker property in non-PRODUCTION mode.

*This is a final piece of clean-up of code that I recently wrote, after which I'm done :-)*

When the `getMainThreadWorkerMessageHandler` function was added, in PR 9385, it did so by basically introducing a `web/app.js` dependency in `src/display/api.js` through the `window.pdfjsNonProductionPdfWorker` property[1]. Even though this is limited to non-`PRODUCTION` mode, i.e. `gulp server`, it still seems unfortunate to have that sort of viewer dependency in the API code itself.

With the new, much nicer and shorter, names introduced in PR 9565 we can remove this non-`PRODUCTION` hack and just use `window.pdfjsWorker` in both the viewer and the API regardless of the build mode.

---

[1] It didn't seem correct to piggy-back on the `window.pdfjsDistBuildPdfWorker` property in non-`PRODUCTION` mode.
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/0c66e4c073272d8/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/0c66e4c073272d8/output.txt

Total script time: 2.71 mins

Published

@timvandermeij
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/62c15606492f067/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/760982d25738544/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/62c15606492f067/output.txt

Total script time: 18.07 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/760982d25738544/output.txt

Total script time: 24.41 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@timvandermeij timvandermeij merged commit 8512596 into mozilla:master Mar 31, 2018
@timvandermeij
Copy link
Contributor

Thank you!

@Snuffleupagus Snuffleupagus deleted the getMainThreadWorkerMessageHandler-non-PRODUCTION branch March 31, 2018 13:50
movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
…rMessageHandler-non-PRODUCTION

Clean-up `getMainThreadWorkerMessageHandler` for non-PRODUCTION mode
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.

3 participants