For the new Loop service (previously known as Talkilla), we are looking at shipping the Firefox desktop parts within Firefox, rather than serving them as a webapp. This is to avoid issues around loading core parts of Firefox from remote servers. This would mean allowing chrome uris to load properly for built-in social providers. We did an assessment in bug 971987, and this seemed possible. I think the work to do here to provide a safe environment for this would be: - Allow chrome uris to be loaded in the panel and chat windows - Disallow installation of providers which use chrome uris in the manifests This would then be enough to prevent websites trying to use random chrome uris, whilst allowing a built-in provider. We'd want follow-up bugs for: - Stopping/hiding the built-in provider in about:addons - Possibly a separate pref for easy activation of the built-in provider

This enables chrome uri loading. It is based on the one from bug 971987, but with added comments. It still needs tests (which I'm hoping Florian can advise on), and the blocking of chrome uris during activation.

Now with a mochitest for the sidebar panel loaded as chrome.

This now allows chrome uris in the panel, sidebar and chat window. It also has tests. Requesting feedback on this as I'd like to get a general feedback on the structure and changes at this stage, with the proviso of the additional todo items below. Todo: - disallow chrome uris for externally activated urls (separate patch) - have a separate preference/mode for internal providers (separate bug)

Comment on attachment 8385402 [details] [diff] [review] WIP Allow chrome uri loading v3 lgtm though much of my earlier comments still stand. - in the test script, Services.obs.addObserver/etc rather than getService - I wouldn't use notifications for this (anticipating future tests where that could make things more difficult), though it seems that is a fine way to test that you have chrome privileges.

(In reply to Shane Caraveo (:mixedpuppy) from comment #4) > Comment on attachment 8385402 [details] [diff] [review] > - I wouldn't use notifications for this (anticipating future tests where > that could make things more difficult), though it seems that is a fine way > to test that you have chrome privileges. I used notifications for two reasons: 1) Getting chrome workers seems to be more difficult than I'd want and is something we don't need. 2) We need something that handles the async load and waits for that to complete, this seemed the easiest way to get that. However, if you've got other suggestions, I'm open to them.

Updated the patch per comments, and to prevent activation of chrome:// uris from outside. I would like to put the hiding from add-on manage and/or separate prefs in a separate bug & patch as I think that will be a bigger issue. I'll push this to try server once it lets me.

Comment on attachment 8387685 [details] [diff] [review] Allow chrome uri loading v4 >+ testActivationChrome: function(next) { I'm not clear on the purpose of this test, especially given my comment below. >+++ b/toolkit/components/social/SocialService.jsm >@@ -590,6 +590,10 @@ this.SocialService = { > }, > > installProvider: function(aDOMDocument, data, installCallback) { >+ // Don't allow installation for e.g. chrome URIs. >+ if (Services.scriptSecurityManager.isSystemPrincipal(aDOMDocument.nodePrincipal)) >+ throw new Error("SocialService.installProvider: system principals are not allowed."); >+ > let manifest; > let installOrigin = aDOMDocument.nodePrincipal.origin; > I don't think this is the gate I'm concerned about. If a provider origin is a system principle, I think it is ok to install it. The problem is if the origin is NOT a system principle but the manifest contains urls that are. The place to prevent that is in SocialService._manifestFromData where we actually look at the contents of the manifest. I want to be sure that _manifestFromData will in fact prevent the use of a system url if the origin is not system principle.

I don't think loading chrome:// URIs in social panels is a good idea. Why do you want that? Does the provider need the privileges, or is it just for packaging purposes?

Comment on attachment 8387685 [details] [diff] [review] Allow chrome uri loading v4 sorry, had not meant to r+ but f+ until we are certain regular providers are unable to use chrome uris.

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #9) > I don't think loading chrome:// URIs in social panels is a good idea. > > Why do you want that? Does the provider need the privileges, or is it just > for packaging purposes? Gavin, is that Q for me, or for the Loop team?

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #9) > I don't think loading chrome:// URIs in social panels is a good idea. > > Why do you want that? Does the provider need the privileges, or is it just > for packaging purposes? If I remember correctly, this is so that Loop has the privileges to use getUserMedia and push notifications without putting popupnotifications/permission prompts in front of the user.

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #9) > I don't think loading chrome:// URIs in social panels is a good idea. > > Why do you want that? Does the provider need the privileges, or is it just > for packaging purposes? Some of it is packaging - we are moving the FF desktop side of Loop to be shipped within Firefox, not loaded separately (to avoid network performance issues etc), hence we need to be able to load chrome uris. If not, then we'd need to use something other than social API, but we felt that Social API gives us much of the functionality that we currently need. The other side is privileges, as Florian mentioned, we're effectively part of Firefox so some privileges, e.g. push notifications we'd like to be able to avoid. Having chrome privs would obviously be useful in those cases. Note that in bug 976109 we're also looking at ways where we can then have some of this running in content space rather than chrome.

(In reply to Mark Banner (:standard8) from comment #13) > Some of it is packaging - we are moving the FF desktop side of Loop to be > shipped within Firefox, not loaded separately (to avoid network performance > issues etc), hence we need to be able to load chrome uris. chrome:// URIs aren't the only way to load shipped-with-Firefox resources (though they are most convenient). > The other side is privileges, as Florian mentioned, we're effectively part > of Firefox so some privileges, e.g. push notifications we'd like to be able > to avoid. Having chrome privs would obviously be useful in those cases. "having chrome privs" is best isolated to the specific areas that you care about, rather than loading all of the code as chrome://. Some of the changes to the social code to deal with chrome:// URIs and privileges are scary.

(In reply to Mark Banner (:standard8) from comment #13) > Note that in bug 976109 we're also looking at ways where we can then have > some of this running in content space rather than chrome. A Loop::Client bug patching browser/components/loop code in github - not an ideal setup for successful fx-team/loop collaboration. We should reach consensus about the high level integration plan before you guys get too far chasing down any specific solutions.

We're currently investigating an about: uri approach as suggest by Gavin, which seems to work and is tracked by bug 976109