In order to simplify m-c code, move some in pdf.js

[calixteman]

• move set/clear|Timeout/Interval and crackURL code in pdf.js
• remove the "backdoor" in the proxy (used to dispatch event) and so return the dispatch function in the initializer
• remove listeners if an error occured during sandbox initialization
• add support for alert and prompt in the sandbox
• add a function to eval in the global scope

[Snuffleupagus]

Why is this removing the destroySandbox-functionality, since I cannot imagine that it'll actually work correctly in the general case?

Please keep in mind that the GENERIC default viewer supports opening more than one document, and I'd thus expect that you need a unique scripting-instance for each document (the same way that basically everything in the default viewer is document specific); see PR #12695 which tries to improve a number of things related to both pdf.sandbox.js building and how it's used in the viewer.

[brendandahl]

I did a quick skim and seems reasonable. I'll let @Rob--W review the parts he suggested.

[Rob--W]

Why is this removing the destroySandbox-functionality, since I cannot imagine that it'll actually work correctly in the general case?

Please keep in mind that the GENERIC default viewer supports opening more than one document, and I'd thus expect that you need a unique scripting-instance for each document (the same way that basically everything in the default viewer is document specific); see PR #12695 which tries to improve a number of things related to both pdf.sandbox.js building and how it's used in the viewer.

I pointed out that the sandbox destruction logic wasn't called, at https://phabricator.services.mozilla.com/D91746?id=370264#inline-555164 with the ending note:

Make this sandbox destruction part of the window. There is already an unload listener, you could call destroySandbox from there (in addition to any other existing calls in PDF.js code).

By this, I meant to call it at least twice: as part of the privileged unload handler (which always runs), and as usual from the destruction logic in PDF.js. @Snuffleupagus has already fixed the latter in #12695, so what remains is only on the m-c side (and also there already).

[Rob--W]

It took a while to review this because the patch is very large, with many more changes than strictly needed to provide the sandboxing functionality. The review was further complicated by the lack of documentation and references to relevant sources.

To support my review task, I composed the following list of locations that are relevant to the feature that I was asked to review (a sandbox mechanism). I also reviewed the other code since examples of actual usage is a good verification of the effectiveness of the API surrounding the sandbox.

• https://phabricator.services.mozilla.com/D91746 - patch for mozilla-central, with Firefox's implementation.
• https://github.com/mozilla/pdf.js.quickjs - PDF.js's bindings/glue for the quickjs sandbox
• In order to simplify m-c code, move some in pdf.js #12689 (this patch)
• https://github.com/mozilla/pdf.js/tree/master/src/scripting_api
• https://github.com/mozilla/pdf.js/blob/master/src/pdf.sandbox.js

There is no clear authorative reference for JavaScript in PDFs. The following are available (with an incomplete changelog).

• April 2007 - Adobe Acrobat SDK version 8.1
  https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/js_api_reference.pdf
• May 2015 - Adobe Acrobat DC SDK
  https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/AcrobatDC_js_api_reference.pdf
• HTML version of the 2015 document, e.g. specifically the util.crackURL method that you're adding to this patch:
  https://help.adobe.com/en_US/acrobat/acrobat_dc_sdk/2015/HTMLHelp/Acro12_MasterBook/JS_API_AcroJS/util_methods.htm

The last published reference is from 5 years ago, so they may be out of date. Interestingly, Chrome's PDF implementation has a README that references the 2007 implementation, at https://source.chromium.org/chromium/chromium/src/+/master:third_party/pdfium/fxjs/README;l=40;drc=d9d3ed281f87f706ecc442c7902abfa8ef5c4928

Could you also add a README.md to the scripting_api repository, to allow other readers to know what you're using as the definition of the API, and how everything fits together?

[Rob--W]

I looked closely at the interaction of the sandbox and quickjs, a bit at the timer, but didn't look at the rest in detail. Since there is already something actionable, I'm returning it for review now.

[Rob--W]

It's getting in a good shape. I do have a few comments, but the overall structure looks sane sane.

[Rob--W]

r+ with comments addressed.

[calixteman]

/botio test

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

Live output at: http://54.67.70.0:8877/50347eb15a009eb/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Received

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

Live output at: http://3.101.106.178:8877/128f561af7695f6/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.67.70.0:8877/50347eb15a009eb/output.txt

Total script time: 25.78 mins

• Font tests: Passed
• Unit tests: Passed
• Integration Tests: Passed
• Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/50347eb15a009eb/reftest-analyzer.html#web=eq.log

[pdfjsbot]

From: Bot.io (Windows)

---

Failed

Full output at http://3.101.106.178:8877/128f561af7695f6/output.txt

Total script time: 27.18 mins

• Font tests: Passed
• Unit tests: Passed
• Integration Tests: Passed
• Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/128f561af7695f6/reftest-analyzer.html#web=eq.log