855136 - Stop requiring call and construct slots for callable proxies

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: don't require call/construct for callable proxies

I ran into a problem the other day where I wanted to create a callable proxy, but I didn't have a value to pass in for the |call| and |construct| arguments. Instead, I just wanted to implement my own ::call hook. This first patch is fairly conservative and just fixes that problem.

Comment 1

[Bobby Holley (:bholley)]

Yeah, the whole Proxy/FunctionProxy distinction always seemed silly to me. I've never seen a reason why need per-instance (rather than per-handler) callables that couldn't be inferred from the proxy private (for wrappers, this would be just invoking the call hook of the wrapped object). Can we get rid of this distinction?

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: don't use call/construct slots for anything but indirect scripted proxies

This patch takes things a little further. We only actually use the call and construct slots for scripted indirect proxies. For all other proxies, we just set them to the target. I've been confused by this in the past. The patch makes it so that the call/construct slots are left undefined for all proxies except the scripted indirect ones.

We can't actually remove these slots because all function proxies use the same class so must have the same number of reserved slots. However, Jeff said that we'll probably remove scripted indirect proxies eventually, and then we will be able to kill these slots. This patch will make that easier.

I had to copy some of the call and construct hooks from BaseProxyHandler to DirectProxyHandler and ScriptedIndirectProxyHandler since they now do different things (the former use the target slot and the latter use the call/construct slot).

I left the definitions of the call/construct hooks in BaseProxyHandler, although all they do now is assert. If you're writing a proxy handler that can be used as a function proxy, you should implement your own version of these hooks. An alternative would be to make these hooks pure virtual inside BaseProxyHandler. That would make things a little harder for one or two proxy handlers that don't inherit from DirectProxyHandler. I could go either way on this decision.

There's kind of a related problem with the fun_toString hook. I solved it in pretty much the same way.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Bobby Holley (:bholley) from comment #1)
> Yeah, the whole Proxy/FunctionProxy distinction always seemed silly to me.
> I've never seen a reason why need per-instance (rather than per-handler)
> callables that couldn't be inferred from the proxy private (for wrappers,
> this would be just invoking the call hook of the wrapped object). Can we get
> rid of this distinction?

I'm not sure what you're asking. Do you mean: can we use one class for all proxies and infer callable-ness in some other way? It seems like we should be able to do that. There is an issue of space since function proxies are bigger. If we get rid of scripted indirect proxies, then that won't be an issue.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Bill McCloskey (:billm) from comment #3)
> (In reply to Bobby Holley (:bholley) from comment #1)
> > Yeah, the whole Proxy/FunctionProxy distinction always seemed silly to me.
> > I've never seen a reason why need per-instance (rather than per-handler)
> > callables that couldn't be inferred from the proxy private (for wrappers,
> > this would be just invoking the call hook of the wrapped object). Can we get
> > rid of this distinction?
> 
> I'm not sure what you're asking. Do you mean: can we use one class for all
> proxies and infer callable-ness in some other way? It seems like we should
> be able to do that. There is an issue of space since function proxies are
> bigger. If we get rid of scripted indirect proxies, then that won't be an
> issue.

Right, I'd forgotten about the semantics of ScriptedIndirectProxies. They could still be fixed (by simply hoisting the call trap into the handler), but it's probably better to just wait for them to go away. The biggest hurdle is probably converting the SpecialPowers wrapper to DirectProxies ;-).

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3d7ab7be4d31
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b2818d4dccfe

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d7ab7be4d31
https://hg.mozilla.org/mozilla-central/rev/b2818d4dccfe
https://hg.mozilla.org/mozilla-central/rev/c9e35e4221e5