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

Handle WorkerTasks, and various PDF document properties, correctly in the "SaveDocument" handler in src/core/worker.js #12477

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

  • Actually register/unregister the WorkerTasks, used when saving each page, correctly.
    To prevent issues when terminating the Worker, we purposely wait for all running WorkerTasks to complete first. Hence we need to actually handle WorkerTasks the same way in "SaveDocument" as in the rest of this file, see e.g. "GetOperatorList" and "GetTextContent".

  • Access PDFDocument properties in a generally safe/consistent way.
    While the current code works fine, given how the PDF document is being loaded, it still seems like a very good idea to be consistent in how we access these kind of properties (since in general you need to avoid MissingDataException everywhere in this file).

  • Change a variable name, since there's essentially no precedent in the code-base for local variable names to start with an underscore.

…in the "SaveDocument" handler in `src/core/worker.js`

 - Actually register/unregister the `WorkerTask`s, used when saving each page, correctly.
   To prevent issues when terminating the Worker, we purposely wait for all running `WorkerTask`s to complete first. Hence we need to actually handle `WorkerTask`s the same way in "SaveDocument" as in the rest of this file, see e.g. "GetOperatorList" and "GetTextContent".

 - Access `PDFDocument` properties in a generally safe/consistent way.
   While the current code works fine, given how the PDF document is being loaded, it still seems like a very good idea to be *consistent* in how we access these kind of properties (since in general you need to avoid `MissingDataException` everywhere in this file).

 - Change a variable name, since there's essentially no precedent in the code-base for *local* variable names to start with an underscore.
@Snuffleupagus
Copy link
Collaborator Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/b2381612e983687/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/fded4ee8c1976e6/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/b2381612e983687/output.txt

Total script time: 4.22 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/fded4ee8c1976e6/output.txt

Total script time: 5.21 mins

  • Unit Tests: Passed

@timvandermeij timvandermeij merged commit 1034769 into mozilla:master Oct 13, 2020
@timvandermeij
Copy link
Contributor

Thank you for cleaning this up a bit!

@Snuffleupagus Snuffleupagus deleted the SaveDocument-WorkerTask branch October 14, 2020 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants