827829 - Implement window.crypto.getRandomValues for B2G

Status: RESOLVED DUPLICATE of bug 673432

(Core :: DOM: Core & HTML, defect)

Description

[David Dahl :ddahl]

With the patches in bug 440046 and bug 683262 we are re-creating the crypto property for Firefox Mobile and adding the crypto.getRandomValues() property. We also need both window.crypto and getRandomValues() for B2G.

Comment 1

[David Dahl :ddahl]

The implementation of adding window.crypto for B2G seems like it will be vastly different as we need to use the message manager to talk to content. Should this patch be patterned after other web apis that use JS to create the window property and the message manager to talk (a)synchronously to the parent process JSM that does all of the random generation?

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This is a dupe of bug 673432, which we already put a lot of effort into. Please re-open if you disagree.

(I thought we already had a working, or near-working implementation at one point. I remember khuey helped tell us what to do.)

Comment 3

[David Dahl :ddahl]

(In reply to Brian Smith (:bsmith) from comment #2)
> This is a dupe of bug 673432, which we already put a lot of effort into.
> Please re-open if you disagree.
> 
> (I thought we already had a working, or near-working implementation at one
> point. I remember khuey helped tell us what to do.)

No, not the case. The other bug was for Firefox Mobile, which bug 683262 and bug 440046 take care of. In the meantime, so many new Web APIs have come into play for FirefoxOS - and almost all of them are written in JS using the message manager. I am wondering if that should also be the case with window.crypto? It seems like a much easier path to me.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Isn't getRandomValues rather performance critical? If so, non-messageManager based implementation
would make more sense.
Also from correctness point of view implementing APIs using C++ is currently better.

Comment 5

[David Dahl :ddahl]

(In reply to Olli Pettay [:smaug] from comment #4)
> Isn't getRandomValues rather performance critical? If so, non-messageManager
> based implementation
> would make more sense.
Indeed, it is implemented in C++ already. See bug 440046

> Also from correctness point of view implementing APIs using C++ is currently
> better.

I am only talking about attaching the crypto object and its properties to the DOM. All of the underlying methods (getRandomValues, etc) are or will be written in C++

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

(In reply to David Dahl :ddahl from comment #5)

> I am only talking about attaching the crypto object and its properties to
> the DOM.
So am I. That is the part where C++ implementations do more correct thing, especially if
WebIDL bindings can be used. But perhaps it doesn't matter too much atm since window object
isn't WebIDL-fied yet.
Anyhow, adding a new property to window using xpidl+c++ is simpler than using JS.

Comment 7

[David Dahl :ddahl]

(In reply to Olli Pettay [:smaug] from comment #6)
> (In reply to David Dahl :ddahl from comment #5)

> Anyhow, adding a new property to window using xpidl+c++ is simpler than
> using JS.
Ok, cool. Is the approach in bug 673432 correct? https://bugzilla.mozilla.org/attachment.cgi?id=635501&action=diff

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Yes, something like that. ContentParent::RecvGetRandomValues would then call the
GetRandomValues in that process and return the result.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

B2G e10s is the same as Fennec XUL e10s so the work in the other bug will work for B2G.

AFAICT, the reason the C++ is better is that we have proper nsIDOMClassInfo which means things like |"crypto" in window| and |"getRandomValues" in window.crypto| work. (IIRC, those are required for the tests to pass too.)

Comment 10

[David Dahl :ddahl]

(In reply to Brian Smith (:bsmith) from comment #9)
> B2G e10s is the same as Fennec XUL e10s so the work in the other bug will
> work for B2G.
> 
Thanks, closing this now.