As a counterpart to bug 1660393, for bug 1660392 we also need a sync access to Resource Protocol from Rust.

I took the JSM file - https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/intl/l10n/L10nRegistry.jsm#681-714

I got this function to look like this:

    NS_IMETHODIMP
    L10nRegistry::LoadSync(const nsACString& aPath, nsACString& aRetVal) {
      nsCOMPtr<nsIURI> uri;
    ​
      nsresult rv = NS_NewURI(getter_AddRefs(uri), aPath);
      NS_ENSURE_SUCCESS(rv, rv);
    ​
      NS_ENSURE_TRUE(uri, NS_ERROR_INVALID_ARG);
    ​
      auto result = URLPreloader::ReadURI(uri);
      if (result.isOk()) {
        aRetVal = result.unwrap();
        return NS_OK;
      }
    ​
      auto err = result.unwrapErr();
      if (err != NS_ERROR_INVALID_ARG && err != NS_ERROR_NOT_INITIALIZED) {
        return err;
      }
    ​
      nsCString data;
    ​
      nsCOMPtr<nsIChannel> channel;
      rv = NS_NewChannel(
          getter_AddRefs(channel), uri, nsContentUtils::GetSystemPrincipal(),
          nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_SEC_CONTEXT_IS_NULL,
          nsIContentPolicy::TYPE_OTHER, nullptr, /* nsICookieJarSettings */
          nullptr,                               /* aPerformanceStorage */
          nullptr,                               /* aLoadGroup */
          nullptr,                               /* aCallbacks */
          nsIRequest::LOAD_BACKGROUND);
    ​
      nsCOMPtr<nsIInputStream> input;
      rv = channel->Open(getter_AddRefs(input));
      NS_ENSURE_SUCCESS(rv, NS_ERROR_INVALID_ARG);
    ​
      rv = NS_ReadInputStreamToString(input, data, -1);
      return rv;
    }

I'll fine tune errors and will make it return information about whether the file was missing separately from any other errors.

In Rust, I'd like it to be exposed as fetch_sync(path: &str) -> Result<Option<String>, ?>

@djg is doing something similar for async I/O (which is JSMs Fetch).

I have several questions about it:

1. Do we have sync a generic Sync I/O that can handle omni.ja and use URLPreloader and falls back on nsChannel?
2. If not, would we want to have it for C++?
3. How do we want to expose it to Rust? xpcom-rs? xpcom-io? Or custom L10nRegistry API?

@froydnj - can you advise, NI other stakeholders?

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #1)

I have several questions about it:

1. Do we have sync a generic Sync I/O that can handle omni.ja and use URLPreloader and falls back on nsChannel?

Not that I know of. Instances of URIPreloader::ReadURI (string bundles, style loader) suggest that they have approximately what you sketched out above, but perhaps customized to their particular uses.

2. If not, would we want to have it for C++?

Is there some new usecase that really wants a sync API? It's not obvious to me that all callers of the API are going to want the same parameters to be passed to NS_NewChannel...but at that point it almost feels like people might as well write out the NS_NewChannel calls themselves?

3. How do we want to expose it to Rust? xpcom-rs? xpcom-io? Or custom L10nRegistry API?

I do not have strong feelings on this matter.

Not that I know of. Instances of URIPreloader::ReadURI (string bundles, style loader) suggest that they have approximately what you sketched out above, but perhaps customized to their particular uses.

Unfortunately, there are cases where URLPreloader doesn't work, and Mossop fixed it in bug 1586216 for L10nRegistry by introducing a fallback onto a synchronous channel.

If this is L10nRegistry specific, I'm ok keeping it in L10nRegistry, but it seems to me like there's not much specific about this use case and such fallback maybe should be set for all/most uses of URLPreloader?

If so, I'd like to think of generic XPCOM Fetch Sync/Async.

If you don't think it's worth it, I'm happy to maintain both in intl/l10n/L10nRegistry.cpp for my own use.

It's not obvious to me that all callers of the API are going to want the same parameters to be passed to NS_NewChannel...but at that point it almost feels like people might as well write out the NS_NewChannel calls themselves?

I could generalize the API enough to let people pass some channel arguments that are relevant to URLPreloader fallback. But once again, the bigger question is if we do have an issue with DYI I/O callsites around our codebase and if it's worth unifying it.

It seems to me, from your comment, that you don't see much value in any centralization, so I'm ok going for L10nRegistry specific Sync/Async Fetch API.

Which leaves us with a question of how to expose it to Rust. Again, we could expose it in a generic way, so that all Rust callers can use it (esp. since Async Fetch is non-trivial), but it feels awkward keeping the C++ bits in L10nRegistry while exposing the Rust side on some generic xpcom-rs.
So maybe the answer is that we should keep our Fetch (both Sync and Async) in intl/l10n/L10nRegistry.cpp and intl/l10n/rust/l10nregistry/.

And if we encounter more uses of such API we can extract it into some generic tooling architecture. How does it sound?

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)

So maybe the answer is that we should keep our Fetch (both Sync and Async) in intl/l10n/L10nRegistry.cpp and intl/l10n/rust/l10nregistry/.

And if we encounter more uses of such API we can extract it into some generic tooling architecture. How does it sound?

I'm happy with this approach.

(In reply to Dan Glastonbury (:djg) | needinfo me from comment #4)

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)

So maybe the answer is that we should keep our Fetch (both Sync and Async) in intl/l10n/L10nRegistry.cpp and intl/l10n/rust/l10nregistry/.

And if we encounter more uses of such API we can extract it into some generic tooling architecture. How does it sound?

I'm happy with this approach.

Ditto.

This has been fixed as part of bug 1660393