Closed eoger closed 4 years ago
Unpack/Pack types at the rust bridge level, and use JSON to talk to JS
Does this have to be JSON, or could we create a live Object that gets shared back to JS via the XPCOM bindings? I don't know enough about the nitty-gritty details of the bridge layer.
Unpack/Pack types at the rust bridge level, and use JSON to talk to JS
Does this have to be JSON, or could we create a live Object that gets shared back to JS via the XPCOM bindings? I don't know enough about the nitty-gritty details of the bridge layer.
Yeah that's a good idea, maybe? We should explore our solutions here!
I realized that we don't need this since we don't even use the ffi 😅, however the original issue could still be re-phrased as "Figure out how to pass complex values back from Rust to JS". After talking to @linacambridge, seems like there's 2 methods: JSON stringify or return a nsIPropertyBag
. I went with the first option!
Adding a bit more from our Slack discussion for context! 😁 There are a few ways to pass complex objects between native and JS code:
jsval
. Unfortunately, this doesn't work in Rust, because there are no JS engine bindings for rust-xpcom. (There is a Rust SpiderMonkey API crate in js/rust
, but last I heard, it's only really for Servo, and breaks somewhat often without regular maintenance). We would also need to change rust-xpcom to handle jsval
s; it currently opts out of generating stubs for methods that take or return them.webidl
-declared dictionary. Same problem as above. Dictionaries are much easier to use from C and C++, but the C++ bindings are autogenerated (which means we'd need to generate them for Rust; Codegen.py
currently only outputs C++), and they also require a JS context. These are easier to use than jsval
s, and the code they generate is super efficient, but even more complicated to implement.nsIPropertyBag
with nsIVariant
values. Currently, the only variant types we support in Rust are strings, Booleans, numbers, and nulls. We'd need to beef this up to include arrays, and recursive nsIPropertyBag
s (limited interface types, where we only support other bags and variants) if we wanted to support nested objects. Property bags are kludgey to construct in JS and kludgey to use just about anywhere, but we can add helpers to make it less so!webidl
support, this is probably the most efficient way: JSON.parse
in JS is lightning-fast, and lets us pass any types from Rust that JS can understand. There's precedent for doing this in Gecko, too, where passing a more elaborate interface type would be inconvenient. It does mean more string allocations—though, we don't have to copy when we convert a Rust String
to a Gecko nsCString
—and more invariants that we have to uphold on the Rust and JS sides, since it's unstructured data. It also requires a JS wrapper...though, given that the XPCOM API we're exposing is pretty low-level, we'd want that anyway.Based on that, I agree, JSON strings are the way to go!
Thanks for that write up!
Based on that, I agree, JSON strings are the way to go!
I got the impression from some earlier comments from @thomcc that there are protobuf
What's our strategy here going forward? Should we back away from protobufs everywhere? If not, should we develop some docs with guidance?
Oh, interesting, I didn't realize there were issues in our existing components!
One other thing that makes JSON convenient to use on Desktop is, we're talking to JavaScript, so native JSON parsing is 1) available, and 2) going to be easier to use and faster than any other format, like protobuf. But that's not the case with Kotlin and Swift.
Whatever we decide, I think developing some docs with guidance around this makes a lot of sense! One of my Q2 goals is to document how to land Rust code in Desktop, and this would be great to add as a "how should I pass stuff back and forth?" section.
Some fxa methods output protocol buffers, and some of them even take protobufs as input. We need to figure out how to unpack/pack protobuf messages in the gecko world. Some ideas:
┆Issue is synchronized with this Jira Task ┆Epic: Choose What to Sync accepted and declined engines ┆Sprint: Backlog