1808171 - Add a protoClass argument to JS_InitClass

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Jan de Mooij [:jandem]]

The pull request on GitHub here made me realize we should change this API to not use the same JSClass* argument for the constructor name and the prototype's JSClass.

Callers can then pass nullptr for protoClass to get a PlainObject prototype, which is preferable nowadays.

This also lets us use plain objects for most debugger object prototypes, which simplifies the code a bit because we can then get rid of the isInstance methods.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 1 - Replace JS_InitClass's clasp argument with protoClass/name arguments. r?sfink!

This decouples the two uses of the passed JSClass, to make it easier to use
JS_InitClass when you don't want to use the same JSClass for the prototype
and the instance objects.

Later patches will change most of the callers to pass nullptr for protoClass.

Also adds documentation for JS_InitClass.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 2 - Use PlainObject for Debugger.Source prototype object. r?sfink!

This matches newer language features and lets us simplify the code a bit.

Depends on D165796

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 3 - Use PlainObject for Debugger.Script prototype object. r?sfink!

Depends on D165797

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 4 - Use PlainObject for Debugger.Environment prototype object. r?sfink!

Depends on D165798

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 5 - Use PlainObject for Debugger.Frame prototype object. r?sfink!

Depends on D165799

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 6 - Use PlainObject for Debugger.Object prototype object. r?sfink!

Depends on D165800

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 7 - Use PlainObject for Debugger.Memory prototype object. r?sfink!

Depends on D165801

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 8 - Remove unnecessary JS_InitClass in testAddPropertyHook. r?sfink!

The test doesn't use AddPropertyTester from JS.

Depends on D165802

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 9 - Use PlainObject for some prototypes in jsapi-tests. r?sfink!

Depends on D165803

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1808171 part 10 - Add separate JSClass for Debugger.prototype. r?sfink!

The Debugger.prototype object only needs slots for the proto objects.

Depends on D165804

Comment 11

[gschwind]

Hello Jandem

In your patch it seems that clasp parameter has no more usage instead of clasp->name to give a name to the property that will be created, thus is it sane to replace it by a plain char const * and rename it to some think more accurate ?

Best regards

Comment 12

[Jan de Mooij [:jandem]]

(In reply to gschwind from comment #11)

In your patch it seems that clasp parameter has no more usage instead of clasp->name to give a name to the property that will be created, thus is it sane to replace it by a plain char const * and rename it to some think more accurate ?

It crossed my mind earlier today but I decided to leave the clasp argument alone. However, thinking about it again, it probably make sense to change this at the same time as it does make the API strictly more powerful.

Comment 13

[Jan de Mooij [:jandem]]

I updated the patch stack to replace clasp with name. I think it's a nice improvement.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e8db9e52c91
part 1 - Replace JS_InitClass's clasp argument with protoClass/name arguments. r=sfink
https://hg.mozilla.org/integration/autoland/rev/c74c5cad3260
part 2 - Use PlainObject for Debugger.Source prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/02caeaadffad
part 3 - Use PlainObject for Debugger.Script prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/748479513ae9
part 4 - Use PlainObject for Debugger.Environment prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/870fbc779738
part 5 - Use PlainObject for Debugger.Frame prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/504d4117adb6
part 6 - Use PlainObject for Debugger.Object prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/62c2b515ca18
part 7 - Use PlainObject for Debugger.Memory prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/f2ef812c01e3
part 8 - Remove unnecessary JS_InitClass in testAddPropertyHook. r=sfink
https://hg.mozilla.org/integration/autoland/rev/4b1da3137488
part 9 - Use PlainObject for some prototypes in jsapi-tests. r=sfink
https://hg.mozilla.org/integration/autoland/rev/731a5b8451e8
part 10 - Add separate JSClass for Debugger.prototype. r=sfink

Comment 15

[Noemi Erli[:noemi_erli]]

Backed out 10 changesets (Bug 1808171) for causing failures in test_HeapSnapshot_takeCensus_06.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Copt%2Cxpcshell%2Ctests%2Ctest-linux1804-64-qr%2Fopt-xpcshell%2Cx1&revision=731a5b8451e8ecd2f127e607e503c33f83b1097a&selectedTaskRun=J7h8U81mQ-CUlu42JATFFQ.0

Log: https://treeherder.mozilla.org/logviewer?job_id=401211980&repo=autoland&lineNumber=4033

Backout: https://hg.mozilla.org/integration/autoland/rev/57bed80129e37851a13261bfa4a1b58fe0d0d50d

Comment 16

[Jan de Mooij [:jandem]]

I added the xpcshell-test tasks on Try but apparently the same X1 task on Try did not run this test :/

Maybe a side-effect of ./mach try auto like bug 1714984. That's unfortunate.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15e1e69037e3
part 1 - Replace JS_InitClass's clasp argument with protoClass/name arguments. r=sfink
https://hg.mozilla.org/integration/autoland/rev/ed74b518c904
part 2 - Use PlainObject for Debugger.Source prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/c3106692cf15
part 3 - Use PlainObject for Debugger.Script prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/7fa4a642d8cc
part 4 - Use PlainObject for Debugger.Environment prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/157ca8f34f3d
part 5 - Use PlainObject for Debugger.Frame prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/62adab0ec9eb
part 6 - Use PlainObject for Debugger.Object prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/a729cd4de5c4
part 7 - Use PlainObject for Debugger.Memory prototype object. r=sfink
https://hg.mozilla.org/integration/autoland/rev/54fbbeaf8477
part 8 - Remove unnecessary JS_InitClass in testAddPropertyHook. r=sfink
https://hg.mozilla.org/integration/autoland/rev/888062ee7777
part 9 - Use PlainObject for some prototypes in jsapi-tests. r=sfink
https://hg.mozilla.org/integration/autoland/rev/7527c113748d
part 10 - Add separate JSClass for Debugger.prototype. r=sfink

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/15e1e69037e3
https://hg.mozilla.org/mozilla-central/rev/ed74b518c904
https://hg.mozilla.org/mozilla-central/rev/c3106692cf15
https://hg.mozilla.org/mozilla-central/rev/7fa4a642d8cc
https://hg.mozilla.org/mozilla-central/rev/157ca8f34f3d
https://hg.mozilla.org/mozilla-central/rev/62adab0ec9eb
https://hg.mozilla.org/mozilla-central/rev/a729cd4de5c4
https://hg.mozilla.org/mozilla-central/rev/54fbbeaf8477
https://hg.mozilla.org/mozilla-central/rev/888062ee7777
https://hg.mozilla.org/mozilla-central/rev/7527c113748d