Open
Bug 892687
Opened 11 years ago
Updated 2 years ago
Make returning object values not require fallible getters/methods
Categories
(Core :: DOM: Bindings (WebIDL), defect)
Core
DOM: Bindings (WebIDL)
Tracking
()
NEW
People
(Reporter: bzbarsky, Unassigned)
References
(Blocks 1 open bug)
Details
Right now, returning an object from WebIDL bindings is always fallible, for several reasons:
1) JS_NewObject can throw, either when allocating the wrapper or setting up its proto chain.
2) JS_Define* can throw when setting up the proto chain.
3) JS_WrapValue can throw.
I will claim that once bug 862848 is fixed, these can all only happen due to OOM. Bobby, is that true for JS_WrapValue?
And at that point, I think we should just make these infallible on the binding side (with MOZ_CRASH as needed).
That would allow us to avoid outputting jitcode to check for exceptions and whatnot, which should make our jitcode faster and smaller and easier to understand.
Sequence and dictionary return values would remain fallible, since those might in fact need to allocate large chunks of memory depending on the length of the sequence and what's in the dictionary.
Reporter | ||
Comment 1•11 years ago
|
||
> is that true for JS_WrapValue?
I realize that JS_WrapValue on a string can do a big allocation; I'll keep that one fallible.
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Reporter | ||
Comment 2•11 years ago
|
||
Hmm. Peter, how do you feel about MOZ_CRASH if someone tries to GetProtoObject() or GetConstructorObject() for a WebIDL binding on a global that does not have JSCLASS_DOM_GLOBAL?
OS: All → Mac OS X
Hardware: All → x86
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(peterv)
Flags: needinfo?(bobbyholley+bmo)
OS: Mac OS X → All
Hardware: x86 → All
Comment 3•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #0)
> I will claim that once bug 862848 is fixed, these can all only happen due to
> OOM. Bobby, is that true for JS_WrapValue?
JSCompartment::wrap only throws for OOM or if the XPConnect wrap hooks throw return null. This can happen if:
* JS_ObjectToOuterObject fails
* JS_GetPrototype or js::GetObjectProto fails
* JS_GetClassPrototype fails
* PreCreate throws on an XPCWN being wrapped cross-compartment
* WrapNativeToJSVal throws trying to create a new XPCWN reflector in a the target compartment.
* XPCNativeSet::GetNewOrUsed fails in the case of the above.
* Somebody tries to wrap a chrome-privileged eval or Function constructor into content.
* Wrapper::Renew (probably just OOM, but I didn't dig into it)
* OOM
The hairy ones shouldn't apply to things that we know are new-binding objects (unless the Function constructor is a new-binding object? anyway, I'm totally happy to MOZ_CRASH in that case). So I think we can treat JS_WrapValue as infallible here.
Flags: needinfo?(bobbyholley+bmo)
Reporter | ||
Comment 4•11 years ago
|
||
We don't necessarily know these values are new-binding objects. All we know is they're some JSObject.
Flags: needinfo?(bobbyholley+bmo)
Comment 5•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #4)
> We don't necessarily know these values are new-binding objects. All we know
> is they're some JSObject.
In what situations might we be returning an XPCWN here, especially an XPCWN with a spineless PreCreate hook?
Anyway, in general I'm happy to MOZ_CRASH here.
Flags: needinfo?(bobbyholley+bmo)
Reporter | ||
Comment 6•11 years ago
|
||
> In what situations might we be returning an XPCWN here
Any time WebIDL has an external interface return value, say. In the long-term, never.
> especially an XPCWN with a spineless PreCreate hook
The only XPCWN with PreCreate hooks are: Window, nsDOMConstructor, Location, IDBEventTarget, History. No idea whether they're spineless. ;)
> Anyway, in general I'm happy to MOZ_CRASH here.
Excellent.
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(peterv)
Reporter | ||
Comment 7•11 years ago
|
||
Well, I used to have patches for this, partly, but my computer decided to eat them. :(
Given that, the lack of progress on the bug this depends on, and the fact that I think I've found a way to work around this in general, I'm just not going to worry about this.
Assignee: bzbarsky → nobody
No longer blocks: 916851
Comment 8•11 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #2)
> Hmm. Peter, how do you feel about MOZ_CRASH if someone tries to
> GetProtoObject() or GetConstructorObject() for a WebIDL binding on a global
> that does not have JSCLASS_DOM_GLOBAL?
Fine by me.
Flags: needinfo?(peterv)
Comment 9•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046
Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.
If you have questions, please contact :mdaly.
Priority: -- → P5
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Reporter | ||
Updated•5 years ago
|
Component: DOM: Core & HTML → DOM: Bindings (WebIDL)
Priority: P5 → --
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•