-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
[api-minor] Adds controlled destruction of the worker. #6546
Conversation
|
||
/** | ||
* Cleans up resources allocated by the page. | ||
* Deprecated, use cleanup() instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about also including a warn(...)
below to notify users of the deprecation (since it's not unlikely that people won't bother reading comments such as this)?
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/34925907f0c548c/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/cc367b486b03a90/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/cc367b486b03a90/output.txt Total script time: 18.17 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/34925907f0c548c/output.txt Total script time: 19.99 mins
Image differences available at: http://107.21.233.14:8877/34925907f0c548c/reftest-analyzer.html#web=eq.log |
d67f6ec
to
61e7bde
Compare
(We need to start 1.2 version for this one) |
1a33bc6
to
ed9b40c
Compare
There are some merge conflicts for this patch. I'm also going to try to list all issues and PRs that this patch resolves or supersedes (some of them are closed already, but I'm listing them anyway for completeness):
|
As discussed on IRC: changing WorkerThread name and making destroy() return a promise. Then r+ |
}; | ||
waitsForPromiseRejected(loadingTask.promise, function(reason) { | ||
expect(true).toEqual(true); | ||
// expect(destroyed).toEqual(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be commented out?
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/0965ea6f5fdfa55/output.txt |
Also renames PDFPageProxy.destroy method to cleanup.
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/0965ea6f5fdfa55/output.txt Total script time: 19.78 mins
|
/botio-windows test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 1 Live output at: http://107.22.172.223:8877/b5c95c1c044ab64/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/b5c95c1c044ab64/output.txt Total script time: 18.80 mins
|
[api-minor] Adds controlled destruction of the worker.
Fixes #5278 |
Really nice work! |
Introduces PDFDocumentLoadingTask destroy() method to initiate tear down
and onDestroyed callback to wait on its completion.Attempt to address intermittent failures during FF testing, e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1016737
That is also an alternative solution for #5629