929570 - (SH-addons) [Meta] Deliver a crisp story for addons to expose APIs to content without __exposedProps__

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

This is part (4) of slaughterhouse. Gabor and I have been working on this intermittently since January, and it's time to tie off all the pieces. Gabor is going to own this.

Comment 1

[Bobby Holley (:bholley)]

Gabor and I are going to sit down tomorrow and sort out the details here.

Comment 2

[Jesse Ruderman]

https://mxr.mozilla.org/addons/search?string=exposedprops
Too many hits, displaying the first 1000

Comment 3

[Gabor Krizsanits (INACTIVE)]

I'll just start to collect the sub tasks for this, I'll just start with some random brainstorming. First question I have here is that will we try to address add-ons using unsafeWindow/wrappedJSObject as well (privileged code manipulation content js objects) or just the __exposedProps__ part (privileged code let content access to some privileged functions/objects).

It would be great to address both issues at once imo, but also probably it's quite challenging.

Also, I expect to solve bug 821809 here as well, which would be awesome.

1., exportHelpers API is ready, but we need a high level, easy to use JS based API on top of it.
I expect some help from Irakli here...
2., documentation / blog (I can write the blog, hopefully Will can help me out with the docs)
3., update SDK code (I can do this part probably but I also want someone else to grasp what I'm doing so a volunteer would be nice from the SDK team )
4., some plan to update all the add-ons...
- understanding the scale of the changes to be done
(it should be easy to fix most add-ons but I'm not sure that an automatisation is possible)
- isolating add-ons with complex cases...

The 4th point is obviously the most challenging one. I hope Dave and Jeff will help me with coordinating this.

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Jesse Ruderman from comment #2)
> https://mxr.mozilla.org/addons/search?string=exposedprops
> Too many hits, displaying the first 1000

I know... and unsafeWindow / wrappedJSObject are not a lot better... I don't expect an easy nor a fast solution here.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Jesse Ruderman from comment #2)
> https://mxr.mozilla.org/addons/search?string=exposedprops
> Too many hits, displaying the first 1000

Almost all of those are from addons that include older versions of Jetpack, which used __exposedProps__. The actual number of addons using __exposedProps__ is much, much lower.

Comment 6

[Bobby Holley (:bholley)]

Er, sorry. I mean current versions of the addon SDK. A major part of this bug is moving Jetpack _off_ of __exposedProps__.

Comment 7

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3)
> I'll just start to collect the sub tasks for this, I'll just start with some
> random brainstorming. First question I have here is that will we try to
> address add-ons using unsafeWindow/wrappedJSObject as well (privileged code
> manipulation content js objects) or just the __exposedProps__ part
> (privileged code let content access to some privileged functions/objects).

My plan is for bug 914970 to address a lot of the cases where people currently use wrappedJSObject, but it won't solve all of them. I think we should focus on putting the best tools in place.

Comment 8

[Bobby Holley (:bholley)]

I just realized that we're going to have to be more careful about CloneNonReflectors, because of issue we've been having with toString()ing. I'm going to think about this a bit more.

Comment 9

[Jorge Villalobos [:jorgev] (he/him)]

A couple of notes here:

* All add-ons using versions of the SDK below 1.14 are either disabled or downgraded on AMO (bug 783046). Once the SDK is no longer using __exposedProps__, we can try to do a similar push for SDK developers to update.
* One tool we have for communicating with developers is the review process. If we have a good story to move away from __exposedProps__ to something safer (the message manager?), we can add a flag and start pushing developers in that direction.

Comment 10

[Jeff Griffiths (:canuckistani) (:⚡︎)]

unsafeWindow needs to be treated differently than __exposedProps__ IMO:

 - I'm betting use of __exposedProps__ is mostly limited to old versions of SDK-based extensions, something which may not be a big deal.

 - SDK code does not use unsafeWindow, so uses of the SDK's unsafeWindow are entirely in userland. This is a much larger problem.

From previous re-packing efforts the SDK team has scripts and techniques for doing analysis of the current versions of SDK-based add-ons. I think we should do this up-front to see how based the unsafeWindow problem currently is.

The tools we have at our disposal are:

 - we can mark specific add-ons on AMO incompatible with newer versions of Firefox
 - we can produce deprecation warnings or exceptions for code that tries to use unsafeWindow - this will make these add-ons fail review on AMO, so new versions using unsafeWindow won't be approved
 - we can work with the AMO team to introduce a new policy that unsafeWindow should not be used
 - we can do outreach with developers both as part of the usual add-on compat process and as an exception to that

One thing I would like to see ASAP ( Firefox 27??? ) is a deprecation warning for unsafeWindow. I've logged bug 930069 for this.

Comment 11

[Bobby Holley (:bholley)]

Gabor and I met today to discuss this stuff. Here are some key takeaways:

(1) We shouldn't expend effort trying to deprecate unsafeWindow. unsafeWindow is dangerous in the current world because we run content scripts with the same principal as the content itself. We need to fix this with bug 821809, after which point unsafeWindow will be no less safe than any usage of .wrappedJSObject.

(2) When we do bug 821809, addons that stick non-primitive properties on unsafeWindow will break, because the property will now be a privileged object and appear opaque. So we've waited to start moving on this, because we don't want to spread __exposedProps__ to nsExpandedPrincipals, but we still need a story on what authors should do instead. That's what this bug is about.

(3) The platform primitives are more or less in place, modulo a few small things. The next step is for some combination of Jeff and Irakli to come up with a slick JSM to wrap the primitives in an easy-to-use API.

Comment 12

[Bobby Holley (:bholley)]

Note that, once we have a prototype of the slick JSM, we should circulate it among stakeholders (big-name addon developers, jorge, Jesse, etc) to make sure it meets everyone's needs.

Comment 13

[Gabor Krizsanits (INACTIVE)]

I just need info people to make sure they read this bug.

Comment 14

[Jeff Griffiths (:canuckistani) (:⚡︎)]

I read this bug.