[Firefox] Remove the FirefoxCom.requestSync method
#17338
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I've successfully ran |
The return value is not, nor has it ever been, used for anything and we should thus be able to just send the message. Note that the responses are already handled by the "message" event listener registered above.
Despite the comment, I believe that changing this should be fine for two separate reasons: - The platform code has an "unload" event listener, see [this code](https://searchfox.org/mozilla-central/rev/edb2612db13e89f1c44ab95b1e4d4366c16eb9fb/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#533-538), that invokes the same method. Hence we should still be guaranteed that the relevant platform method will run. - The `FirefoxComDataRangeTransport.abort` method is never actually invoked in the Firefox PDF Viewer. Note that the [`PDFDataRangeTransport.abort` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L759) is only invoked via the [`PDFDataTransportStream.cancelAllRequests` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/transport_stream.js#L167-L175), which in turn is only invoked via the [`WorkerTransport.destroy` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L2485-L2487). That method is invoked via the [`PDFDocumentLoadingTask.destroy` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L630), which in the viewer is only invoked via the [`PDFViewerApplication.close` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/web/app.js#L919) which is never actually called in the Firefox PDF Viewer. All-in-all, given the existing platform code *and* the current viewer-implementation it should thus be safe to not wait for the "abortLoading" message to complete.
After the two previous commits, which removed the remaining call-sites, this method is no longer used and can thus be removed. As mentioned in the JSDocs for the now removed method, synchronous communication between the viewer and the platform code isn't really a good idea. Once this patch has landed in mozilla-central some additional clean-up of the platform code will also be possible.
Snuffleupagus
force-pushed
the
rm-FirefoxCom-requestSync
branch
from
b3a604f
to
b03ce96
Compare
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Dec 1, 2023
…js-reviewers,calixte After the changes in mozilla/pdf.js#17338, which landed in bug 1867213, we longer send any synchronous events from the viewer to the platform-code. Differential Revision: https://phabricator.services.mozilla.com/D195082
aosmond
pushed a commit
to aosmond/gecko
that referenced
this pull request
Dec 1, 2023
…js-reviewers,calixte After the changes in mozilla/pdf.js#17338, which landed in bug 1867213, we longer send any synchronous events from the viewer to the platform-code. Differential Revision: https://phabricator.services.mozilla.com/D195082
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Dec 7, 2023
…js-reviewers,calixte After the changes in mozilla/pdf.js#17338, which landed in bug 1867213, we longer send any synchronous events from the viewer to the platform-code. Differential Revision: https://phabricator.services.mozilla.com/D195082 UltraBlame original commit: e4d00d2a30f40c896b99c6ab4f631a5d5f80e30d
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Dec 7, 2023
…js-reviewers,calixte After the changes in mozilla/pdf.js#17338, which landed in bug 1867213, we longer send any synchronous events from the viewer to the platform-code. Differential Revision: https://phabricator.services.mozilla.com/D195082 UltraBlame original commit: e4d00d2a30f40c896b99c6ab4f631a5d5f80e30d
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Dec 7, 2023
…js-reviewers,calixte After the changes in mozilla/pdf.js#17338, which landed in bug 1867213, we longer send any synchronous events from the viewer to the platform-code. Differential Revision: https://phabricator.services.mozilla.com/D195082 UltraBlame original commit: e4d00d2a30f40c896b99c6ab4f631a5d5f80e30d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please refer to the individual commit messages for additional details.