1589082 - nsOSHelperAppService::GetProtocolHandlerInfo and GetProtocolHandlerInfoFromOS do not work in the child

Status: RESOLVED FIXED

(Firefox :: File Handling, defect, P1)

Description

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #47)

While investigating bug 1586148 I'm running into a lack of information in the child, and the child version of the OS helper app service seems to have been added here. While I can work around the issue in bug 1586148, it's confusing me and we probably need a follow-up if I'm not misreading what's happening here.

Specifically, the patches [in bug 1452278] implemented nsOSHelperAppServiceChild::GetProtocolHandlerInfoFromOS as relying on nsOSHelperAppServiceChild::OSProtocolHandlerExists. But the implementation of the latter is trivial and always returns NS_ERROR_NOT_IMPLEMENTED. The former is decidedly not trivial, even has review comments in phabricator - but as far as I can tell:

NS_IMETHODIMP
nsOSHelperAppServiceChild::GetProtocolHandlerInfoFromOS(
    const nsACString& aScheme, bool* aFound, nsIHandlerInfo** aRetVal) {
  MOZ_ASSERT(!aScheme.IsEmpty(), "No scheme was specified!");

  nsresult rv =
      OSProtocolHandlerExists(PromiseFlatCString(aScheme).get(), aFound);
  if (NS_WARN_IF(NS_FAILED(rv))) {
    return rv;
  }

it always early returns here, because rv is always going to be NS_ERROR_NOT_IMPLEMENTED. Looks like this code meant to use nsOSHelperAppServiceChild::ExternalProtocolHandlerExists.

Comment 1

[:Gijs (he/him)]

So I did a trypush to see if/when we hit this, and it doesn't look like we do in any of our browser mochitests or xpcshell tests ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb1903bc667cccb38f0f629818cb5cd43a440c68 ). Should we just hardcode NS_ERROR_NOT_IMPLEMENTED in GetProtocolHandlerInfoFromOS and leave it like that?

Comment 2

[Haik Aftandilian [:haik]]

(In reply to :Gijs (he/him) from comment #1)

So I did a trypush to see if/when we hit this, and it doesn't look like we do in any of our browser mochitests or xpcshell tests ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb1903bc667cccb38f0f629818cb5cd43a440c68 ). Should we just hardcode NS_ERROR_NOT_IMPLEMENTED in GetProtocolHandlerInfoFromOS and leave it like that?

Yes, that sounds fine to me. If we end up needing in child processes, we can add an implementation.

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1589082 - make it clear we don't implement getProtocolHandlerInfoFromOS in the child service on mac, r?haik

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d410872412d5
make it clear we don't implement getProtocolHandlerInfoFromOS in the child service on mac, r=haik

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d410872412d5