771081 - Rename CrossOriginWrapper

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

The name is terribly confusing. Now that NoWaiverWrapper is gone, we should fix this up.

Luke suggested TransitivelyWaivingWrapper, which sounds fine to me, unless anyone has a better suggestion.

Blake, how does TransitivelyWaivingWrapper sound?

Comment 1

[Gabor Krizsanits (INACTIVE)]

Yaaay. Also, once we have the name you can assign this one to me anytime.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

How about WaiveXrayWrapper? TransitivelyWaiving sounds like it could waive just about anything.

Comment 3

[Gabor Krizsanits (INACTIVE)]

(In reply to Blake Kaplan (:mrbkap) from comment #2)
> How about WaiveXrayWrapper? 

Sounds good, but isn't that kind of taken already (WaiveXrayWrapperWrapper)?

Comment 4

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3)
> (In reply to Blake Kaplan (:mrbkap) from comment #2)
> > How about WaiveXrayWrapper? 
> 
> Sounds good, but isn't that kind of taken already (WaiveXrayWrapperWrapper)?

We should just rename that to XrayWaiver IMO.

Comment 5

[Gabor Krizsanits (INACTIVE)]

So XrayWaiver and WaiveXrayWrapper are a bit too easy to mix up for me. One of the trivial difference between them is one of them is CCW the other is SCW. Would it make sense to make that difference explicit in their names to avoid confusion?

If I'm the only one who finds them easy to mix up, I like them already better than the current names. Btw what is against simply calling it TransparentWrapper?

Comment 6

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #5)
> So XrayWaiver and WaiveXrayWrapper are a bit too easy to mix up for me. One
> of the trivial difference between them is one of them is CCW the other is
> SCW. Would it make sense to make that difference explicit in their names to
> avoid confusion?

They accomplish two very distinct things. The second isn't just the SCW version of the first - it's serves as a second identity to make the operation of the first possible.

In our parlance, an object "has a waiver" (a noun) if it's bound inside XrayWaiver. We then use the WaiveXrayWrapper to access it.

How about s/WaiveXrayWrapper/WaivedXrayWrapper/? I think that's more precise, and might help alleviate the confusion a little bit.
 
> If I'm the only one who finds them easy to mix up, I like them already
> better than the current names. Btw what is against simply calling it
> TransparentWrapper?

Because that's much less explicit about what's actually going on, IMO. Waiving Xray gives transparent semantics, but it _also_ clamps the principal. And that would also imply that same-origin content wrappers are somehow not transparent.

Comment 7

[Gabor Krizsanits (INACTIVE)]

(In reply to Bobby Holley (:bholley) from comment #6)
> They accomplish two very distinct things. 

I know, but it seemed like an easy way to distinguish them.

> In our parlance, an object "has a waiver" (a noun) if it's bound inside
> XrayWaiver. We then use the WaiveXrayWrapper to access it.

This sold it to me.

> How about s/WaiveXrayWrapper/WaivedXrayWrapper/? I think that's more
> precise, and might help alleviate the confusion a little bit.

I prefer the other one. Only one character difference is too easy for the eyes and the ears to mix up.

Comment 8

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #7)
> > In our parlance, an object "has a waiver" (a noun) if it's bound inside
> > XrayWaiver. We then use the WaiveXrayWrapper to access it.
> 
> This sold it to me.

w00t! Care to whip up a patch? ;-)

Comment 9

[Gabor Krizsanits (INACTIVE)]

Attachment: part1: Rename CrossOriginWrapper class

Comment 10

[Gabor Krizsanits (INACTIVE)]

Attachment: part2: Rename CrossOriginWrapper files

Comment 11

[Gabor Krizsanits (INACTIVE)]

Attachment: part3: Rename WaiveXrayWrapperWrapper

Comment 12

[Andreas Gal :gal]

Comment on attachment 642407 [details] [diff] [review]
part3: Rename WaiveXrayWrapperWrapper

Much better naming.

Comment 13

[Gabor Krizsanits (INACTIVE)]

https://tbpl.mozilla.org/?tree=Try&rev=1dfd8c22d506

Comment 14

[Luke Wagner [:luke]]

\o/

Comment 15

[Gabor Krizsanits (INACTIVE)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e55d5df611b9
http://hg.mozilla.org/integration/mozilla-inbound/rev/f4aae5703855
http://hg.mozilla.org/integration/mozilla-inbound/rev/ecc4f73417d8

Comment 16

[Ed Morley [:emorley]]

Didn't make it to mozilla-central before the uplift (merge was blocked on bug 774259). Adjusting milestone accordingly.

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e55d5df611b9
https://hg.mozilla.org/mozilla-central/rev/f4aae5703855
https://hg.mozilla.org/mozilla-central/rev/ecc4f73417d8