Nix the parent argument from JS_NewObjectWithGivenProto; convert the consumers that still need to use it to a new JS_NewObjectWithGivenProtoAndParent API

RESOLVED FIXED in Firefox 39

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla39
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Depends on: 1136292
Created attachment 8568765 [details] [diff] [review]
Drop the parent arg from JS_NewObjectWithGivenProto and introduce a JS_NewObjectWithGivenProtoAndParent for the few cases that still pass in a custom parent
Attachment #8568765 - Flags: review?(jwalden+bmo)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8568765 [details] [diff] [review]
Drop the parent arg from JS_NewObjectWithGivenProto and introduce a JS_NewObjectWithGivenProtoAndParent for the few cases that still pass in a custom parent

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

::: js/src/ctypes/CTypes.cpp
@@ +909,5 @@
>    MOZ_ASSERT(fnproto);
>  
>    // Set up ctypes.CType.prototype.
> +  RootedObject prototype(cx, JS_NewObjectWithGivenProtoAndParent(cx, &sCTypeProtoClass,
> +                                                                 fnproto, parent));

*evil eye*

::: js/src/jsfriendapi.h
@@ +66,5 @@
>  // attached to them.
>  extern JS_FRIEND_API(JSObject *)
>  JS_NewObjectWithoutMetadata(JSContext *cx, const JSClass *clasp, JS::Handle<JSObject*> proto);
>  
> +// Like JS_NewObjectWithGivenProto but allows passing an explicit parent argument.  Don't use this; it's deprecated.

If you can think of a way to squirrel Deprecated into the function name, that'd be awesome.  The comment's nice, but we both know most people won't read it.  I'm totally fine making it JS_DeprecatedNewObjectWithGivenProtoAndParent so it's obnoxious enough people will want to use it even less, if you are!
Attachment #8568765 - Flags: review?(jwalden+bmo) → review+
> *evil eye*

Yeah, so, I was going to try to go through those remaining consumers, at some point....

> I'm totally fine making it JS_DeprecatedNewObjectWithGivenProtoAndParent

Done.
Blocks: 805052
Blocks: 1136516
Blocks: 1136520
Blocks: 1136523
(In reply to Not doing reviews right now from comment #3)
> > *evil eye*
> 
> Yeah, so, I was going to try to go through those remaining consumers, at
> some point....

This was an evil eye at the code, not at you, and not at happening to preserve the case for now.  :-)  Just to be clear.

In general, as long as we make forward progress on a problem, even if it's incomplete forward progress, I'm satisfied.  We have more good code after this, and no good code has become bad.  That's a win for me.
> This was an evil eye at the code, not at you

Sure, I got that.  ;)  You should look at bug 1136523.
https://hg.mozilla.org/integration/mozilla-inbound/rev/186c909aa7f0
https://hg.mozilla.org/mozilla-central/rev/186c909aa7f0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.