[api-minor] Only use Workers when postMessage transfers are supported (PR 11123 follow-up)
#14291
Conversation
…ed (PR 11123 follow-up) Given that all modern browsers now support `postMessage` transfers, and have for years, it no longer seems necessary for the PDF.js library to support using Workers unless the `postMessage` transfers functionality is available. This patch is a follow-up to PR 11123, which made it impossible to *manually* disable `postMessage` transfers for performance reasons (since it increases memory usage), which hasn't caused any bug reports as far as I know.[1] Hence we'll now only support *proper* Worker implementations, with fully working `postMessage` transfers, and fallback to using "fake" Workers otherwise. --- [1] At the time of that PR we still "supported" IE, which is why this code was left intact.
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/9921149ebc696dd/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/e3ce12f6202d3d8/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/9921149ebc696dd/output.txt Total script time: 22.20 mins
Image differences available at: http://54.241.84.105:8877/9921149ebc696dd/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e3ce12f6202d3d8/output.txt Total script time: 42.29 mins
Image differences available at: http://54.193.163.58:8877/e3ce12f6202d3d8/reftest-analyzer.html#web=eq.log |
|
I'm pretty much certain that the integration-test failures aren't related to this patch, given that I'm not touching any test-related code in this patch, that they only fail on Linux, and finally that other recent PRs have shown identical errors. |
|
Oh... these errors look strange. |
|
Indeed, I have confirmed in #14233 (comment) that it also happened here, so it's not specific to this patch. Fortunately this is already tracked in #14293. |
|
Nice clean-up! |
Given that all modern browsers now support
postMessagetransfers, and have for years, it no longer seems necessary for the PDF.js library to support using Workers unless thepostMessagetransfers functionality is available.This patch is a follow-up to PR #11123, which made it impossible to manually disable
postMessagetransfers for performance reasons (since it increases memory usage), which hasn't caused any bug reports as far as I know.[1]Hence we'll now only support proper Worker implementations, with fully working
postMessagetransfers, and fallback to using "fake" Workers otherwise.[1] At the time of that PR we still "supported" IE, which is why this code was left intact.