Eddy Bruel [:ejpbruel] Reporter
	Description • 12 years ago

The class AbstractWrapper is currently not used anywhere. More precisely, it looks like the only code that uses it does not use the policy enforcement traps, and could therefore inherit from IndirectProxyHandler instead. If that's true, it's probably better to get rid of this class. Proxies are currently in flux, and changes in other parts of the code could easily lead to AbstractWrapper breaking without us noticing it.

	Bobby Holley (:bholley)
	Comment 2 • 12 years ago

Comment on attachment 640200 [details] [diff] [review]
Patch to be reviewed

This looks good! It also means, I think, that we no longer need the multiple inheritance for DirectWrapper, so we could just fold everything into DirectWrapper, and get rid of those toWrapper and toProxyHandler virtual methods. What do you think?

	Eddy Bruel [:ejpbruel] Reporter
	Comment 3 • 12 years ago

Adding bug 771429 as a dependency, since it looks as bz might have a use for IndirectWrapper at all.

	Eddy Bruel [:ejpbruel] Reporter
	Comment 4 • 12 years ago

(In reply to Bobby Holley (:bholley) from comment #2)
> Comment on attachment 640200 [details] [diff] [review]
> Patch to be reviewed
> 
> This looks good! It also means, I think, that we no longer need the multiple
> inheritance for DirectWrapper, so we could just fold everything into
> DirectWrapper, and get rid of those toWrapper and toProxyHandler virtual
> methods. What do you think?

Provided bz doesn't need IndirectWrapper, I totally agree :-)

	Eddy Bruel [:ejpbruel] Reporter
	Comment 5 • 12 years ago

(In reply to Bobby Holley (:bholley) from comment #2)
> Comment on attachment 640200 [details] [diff] [review]
> Patch to be reviewed
> 
> This looks good! It also means, I think, that we no longer need the multiple
> inheritance for DirectWrapper, so we could just fold everything into
> DirectWrapper, and get rid of those toWrapper and toProxyHandler virtual
> methods. What do you think?

My understanding of our last irc convo was that it is ok for us to move forward with this. Can you confirm?

	Bobby Holley (:bholley)
	Comment 6 • 12 years ago

Well, I think this patch wouldn't be correct until IndirectProxyHandler is unwrappable, right? Otherwise, we change the behavior of SandboxProxy.

	Eddy Bruel [:ejpbruel] Reporter
	Comment 7 • 12 years ago

(In reply to Bobby Holley (:bholley) from comment #6)
> Well, I think this patch wouldn't be correct until IndirectProxyHandler is
> unwrappable, right? Otherwise, we change the behavior of SandboxProxy.

You're right. Good thing that I double checked :-)

	André Bargull [:anba]
	Comment 8 • 8 years ago

Abstract/IndirectMapper was removed in https://hg.mozilla.org/mozilla-central/rev/da2e3ebd1c26