[Meta] Fix log issues from the dumpio: true Puppeteer option

[timvandermeij]

In PR #18260 we enabled dumpio: true for Puppeteer. This option forwards the browser logs to Node's log stream so that we can see any warnings/errors that are logged in e.g. the web console. This surfaced a few interesting issues that we should fix, not only to reduce the amount of logs but mainly because they might point at actual issues in our code.

This issue is a meta issue for this effort. We should go over the logs and file individual issues for each interesting issues. The logs from the last run in the PR are:

• Windows: http://54.193.163.58:8877/32bb151d23ef0ea/output.txt
• Linux: http://54.241.84.105:8877/2495d314125b7a2/output.txt

This meta issue already contains following issues:

• Fix an query with an empty selector in integration tests #18246
• Fix dumpio: true logs about #createBundleFallback #18264
• Fix dumpio: true logs about Script terminated by timeout #18265 (with follow-up Add a new helper, in the viewer, to close everything during testing #18289)
• Fix dumpio: true logs about translateMutations #18279 (with follow-ups Try to shutdown Fluent "more" when closing the viewer during testing #18313, Fix integration test "Stamp Editor Copy/paste from a tab to an other" #18318/Fix the "copy/paste from a tab to an other" stamp editor integration test #18320, Fix the integration tests passing despite one test failure #18319/Don't ignore errors in the Jasmine suite start/end stages #18321 and Remove the requestAnimationFrame work-around in L10n.prototype.destroy (PR 18313 follow-up) #18333)
• [Editor] Remove the various listeners when destroying the editor manager #18284 (with follow-up [Editor] Don't create an observer for the stamp annotation after the viewer has been closed #18301)
• Avoid to call a function in the js sandbox when it has been destroyed #18286
• Close PDFFindBar when closing the viewer during testing #18302
• Disable system addon updates for Firefox in testing mode #18316
• Close the page in the text layer caret selection integration test #18317
• Cancel the requestAnimationFrame in the watchScroll helper (PR 18193 follow-up) #18335
• Cancel the requestAnimationFrame in the API when cancelling rendering #18336
• Disable new tab page stuff in Firefox when running tests #18356
• [Editor] Avoid error when an editor is unselected during the pdf.js closing #18426
• Disable network connections to Contile for the tests #18433

[timvandermeij]

Having looked through the logs of the last few weeks, which are fortunately pretty clean after all the fixed above, I think we have two main categories of tracebacks left:

1. dispatchEventInSandbox-related logs such as in http://54.241.84.105:8877/c51fe1b82f42d3b/output.txt. In Avoid dispatching an event in pdfjsSandbox when destruction has run #18293 this was attempted to be solved, but it looks like the problem lies elsewhere.
2. bindListeners-related logs such as in http://54.193.163.58:8877/5f4c377dce34650/output.txt. This one seems to be because in at least the regular and secondary toolbar we only register event listeners but don't unregister them, and that should hopefully be easy to fix with e.g. abort controllers.

[Snuffleupagus]

2. bindListeners-related logs such as in http://54.193.163.58:8877/5f4c377dce34650/output.txt. This one seems to be because in at least the regular and secondary toolbar we only register event listeners but don't unregister them, and that should hopefully be easy to fix with e.g. abort controllers.

What's triggering those event listeners though, since the Toolbar/SecondaryToolbar have no async code and the errors appear to suggest that the tests somehow click on toolbar-buttons after we've invoked testingClose in the viewer?

[timvandermeij]

Good question, and I'm not really sure. However, I just found http://54.193.163.58:8877/7a0c5f232507dff/output.txt and http://54.193.163.58:8877/c0e29236d621ca3/output.txt which both also have the bindListener traceback at the same position in the logs, so that might be an indication that the must check the keyboard event is limited to the input integration test does something to trigger it (it interacts with the page number field in the toolbar).

[calixteman]

We're deleting the page number here:

pdf.js/test/integration/freetext_editor_spec.mjs

Lines 2922 to 2931 in 145951d

	await page.click("#pageNumber");
	await awaitPromise(handle);
	handle = await createPromise(page, resolve => {
	document
	.getElementById("pageNumber")
	.addEventListener("keyup", resolve, { once: true });
	});
	await page.keyboard.press("Backspace");
	await awaitPromise(handle);

and I guess the callback defined here:

pdf.js/web/toolbar.js

Lines 212 to 217 in 145951d

	pageNumber.addEventListener("change", function () {
	eventBus.dispatch("pagenumberchanged", {
	source: self,
	value: this.value,
	});
	});

is triggered just before we close.
So I think we should add the abort controller stuff in the toolbar to remove every listener we've here.

[timvandermeij]

It looks like there's a third one in addition to the two from #18281 (comment). For example in http://54.241.84.105:8877/00b7f8245c22a11/output.txt the #colorSelect one appears; this one happens less often but I have seen it a couple of times now. I don't recall having seen any other types of tracebacks, so for now I think it's limited to these three.