Closed
Bug 1116269
Opened 6 years ago
Closed 6 years ago
Add WebCrypto to GlobalProperties for import
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: mt, Assigned: mt)
References
Details
(Keywords: dev-doc-complete)
Attachments
(9 files, 1 obsolete file)
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
39 bytes,
text/x-review-board-request
|
rbarnes
:
review+
|
Details |
I'm looking to implement bug 975144, there is a good chance that the sandboxed code will need to run some crypto functions. WebCrypto is nicely self-contained, so this should be straightforward to expose.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8542372 -
Flags: review?(rlb)
Attachment #8542372 -
Flags: review?(peterv)
Assignee | ||
Comment 2•6 years ago
|
||
/r/1791 - Bug 1116269 - Crypto/SubtleCrypto only depend on nsIGlobalObject /r/1793 - Bug 1116269 - Add 'crypto' to sandbox global properties Pull down these commits: hg pull review -r cde3bd84767b1c25c885fbc02ad88814325b2054
Comment 3•6 years ago
|
||
Comment on attachment 8542372 [details]
MozReview Request: bz://1116269/mt
Gabor is probably the best alternative xpconnect reviewer these days.
This looks good at first glance, but I didn't have time to audit things very carefully (especially the crypto-side changes), so bouncing to gabor.
Attachment #8542372 -
Flags: review?(peterv) → review?(gkrizsanits)
Comment 4•6 years ago
|
||
https://reviewboard.mozilla.org/r/1789/#review1213 ::: js/xpconnect/src/Sandbox.cpp (Diff revision 1) > + return JS_DefineProperty(cx, obj, "crypto", wrapped, JSPROP_ENUMERATE); Please be consistent with naming here. Either call it WebCrypto or crypto everywhere (not sure which is the name of the ctor on a regular DOM window but you should use the same here too) Patches look great but I'm missing a simple test at least from it. Could you please add one? (I would do an xpcshell test under js/xpconnect)
Comment 5•6 years ago
|
||
Comment on attachment 8542372 [details]
MozReview Request: bz://1116269/mt
I've did the review on review board, please flag me back once you added the test and fixed the naming issue I mentioned there.
Attachment #8542372 -
Flags: review?(gkrizsanits)
Assignee | ||
Comment 6•6 years ago
|
||
https://reviewboard.mozilla.org/r/1789/#review1215 > Please be consistent with naming here. Either call it WebCrypto or crypto everywhere (not sure which is the name of the ctor on a regular DOM window but you should use the same here too) The name of the API is WebCrypto; the name of the property on window is "crypto". I'm trying to ensure that the property is named consistently. I can't call the flag "Crypto", since that conflicts with the name of the Crypto class that this instantiates. Do you have any suggestions? I don't know how to link to files in reviewboard (I'm only just learning still), but I guess that you missed [test_crypto.js](https://reviewboard.mozilla.org/r/1789/diff/7).
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #5) > I've did the review on review board, please flag me back once you added the > test and fixed the naming issue I mentioned there. It looks like reviewboard is adding comments to bugzilla (I'm only just learning how this works myself). It doesn't quote properly though.
Assignee | ||
Updated•6 years ago
|
Attachment #8542372 -
Flags: review?(gkrizsanits)
Assignee | ||
Comment 8•6 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=96065fa5b34f - all green
Comment 9•6 years ago
|
||
(In reply to Martin Thomson [:mt] from comment #7) > It looks like reviewboard is adding comments to bugzilla (I'm only just > learning how this works myself). It doesn't quote properly though. Right, I'm totally new to this too... (In reply to Martin Thomson [:mt] from comment #6) > The name of the API is WebCrypto; the name of the property on window is > "crypto". I'm trying to ensure that the property is named consistently. I > can't call the flag "Crypto", since that conflicts with the name of the > Crypto class that this instantiates. Do you have any suggestions? The flag should not have capital C, just simply "crypto" as the property on the global. And then it shouldn't conflict with the class name, or am I missing something? The idea is that those bool flags (atob/File/Blob/...) named exactly as the properties will appear on the global (all lower case in this case). Not a regular coding style, but quite useful there, so I wanted to preserve it... And sorry about the test, probably I just looked it over as I'm not too familiar with this review-board yet. It looks great!
Updated•6 years ago
|
Attachment #8542372 -
Flags: review?(gkrizsanits) → review+
Assignee | ||
Updated•6 years ago
|
Attachment #8542372 -
Flags: review+ → review?(peterv)
Assignee | ||
Comment 10•6 years ago
|
||
/r/1791 - Bug 1116269 - Crypto/SubtleCrypto only depend on nsIGlobalObject, r=rbarnes /r/1793 - Bug 1116269 - Add 'crypto' to sandbox global properties, r=gabor Pull down these commits: hg pull review -r 6c2ea4e0ab880c38c90828ce42eca784b2791a77
Assignee | ||
Updated•6 years ago
|
Attachment #8542372 -
Flags: review?(peterv)
Assignee | ||
Comment 11•6 years ago
|
||
/r/1791 - Bug 1116269 - Crypto/SubtleCrypto only depend on nsIGlobalObject, r=rbarnes /r/1793 - Bug 1116269 - Add 'crypto' to sandbox global properties, r=gabor Pull down these commits: hg pull review -r 6c2ea4e0ab880c38c90828ce42eca784b2791a77
Assignee | ||
Comment 12•6 years ago
|
||
/r/1791 - Bug 1116269 - Crypto/SubtleCrypto only depend on nsIGlobalObject, r=rbarnes /r/1793 - Bug 1116269 - Add 'crypto' to sandbox global properties, r=gabor /r/1865 - Bug 975144 - WebIDL changes for RTC identity sandbox /r/1867 - Bug 975144 - Implementation of RTC identity DOM component /r/1869 - Bug 975144 - Rework RTC identity to use JS sandbox /r/1871 - Bug 975144 - Updating test IdP for new API /r/1873 - Bug 975144 - Updating RTC identity tests /r/1875 - Bug 975144 - Adding rtcIdentityProvider to sandbox global scope /r/1877 - Bug 975144 - Tests for rtcIdentityProvider property Pull down these commits: hg pull review -r 8c84952758cc934e83e42129b5e57938f3642e5d
Assignee | ||
Comment 13•6 years ago
|
||
OK, Richard, please disregard reviewboard being stupid. Can you look at https://reviewboard.mozilla.org/r/1791/ please?
Flags: needinfo?(rlb)
Updated•6 years ago
|
Flags: needinfo?(rlb)
Attachment #8542372 -
Flags: review?(rlb) → review+
Assignee | ||
Comment 14•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b206ea15cb59 https://hg.mozilla.org/integration/mozilla-inbound/rev/da03d6aff37d
Comment 15•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b206ea15cb59 https://hg.mozilla.org/mozilla-central/rev/da03d6aff37d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 17•6 years ago
|
||
I've updated https://developer.mozilla.org/en-US/docs/Components.utils.importGlobalProperties
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 18•6 years ago
|
||
Attachment #8542372 -
Attachment is obsolete: true
Attachment #8618986 -
Flags: review+
Attachment #8618987 -
Flags: review+
Attachment #8618988 -
Flags: review+
Attachment #8618989 -
Flags: review+
Attachment #8618990 -
Flags: review+
Attachment #8618991 -
Flags: review+
Attachment #8618992 -
Flags: review+
Attachment #8618993 -
Flags: review+
Attachment #8618994 -
Flags: review+
Assignee | ||
Comment 19•6 years ago
|
||
Assignee | ||
Comment 20•6 years ago
|
||
Assignee | ||
Comment 21•6 years ago
|
||
Assignee | ||
Comment 22•6 years ago
|
||
Assignee | ||
Comment 23•6 years ago
|
||
Assignee | ||
Comment 24•6 years ago
|
||
Assignee | ||
Comment 25•6 years ago
|
||
Assignee | ||
Comment 26•6 years ago
|
||
Assignee | ||
Comment 27•6 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•