At the point of ClassSpec delegation was introduced (bug 861219), ClassSpec was embedded into Class, and I think that was one of the main reason why delegation was introduced. now ClassSpec is separated from Class (bug 1260984), and we can just point other ClassSpec, perhaps a ClassSpec from multiple Class'es. so, in some case, we could remove delegation, depending on `flags` member.
If ClassSpec is delegated the following method just returns delegated ClassSpec's one: * createConstructorHook (for createConstructor_) * createPrototypeHook (for createPrototype_) * constructorFunctions (for constructorFunctions_) * constructorProperties (for constructorProperties_) * prototypeFunctions (for prototypeFunctions_) * prototypeProperties (for prototypeProperties_) * finishInitHook (for finishInit_) and the following methods are used only internally: * defined always true if delegated() is true, also true for all existing delegated ClassSpecs * PromiseObjectClassSpec * DateObjectClassSpec * ArrayBufferObjectClassSpec * TypedArrayObjectClassSpecs * delegated * delegatedClassSpec The only difference are the following 2 methods, that behaves differently depending on |flags| field * inheritanceProtoKey depends |flags & ProtoKeyMask| * shouldDefineConstructor depends |flags & DontDefineConstructor| If above flags are same between delegation source and target, we can just point delegated ClassSpec instead of using IsDelegated. Here's current consumers: * Promise source: PromiseObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: PromiseObjectClassSpec target.flags: 0 * Date source: DateObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: DateObjectClassSpec target.flags: 0 * ArrayBuffer source: ArrayBufferObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: ArrayBufferObjectClassSpec target.flags: 0 * SharedArrayBuffer source: SharedArrayBufferObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: ArrayBufferObjectClassSpec target.flags: 0 * %TypedArray% source: TypedArrayObjectProtoClassSpecs source.flags: JSProto_TypedArray | ClassSpec::IsDelegated target: TypedArrayObjectClassSpecs target.flags: JSProto_TypedArray |flags & ProtoKeyMask| and |flags & DontDefineConstructor| are same in all cases, and apparently we don't need delegation for current usage.
I forgot 2 new consumers * Map source: MapObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: MapObject::classSpec_ target.flags: 0 * Set source: SetObjectProtoClassSpec source.flags: ClassSpec::IsDelegated target: SetObject::classSpec_ target.flags: 0 so they're also okay.
try run that removes delegation https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f7854959afe4b049d61a24c43abc9c478d7735
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Created attachment 8812645 [details] [diff] [review] Remove ClassSpec delegation and point target ClassSpec directly from prototype class. Partially backed out bug 861219 patch that introduces delegation, so now there's no delegation. and also changed the reference to ClassSpec with ClassSpec::IsDelegated to the reference to delegation target. and moved prototype class to class static member to avoid forward declaration in cpp files. https://treeherder.mozilla.org/#/jobs?repo=try&revision=30f7854959afe4b049d61a24c43abc9c478d7735
Attachment #8812645 - Flags: review?(jdemooij)
Comment on attachment 8812645 [details] [diff] [review] Remove ClassSpec delegation and point target ClassSpec directly from prototype class. Review of attachment 8812645 [details] [diff] [review]: ----------------------------------------------------------------- This is a great refactoring and it really simplifies things. Nice work!
Attachment #8812645 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/177b7924440c450525d1127c9c2f1d3d642e4a2c Bug 1318815 - Remove ClassSpec delegation and point target ClassSpec directly from prototype class. r=jandem
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.