Bug 1697135 Comment 17 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I have some guesses as to what might cause this (specifically, the code around https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/uriloader/exthandler/nsExternalHelperAppService.cpp#1606-1621 ). What I don't understand is why it is a problem? That is, it is potentially superfluous IO, so from a performance perspective it's not ideal. But I'd like to better understand why this behaviour causes issues for you, because the comments so far don't seem to care about performance, but hint that this is somehow causing correctness issues in other software... without understanding what those are, it's hard for me to be sure what is the right way of addressing this, and/or making sure that other changes to our filesystem management for downloads don't break your stuff in other ways.

That said... we recently created a separate API that wouldn't require the file to exist, in order for this check to work, namely https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/toolkit/components/reputationservice/nsIApplicationReputation.idl#73, so perhaps we can swap that out and it'd resolve the issue here? The difference is that on *nix and macOS, the executable-ness is tied to permissions, for which we'd need to create a file... except that the existing check seems to cause problems there anyway (cf. bug 1664646). So perhaps this would be a positive change regardless...

the other confusion here is that apparently this didn't happen in 79, which is confusing because the code I linked to is much, *much* older than that.
I have some guesses as to what might cause this (specifically, the code around https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/uriloader/exthandler/nsExternalHelperAppService.cpp#1606-1621 ). What I don't understand is why it is a problem? That is, it is potentially superfluous IO, so from a performance perspective it's not ideal. But I'd like to better understand why this behaviour causes issues for you, because the comments so far don't seem to care about performance, but hint that this is somehow causing correctness issues in other software... without understanding what those are, it's hard for me to be sure what is the right way of addressing this, and/or making sure that other changes to our filesystem management for downloads don't break your stuff in other ways.

That said... we recently created a separate API that wouldn't require the file to exist, in order for this check to work, namely https://searchfox.org/mozilla-central/rev/3de1b6791ea569cdf7773f7cd512cf2e6cc1d3f0/toolkit/components/reputationservice/nsIApplicationReputation.idl#73, so perhaps we can swap that out and it'd resolve the issue here? The difference is that on *nix and macOS, the executable-ness is tied to permissions, for which we'd need to create a file... except that the existing check seems to cause problems there anyway (cf. bug 1664646). So perhaps this would be a positive change regardless...

the other confusion here is that apparently this didn't happen in 79, which is confusing because the code I linked to is much, *much* older than that. A regression window would still be helpful.

Back to Bug 1697135 Comment 17