Small clean-up of the PDFDocumentProxy.destroy method and related code

[Snuffleupagus]

Note how PDFDocumentProxy.destroy is a nothing more than an alias for PDFDocumentLoadingTask.destroy. While removing the latter method would be a breaking API change, there's still room for at least some clean-up here.

The main changes in this patch are:

• Stop providing a PDFDocumentLoadingTask instance separately when creating a PDFDocumentProxy, since the loadingTask is already available through the WorkerTransport instance.
• Stop tracking the PDFDocumentProxy instance on the WorkerTransport, since that property is completely unused.
• Simplify the 'Multiple getDocument instances' unit-tests by only destroying once, rather than twice, for each document.

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Windows)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

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

Total script time: 18.00 mins

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

[pdfjsbot]

From: Bot.io (Windows)

---

Success

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

Total script time: 25.73 mins

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

[timvandermeij]

It looks like this proxy is exposed in the API, so should this change be marked as [api-minor]/[api-major] given that it changes the constructor's signature?

[Snuffleupagus]

It looks like this proxy is exposed in the API, so should this change be marked as [api-minor]/[api-major] given that it changes the constructor's signature?

Sorry, I'm slightly confused as to which proxy you're referring to here?

Ninja-edit: Are you worried that users may be relying the loadingTask property that's currently exposed on a PDFDocumentProxy instance?
In that case, you're obviously correct that it's bad to just break this without even a warning!

When calling getDocument, you get a PDFDocumentLoadingTask instance in return whose promise property will eventually be resolved with a PDFDocumentProxy instance.
It thus seem somewhat strange, to me at least, to get at the PDFDocumentLoadingTask from the PDFDocumentProxy, but I worry that that might not be as uncommon as I'd hoped. (Considering that getDocument seem to often be treated as if it returned a normal Promise.)

How about just adding a loadingTask getter on PDFDocumentProxy, to not break any existing code and still allow for some clean-up?
Really good comment, by the way!

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Windows)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

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

Total script time: 17.86 mins

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

[pdfjsbot]

From: Bot.io (Windows)

---

Success

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

Total script time: 25.74 mins

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

[timvandermeij]

Are you worried that users may be relying the loadingTask property that's currently exposed on a PDFDocumentProxy instance?

Yes, that was what I was thinking about.

How about just adding a loadingTask getter on PDFDocumentProxy, to not break any existing code and still allow for some clean-up?

I'm now actually questioning if we need to do that, given that we also remove the pdfDocument and rename the numPages members, so there is no telling if people actually depend on that. What we could of course do is add getters for those members as well like you did for loadingTask, but if we do then I think we should add a call to the deprecated function inside them so we can remove them for the next major release.

On the other hand, I don't think we actually documented/advertised using those properties of the document proxy, so I'm not sure if it actually qualifies as an API change. What do you think about this?

[Snuffleupagus]

I'm now actually questioning if we need to do that, given that we also remove the pdfDocument and rename the numPages members,

But that's on the WorkerTransport, which should be marked as a "private" property everywhere and not directly exposed anywhere that I know of.
Hence I don't think that matters at all here, unless of course I'm overlooking something!

The reason for wanting to keep a way to go from PDFDocumentProxy to PDFDocumentLoadingTask is for those cases where the user treats getDocument as returning a promise and wanting to access the loadingTask (which is backwards, but has existed for a long time).

[timvandermeij]

You're completely right; I overlooked that part. I took another look and I'm now also convinced that this is the right approach. Thank you!