Closed
Bug 1122758
Opened 9 years ago
Closed 9 years ago
Get rid of XPCJSObjectHolder::newHolder()
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
6.35 KB,
patch
|
gkrizsanits
:
review+
|
Details | Diff | Splinter Review |
This is a weird function. I think it just makes sense to inline it and make the ctor public.
Assignee | ||
Comment 1•9 years ago
|
||
I'm not entirely sure this is a good idea, but it seems like an improvement to me. try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1f3b868c1f6
Attachment #8554621 -
Flags: review?(gkrizsanits)
Comment 2•9 years ago
|
||
Comment on attachment 8554621 [details] [diff] [review] Get rid of XPCJSObjectHolder::newHolder(). Review of attachment 8554621 [details] [diff] [review]: ----------------------------------------------------------------- I think it's definitely an improvement, and sorry about the delay I was having a holiday. ::: js/xpconnect/src/nsXPConnect.cpp @@ +823,5 @@ > > if (NS_SUCCEEDED(rv) && !rval.isPrimitive()) { > + JSObject *obj = rval.toObjectOrNull(); > + if (!obj) > + return NS_ERROR_FAILURE; I think the !rval.isPrimitive() ensures that rval is a valid object, so why do we need this check here?
Attachment #8554621 -
Flags: review?(gkrizsanits) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #2) > I think the !rval.isPrimitive() ensures that rval is a valid object, so why > do we need this check here? Good point. try run with that check removed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f78fec40d632
Assignee | ||
Comment 4•9 years ago
|
||
I removed the redundant check. https://hg.mozilla.org/integration/mozilla-inbound/rev/40aeb4e4292c
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/40aeb4e4292c
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•