dawn_node: Implement the [SameObject] IDL attribute

This is done by moving the set up of these attributes to the place where
the wrapper objects are created, by doing:

  jsObject.DefineProperty("foo", impl->getOnceFoo());

Three alternatives that weren't chosen are:

 - Caching a weak reference to the member's Napi::Value on the wrapper
   struct, and recreate it only as needed. This is good because it
   doesn't risk using the value after it is GCed, but it can result in
   multiple calls to the getters, which could be unexpected (for example
   for GPUDevice.lost in a follow-up CL).
 - Caching a persistent reference to the member's Napi::Value on the
   wrapper struct. This calls the getter once and doesn't risk using the
   value after it is GCed. However if Javascript does something like
   `myGPUDevice.limits.device = myGPUDevice`, a cycle is created that
   the GC doesn't have visibility into, and that can't be collected.
   (the origin of the edge of the reference graph that persistent
   references make is unknown to the GC).
 - Caching the member on a hidden variable of the JS object. I didn't
   find a way to do this. The closest would have been to do
   jsObject[Symbol(...)] = cachedValue but even symbols can be retrieved
   with Object.getOwnPropertySymbols.

Bug: dawn:1123
Fixed: dawn:1144
Change-Id: I1bc82dd9d10be95bf2bdca73bdfb843bc556d2df
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/85361
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2022-04-01 11:39:26 +00:00 committed by Dawn LUCI CQ
parent 76517cfc32
commit ecc3fe6f0f
3 changed files with 32 additions and 2 deletions

View File

@ -135,7 +135,6 @@ namespace wgpu::binding {
} }
interop::Interface<interop::GPUQueue> GPUDevice::getQueue(Napi::Env env) { interop::Interface<interop::GPUQueue> GPUDevice::getQueue(Napi::Env env) {
// TODO(crbug.com/dawn/1144): Should probably return the same Queue JS object.
return interop::GPUQueue::Create<GPUQueue>(env, device_.GetQueue(), async_); return interop::GPUQueue::Create<GPUQueue>(env, device_.GetQueue(), async_);
} }

View File

@ -149,10 +149,12 @@ Wrappers* Wrappers::instance = nullptr;
InstanceMethod("{{$m.Name}}", &W{{$.Name}}::{{$m.Name}}), InstanceMethod("{{$m.Name}}", &W{{$.Name}}::{{$m.Name}}),
{{- end}} {{- end}}
{{- range $a := AttributesOf $}} {{- range $a := AttributesOf $}}
{{- if not (HasAnnotation $a "SameObject")}}
InstanceAccessor("{{$a.Name}}", &W{{$.Name}}::get{{Title $a.Name}}, InstanceAccessor("{{$a.Name}}", &W{{$.Name}}::get{{Title $a.Name}},
{{- if $a.Readonly}} nullptr{{else}} &W{{$.Name}}::set{{Title $a.Name}}{{end -}} {{- if $a.Readonly}} nullptr{{else}} &W{{$.Name}}::set{{Title $a.Name}}{{end -}}
), ),
{{- end}} {{- end}}
{{- end}}
{{- range $c := ConstantsOf $}} {{- range $c := ConstantsOf $}}
StaticValue("{{$c.Name}}", ToJS(env, {{$.Name}}::{{$c.Name}}), napi_default_jsproperty), StaticValue("{{$c.Name}}", ToJS(env, {{$.Name}}::{{$c.Name}}), napi_default_jsproperty),
{{- end}} {{- end}}
@ -219,6 +221,7 @@ Wrappers* Wrappers::instance = nullptr;
{{- end}} {{- end}}
{{- range $a := AttributesOf $}} {{- range $a := AttributesOf $}}
{{- if not (HasAnnotation $a "SameObject")}}
Napi::Value get{{Title $a.Name}}(const Napi::CallbackInfo& info) { Napi::Value get{{Title $a.Name}}(const Napi::CallbackInfo& info) {
return ToJS(info.Env(), impl->get{{Title $a.Name}}(info.Env())); return ToJS(info.Env(), impl->get{{Title $a.Name}}(info.Env()));
} }
@ -233,7 +236,8 @@ Wrappers* Wrappers::instance = nullptr;
Napi::Error::New(info.Env(), res.error).ThrowAsJavaScriptException(); Napi::Error::New(info.Env(), res.error).ThrowAsJavaScriptException();
} }
} }
{{- end }} {{- end}}
{{- end}}
{{- end}} {{- end}}
}; };
{{end}} {{end}}
@ -331,6 +335,16 @@ Interface<{{$.Name}}> {{$.Name}}::Bind(Napi::Env env, std::unique_ptr<{{$.Name}}
auto object = wrappers->{{$.Name}}_ctor.New({}); auto object = wrappers->{{$.Name}}_ctor.New({});
auto* wrapper = Wrappers::W{{$.Name}}::Unwrap(object); auto* wrapper = Wrappers::W{{$.Name}}::Unwrap(object);
wrapper->impl = std::move(impl); wrapper->impl = std::move(impl);
{{- /*Add the [SameObject] members as read-only property on the JS object.*/ -}}
{{- range $a := AttributesOf $}}
{{- if HasAnnotation $a "SameObject"}}
object.DefineProperty(Napi::PropertyDescriptor::Value(
"{{$a.Name}}", ToJS(env, wrapper->impl->get{{Title $a.Name}}(env))
));
{{- end}}
{{- end}}
return Interface<{{$.Name}}>(object); return Interface<{{$.Name}}>(object);
} }

View File

@ -108,6 +108,7 @@ func run() error {
"ConstantsOf": constantsOf, "ConstantsOf": constantsOf,
"EnumEntryName": enumEntryName, "EnumEntryName": enumEntryName,
"Eval": g.eval, "Eval": g.eval,
"HasAnnotation": hasAnnotation,
"Include": g.include, "Include": g.include,
"IsBasicLiteral": is(ast.BasicLiteral{}), "IsBasicLiteral": is(ast.BasicLiteral{}),
"IsConstructor": isConstructor, "IsConstructor": isConstructor,
@ -493,6 +494,22 @@ func enumEntryName(s string) string {
return "k" + strings.ReplaceAll(pascalCase(strings.Trim(s, `"`)), "-", "") return "k" + strings.ReplaceAll(pascalCase(strings.Trim(s, `"`)), "-", "")
} }
func findAnnotation(list []*ast.Annotation, name string) *ast.Annotation {
for _, annotation := range list {
if annotation.Name == name {
return annotation
}
}
return nil
}
func hasAnnotation(obj interface{}, name string) bool {
if member, ok := obj.(*ast.Member); ok {
return findAnnotation(member.Annotations, name) != nil
}
panic("Unhandled AST node type in hasAnnotation")
}
// Method describes a WebIDL interface method // Method describes a WebIDL interface method
type Method struct { type Method struct {
// Name of the method // Name of the method