Closed Bug 1127443 Opened 5 years ago Closed 5 years ago

Remove proto parameter from JS_NewObject

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: evilpie, Assigned: evilpie)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

This parameter is kind of evil. If you pass null we search for the right proto, but otherwise it behaves just like calling JS_NewObjectWithGivenProto.
Assignee: nobody → evilpies
Moves callers that use the JS_NewObject proto argument to JS_NewObjectWithGivenProto
Attachment #8556590 - Flags: review?(jwalden+bmo)
Attachment #8556590 - Flags: review?(bzbarsky)
Attachment #8556593 - Flags: review?(jwalden+bmo)
Attachment #8556593 - Flags: review?(bzbarsky)
If you passed a non-null object as proto to JS_NewObject, we would already omit this.
Attachment #8556596 - Flags: review?(jdemooij)
Comment on attachment 8556590 [details] [diff] [review]
Move some JS_NewObject callers to JS_NewObjectWithGiveProto

Why the behavior change in InitTearOffJSObject?
Flags: needinfo?(evilpies)
I assume the "don't mark" changeset is landing first, by the way?
Using JS_GetObjectPrototype as the proto, is the same as using JS::NullPtr(). Just did this change here so I could simplify in the next bug.

I was going to land all the patches in one go, but I guess I can land that first.
Flags: needinfo?(evilpies)
Comment on attachment 8556593 [details] [diff] [review]
Remove JS_NewObject proto argument.

Mind filing a bout on the GlobalResolve bit?  I'm 99% sure "obj" there must be cx->global() so we can drop that parent arg.

> xpc::NewOutObject(JSContext* cx, JSObject* scope)

Pretty sure that scope and cx are same-compartment here, so we could just drop the parent arg to JS_NewObject here as well, as well as dropping the "scope" argument to NewOutObject.  File a followup on that?

r=me on the changes outside js/src
Attachment #8556593 - Flags: review?(bzbarsky) → review+
Comment on attachment 8556590 [details] [diff] [review]
Move some JS_NewObject callers to JS_NewObjectWithGiveProto

Oh, indeed, JS_GetObjectPrototype, not JS_GetPrototype.  Silly me!

r=me on the non-js/src bits
Attachment #8556590 - Flags: review?(bzbarsky) → review+
Comment on attachment 8556596 [details] [diff] [review]
Don't mark objects created by JS_NewObjectWithGivenProto as having unknown properties.

Review of attachment 8556596 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, didn't know we had that call.
Attachment #8556596 - Flags: review?(jdemooij) → review+
Attachment #8556590 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8556593 [details] [diff] [review]
Remove JS_NewObject proto argument.

Review of attachment 8556593 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/ExportHelpers.cpp
@@ +463,5 @@
>  
>      RootedObject obj(cx);
>      {
>          JSAutoCompartment ac(cx, scope);
> +        obj = JS_NewObject(cx, nullptr, scope);

This can be JS_NewPlainObject, right?

::: js/xpconnect/src/XPCWrappedJSClass.cpp
@@ +1466,5 @@
>  JSObject*
>  xpc::NewOutObject(JSContext* cx, JSObject* scope)
>  {
>      RootedObject global(cx, JS_GetGlobalForObject(cx, scope));
> +    return JS_NewObject(cx, nullptr, global);

And here.
Attachment #8556593 - Flags: review?(jwalden+bmo) → review+
Tom, can you update the documentation according to the modification made in this bug?

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_NewObject
Flags: needinfo?(evilpies)
Keywords: dev-doc-needed
Thanks for reminding me. The documentation here is already super confusing, but I tried to do some small cleanup.
Flags: needinfo?(evilpies)
You need to log in before you can comment on or make changes to this bug.