script: Pass &mut JSContext to method of Crypto interface#42809
script: Pass &mut JSContext to method of Crypto interface#42809TimvdLippe merged 1 commit intoservo:mainfrom
&mut JSContext to method of Crypto interface#42809Conversation
This patch changes the method of `Crypto` interface to use the new `&mut JSContext`. The method `crypto.subtle` calls `SubtleCrypto::new` to create an instance of `SubtleCrypto` and the new `&mut JSContext` is passed to `SubtleCrypto::new`. Therefore, the new `reflect_dom_object_with_cx` method is also used in `SubtleCrypto::new`. Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
servo-highfive
added
the
S-awaiting-review
| impl CryptoMethods<crate::DomTypeHolder> for Crypto { | ||
| /// <https://w3c.github.io/webcrypto/#dfn-Crypto-attribute-subtle> | ||
| fn Subtle(&self, can_gc: CanGc) -> DomRoot<SubtleCrypto> { | ||
| fn Subtle(&self, cx: &mut js::context::JSContext) -> DomRoot<SubtleCrypto> { |
There was a problem hiding this comment.
I would import js::context::JSContext at the top of the file:
| fn Subtle(&self, cx: &mut js::context::JSContext) -> DomRoot<SubtleCrypto> { | |
| fn Subtle(&self, cx: &mut JSContext) -> DomRoot<SubtleCrypto> { |
There was a problem hiding this comment.
servo/components/script/dom/crypto.rs
Lines 52 to 56 in 5853926
The function signature of
GetRandomValues generated by script_binding still contain the JSContext from crate::script_runtime (even through it is not actually used).
Do we have conventional alias for them if both are imported?
There was a problem hiding this comment.
I don't know of any convention, but if you are avoiding a name collision, then using the qualified name is probably fine.
There was a problem hiding this comment.
Qualified name is used here mainly for avoiding name collision. We may need to clean it up once the crate::script_runtime::JSContext is fully replaced by js::context::JSContext.
There was a problem hiding this comment.
FYI in all other PRs we currently use the fully qualified written out name (275 usages in 74 files and counting). Therefore, for consistency let's keep using that for now. Later, we can also clean it up?
| pub(crate) fn new(global: &GlobalScope, can_gc: CanGc) -> DomRoot<SubtleCrypto> { | ||
| reflect_dom_object(Box::new(SubtleCrypto::new_inherited()), global, can_gc) | ||
| pub(crate) fn new( | ||
| cx: &mut js::context::JSContext, |
servo-highfive
removed
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
4 participants
This patch changes the method of
Cryptointerface to use the new&mut JSContext.The method
crypto.subtlecallsSubtleCrypto::newto create an instance ofSubtleCryptoand the new&mut JSContextis passed toSubtleCrypto::new. Therefore, the newreflect_dom_object_with_cxmethod is also used inSubtleCrypto::new.Testing: Refactoring. Existing tests suffice.
Fixes: Part of #42638