refactor: CGClassConstructHook into Handwritten Helpers #31949
Conversation
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
… into extract-contructhook
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
| pub unsafe fn construct_html_custom(global: &GlobalScope, cx: *mut JSContext, args: &CallArgs, descriptor_name: &str) -> bool { | ||
| let global = DomRoot::downcast::<dom::types::Global>(global).unwrap(); // Adjust type as needed | ||
| let prototype_id = match descriptor_name { | ||
| "HTMLDivElement" => PrototypeList::ID::HTMLDivElement, |
There was a problem hiding this comment.
Instead of taking string argument, we should take a PrototypeList::ID argument that we can use directly. The python code can construct the required value.
| pub unsafe fn construct_default_custom(global: &GlobalScope, cx: *mut JSContext, args: &CallArgs, descriptor_name: &str, ctor_name: &str) -> bool { | ||
| let prototype_id = match descriptor_name { | ||
| "HTMLDivElement" => PrototypeList::ID::HTMLDivElement, |
There was a problem hiding this comment.
Same comment about using a PrototypeList::ID argument instead of a string.
| // Add cases for other descriptor names as needed | ||
| _ => panic!("Unsupported descriptor name: {}", descriptor_name), | ||
| }; | ||
| htmlconstructor::call_html_constructor::<dom::types::Global>( |
There was a problem hiding this comment.
Instead of using <dom::types::Global> here, we need to add a generic type to this function (pub unsafe fn construct_html_custom<T: DerivedFrom<Element> + DomObject>) and use that generic type (htmlconstructor::call_html_constructor::<T>).
| use crate::dom::bindings::codegen::PrototypeList::ID; | ||
| // Function for HTML constructor case | ||
| pub unsafe fn construct_html_custom(global: &GlobalScope, cx: *mut JSContext, args: &CallArgs, descriptor_name: &str) -> bool { | ||
| let global = DomRoot::downcast::<dom::types::Global>(global).unwrap(); // Adjust type as needed |
There was a problem hiding this comment.
I believe this needs to be a downcast to Window, not Global, for the call to call_html_constructor to succeed.
| args, | ||
| &global, | ||
| prototype_id, // Adjust as needed | ||
| descriptor_name, |
There was a problem hiding this comment.
We're missing the creator argument here. We should accept that as an argument in construct_html_custom and let the python code pass the appropriate function value like it does before the changes in this PR.
| cx, | ||
| args, | ||
| prototype_id, // Adjust as needed | ||
| descriptor_name, |
There was a problem hiding this comment.
Same comment about the creator argument here as earlier. This should be passed in by the python code.
| return false; | ||
| } | ||
|
|
||
| let nativeName = MakeNativeName(ctor_name); |
There was a problem hiding this comment.
MakeNativeName is a python function. This is going to be a bit tricky; I think we're going to need to leave the actual constructor call in the generated python code, because there's a lot of unique logic in there. What we can do is something like this:
pub unsafe fn construct_default_custom(..., constructor: impl FnOnce(*mut JSContext, &CallArgs, &GlobalScope) -> bool) -> bool {
...
constructor(cx, args, global)
}Then modify the python code to call f"construct_default_custom(&global, cx, &args, \"{ctorName}\", |cx, args, global| {{ {constructorInvoke} }});\n" where constructorInvoke is the result of calling .define() on the constructorCall CGMethodCall value.
|
@rohansx Do you plan to keep working on this one? |
Yes.. I'll work on this issue @mrobinson |
Extracted the constructor hook generated by CGClassConstructHook into two new, handwritten generic helper functions. This approach follows the model established in Extract generated finalizers into generic helper functions. #31569.
The new helper functions are designed to handle both cases of isHTMLConstructor being true or false, ensuring compatibility with the existing codebase while enhancing readability.
Removed the Python script responsible for generating the previous, less readable Rust code, replacing it with our new, cleaner solution.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThese changes fix Extract generated class constructor hook into handwritten functions #31571