Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement FormData. #645

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Implement FormData. #645

wants to merge 22 commits into from

Conversation

linsmod
Copy link

@linsmod linsmod commented Aug 13, 2024

In this pull request, FormData Web API and it's qjs binding is implmented.

For prototype, see below:

type FormDataPart={}
export interface FormData {
    new():FormData;
    append(name: string, value: BlobPart, fileName?: string): void;
    // This method name is a placeholder of **delete** method to avoid using C++ keyword
    // and will be replaced to **delete** when installing in MemberInstaller::InstallFunctions.
    form_data_delete(name: string): void;
    get(name: string): BlobPart
    getAll(name: string): BlobPart[];
    has(name: string): boolean;
    set(name: string, value: BlobPart, fileName?: string): void;
    forEach(callbackfn: Function, thisArg?: any): void;
    keys():string[]
    values():BlobPart[]
    entries():FormDataPart[]
}

Todo: Making FetchModule support FormData body.

Todo: Should using fileName parameter in append/set implementation, we do NOT using it currently.

@linsmod linsmod mentioned this pull request Aug 13, 2024
@andycall andycall self-requested a review August 13, 2024 14:08
append(name: string, value: BlobPart, fileName?: string): void;
// This method name is a placeholder of **delete** method to avoid using C++ keyword
// and will be replaced to **delete** when installing in MemberInstaller::InstallFunctions.
form_data_delete(name: string): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unknown API

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append: https://developer.mozilla.org/en-US/docs/Web/API/FormData/append
delete: https://developer.mozilla.org/en-US/docs/Web/API/FormData/delete

"delete" is renamed to "form_data_delete" here to avoid using C++ keyword, otherwise will cause a keyword conflict in generated C++ binding code.

It will be renamed back to "delete" when register into qjs , see: MemberInstaller::InstallFunctions

type FormDataPart={} // dummy code. Real one is introduced in C++ form_data_part.h, also blob_part.h
export interface FormData {
new():FormData;
append(name: string, value: BlobPart, fileName?: string): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  append(name: string, value: BlobPart, fileName?: string): void;
  append(name: string, value: string, fileName?: string): void;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlobPart支持两者,而且内部也可以区分,似乎不必要创建两个函数?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

重载需要写两个,这样生成器也会生成两个重载,来应对不同的参数传递方式

buildAndRunF.sh Outdated
@@ -0,0 +1,21 @@
runexample(){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

delete this scripts

@andycall
Copy link
Member

@linsmod
image

The binding from C++ to Dart with FormData class finished

@linsmod
Copy link
Author

linsmod commented Aug 14, 2024

The binding from C++ to Dart with FormData class finished

Got! Awesome job!

@andycall
Copy link
Member

The implementation of Chrome's FormData support is an good reference.

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/form_data.h

@linsmod
Copy link
Author

linsmod commented Aug 14, 2024

绑定到Dart的代码,是否可以通过直接传递entries的引用/指针来简化?因为FormData已经在C++层面实现了,如果再通过dart实现就重复了。

我就想投机取巧一下:
比如C++一边createBindingObject传递entries_:

FormData::FormData(JSContext* ctx) : BindingObject(ctx) {
  GetExecutingContext()->dartMethodPtr()->createBindingObject(GetExecutingContext()->isDedicated(),
                                                              GetExecutingContext()->contextId(), bindingObject(),
                                                              CreateBindingObjectType::kCreateFormData, &entries_, 0);
}

 private:
  std::vector<std::shared_ptr<FormDataEntry>> entries_;

Dart一边在某种构造函数内把C++传递的entries_指针取出来, 伪代码如下:

class FormData extends DynamicBindingObject {
 List<FormDataEntry> entries = [];
  FormData(BindingContext context, List<dynamic> domMatrixInit): super(context){
    entries=context.args;
}
 

关于创建绑定,还有一个疑问。我看到有一个创建绑定的初始化函数,但是没有看到被调用方的具体实现,只有测试代码中有一个空函数, 从Dart创建NativeBindingObject不是必要的吗?:
void TEST_CreateBindingObject(double context_id, void* native_binding_object, int32_t type, void* args, int32_t argc) {}

@andycall
Copy link
Member

绑定到Dart的代码,是否可以通过直接传递entries的引用/指针来简化?因为FormData已经在C++层面实现了,如果再通过dart实现就重复了。

我就想投机取巧一下: 比如C++一边createBindingObject传递entries_:

FormData::FormData(JSContext* ctx) : BindingObject(ctx) {
  GetExecutingContext()->dartMethodPtr()->createBindingObject(GetExecutingContext()->isDedicated(),
                                                              GetExecutingContext()->contextId(), bindingObject(),
                                                              CreateBindingObjectType::kCreateFormData, &entries_, 0);
}

 private:
  std::vector<std::shared_ptr<FormDataEntry>> entries_;

Dart一边在某种构造函数内把C++传递的entries_指针取出来, 伪代码如下:

class FormData extends DynamicBindingObject {
 List<FormDataEntry> entries = [];
  FormData(BindingContext context, List<dynamic> domMatrixInit): super(context){
    entries=context.args;
}
 

关于创建绑定,还有一个疑问。我看到有一个创建绑定的初始化函数,但是没有看到被调用方的具体实现,只有测试代码中有一个空函数, 从Dart创建NativeBindingObject不是必要的吗?: void TEST_CreateBindingObject(double context_id, void* native_binding_object, int32_t type, void* args, int32_t argc) {}

这么搞的话,问题可多了。跨线程问题,跨 VM 内存管理,Dart FFI 本身的性能问题,内存回收问题。

别瞎想了,再写一遍吧

@linsmod
Copy link
Author

linsmod commented Aug 14, 2024

js 里面 new FormData()并添加数据, 在 dart 里面成了空的Map。

@linsmod
Copy link
Author

linsmod commented Aug 14, 2024

readonly forEach: JSArrayProtoMethod;
被改成这样了之后我就不会了,没有找到任何具体实现例子。

我理解如果要实现JSArrayProtoMethod是缺少一个Iterator的模板的,如果有那个模板,那么代码生成那里改一下,让生成的代码去调用目标的Converter::ToIterator,是不是就行了。

因为我现在想在js那边直接把FormData转成普通JS对象,也就是dart里面的Map,为什么想这么做?因为js传递的FormData在fetch.dart里面就变成了一个空Map,即使js那边添加了数据,在Dart那边看也是空的,那不如将计就计直接用JS对象{}。如果要转{}就需要遍历内容,但因为现在改成了readonly forEach: JSArrayProtoMethod; 我在js那边forEach就取不到内容,空空如也。

(PS:注意到你们似乎在移植blink,那个能搞完,就省很多实现Web标准的功夫了)

@andycall
Copy link
Member

估计哪里搞错了,预期 new FormData 传到 dart 是会变成昨天搞的 FormData Dart 对象的,我来看看为啥

@andycall
Copy link
Member

readonly forEach: JSArrayProtoMethod; 被改成这样了之后我就不会了,没有找到任何具体实现例子。

我理解如果要实现JSArrayProtoMethod是缺少一个Iterator的模板的,如果有那个模板,那么代码生成那里改一下,让生成的代码去调用目标的Converter::ToIterator,是不是就行了。

因为我现在想在js那边直接把FormData转成普通JS对象,也就是dart里面的Map,为什么想这么做?因为js传递的FormData在fetch.dart里面就变成了一个空Map,即使js那边添加了数据,在Dart那边看也是空的,那不如将计就计直接用JS对象{}。如果要转{}就需要遍历内容,但因为现在改成了readonly forEach: JSArrayProtoMethod; 我在js那边forEach就取不到内容,空空如也。

(PS:注意到你们似乎在移植blink,那个能搞完,就省很多实现Web标准的功夫了)

你说的没错,是得单独写个 iterator 才能完整实现这个。readonly forEach: JSArrayProtoMethod; 依赖了 quickjs 自身的特定,类数组的对象能 work,但是对于 FormData 就不凑效了

@andycall
Copy link
Member

@linsmod 重写了 FormData,并重新实现了 Iterator 功能,现在 Foreach 和返回 interator 的函数都 work 了。

估计哪里搞错了,预期 new FormData 传到 dart 是会变成昨天搞的 FormData Dart 对象的,我来看看为啥

现在把 formData 传给 fetch API 已经能在 C++ 那边收到了,接下来就是把这个 FormData 当成指针传递给 Dart。

绑定到Dart的代码,是否可以通过直接传递entries的引用/指针来简化?因为FormData已经在C++层面实现了,如果再通过dart实现就重复了。

这个我想了一下,是可以通过一种简单的方式实现的,Dart FormData 只需要实现一个功能 —— 反向通过 Dart FFI 把 FormData 的 bytes 读回 Dart 环境,然后再这个 bytes 传给 http 模块,这样就搞定了。

所以接下来的任务是:

  1. 将 FormData 转成 NativeValue,然后发送到 Dart 那边,这里需要调整 invokeModule 这个函数
  2. 实现 Dart --> C++ FormData 的通道,实现 HandleCallFromDartSide 函数即可。
  3. 实现 FormData 的 encoding 逻辑,需要将 FormData 的数据结构编码成 MultiPart byte 格式,这里可以参考 blink 的代码:1, 2
  4. 由于 Dart 和 C++ 是运行在两个线程内,从 Dart 里通过 FFI 读取 bytes,再回到 Dart 属于跨线程调用逻辑,需要处理同步问题

@linsmod 接下来就交给你了,我得去搞别人的事情了,加油。

@linsmod
Copy link
Author

linsmod commented Aug 17, 2024

出差了,得过段时间。

@linsmod
Copy link
Author

linsmod commented Aug 23, 2024

编译不过,好像是改动了这里原因:

  NativeValue* invokeModule(bool is_dedicated,
                            void* callback_context,
                            double context_id,
                            int64_t profile_link_id,
                            SharedNativeString* moduleName,
                            SharedNativeString* method,
                            NativeValue* params,
                            uint32_t argc,
                            AsyncModuleCallback callback);
dart_methods.h:143:16: note:   candidate expects 9 arguments, 8 provided
/home/wulin/webf_copy/bridge/core/frame/module_manager.cc:186:52: error: no matching function for call to ‘webf::DartMethodPointer::invokeModule(bool, std::nullptr_t, double, int64_t, std::unique_ptr<webf::SharedNativeString>::pointer, std::unique_ptr<webf::SharedNativeString>::pointer, webf::NativeValue*, webf::NativeValue* (&)(void*, double, const char*, webf::NativeValue*, Dart_Handle, webf::InvokeModuleResultCallback))
  186 |     result = context->dartMethodPtr()->invokeModule(
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  187 |         context->isDedicated(), nullptr, context->contextId(), context->dartIsolateContext()->profiler()->link_id(),
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  188 |         module_name_string.get(), method_name_string.get(), &params, handleInvokeModuleUnexpectedCallback);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/wulin/webf_copy/bridge/core/dart_isolate_context.h:11,
                 from /home/wulin/webf_copy/bridge/core/executing_context.h:26,
                 from /home/wulin/webf_copy/bridge/bindings/qjs/qjs_interface_bridge.h:9,
                 from /home/wulin/webf_copy/bridge/bindings/qjs/generated_code_helper.h:11,
                 from /home/wulin/webf_copy/bridge/out/qjs_module_manager_options.h:9,
                 from /home/wulin/webf_copy/bridge/core/frame/module_manager.h:12,
                 from /home/wulin/webf_copy/bridge/core/frame/module_manager.cc:5:

@andycall
Copy link
Member

andycall commented Aug 25, 2024

编译不过,好像是改动了这里原因:

修好了

@linsmod 可以留个联系方式吗,我开个会议给你讲讲后面怎么搞。 如果可以的话,可以把微信二维码发我邮箱一下

dongtiangche@outlook.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants