Currently we have a lot of code installed in shell.js just to allow Gaia System app to access it's embedded frame. These mozContentEvent/mozChromeEvent glue, although easy to implement (like bug 1165134), are race-prone (like bug 1167465), and unfortunately un-testable. The purpose of this bug is to implement a proxy Browser API for the use case above. It was briefly mentioned in dev-b2g post "Proposal to get rid of some of the shell.js <-> System app custom events, a.k.a. mozChromeEvent" at point (6). The API interface should be identical to the Browser API itself, so it will not cover other use cases implemented in the post. There are many ways to implement this: 1) Attempt to load BrowserElementParent.js into our API impl and somehow make it connect to the child frame script within the same process with observer. 2) Re-impl the interface and proxy the call to BrowserElementChildPreload.js directly with observer messages. 3) Proxy the calls with another pair of content/parent scripts -- have the parent script talk to the BrowserElementParent -- in essence, a content-parent-content-parent-content flow. A few hours of investigation shows (1) requires non-trivial refactor on BrowserElementParent.js to allow it to take in both observer and mm message. It also does not allow us to impl feature provided by native code. (2) would avoid refactor the BrowserElementParent.js script, but it would also prevent us from access some features provided by native code. So (3), while is the least permanent solution, is the one :kanru and I currently is looking at. It also require minimal code change which free us to do other more important things. I will update the first WIP once it's done and worked. The API is temporary named |navigator.mozBrowserElementProxy|.

The broken, obsolete (2) approach. Put it here so it don't get lost in case we change our minds.

This is the first working version. Done: 0. Implement BrowserElementProxy as a XPCOM object and expose it 1. Use observer to allow BrowserElementProxy talk to BrowserElementChildPreload to talk to each other. 2. Correctly deal with the assumptions of one-to-one relationship of *Proxy and *ChildPreload to ensure they properly ignore messages from other pairs in the same process. 3. Forward event listeners and method calls. To do: 4. Hide the interface under a permission. 5. allowedAudioChannels 6. Tests. Kanru, as offline discussed, please take a look and give me some feedback on this. Thanks!

Some of Bobby's wrappers might help us here. If I understand the situation correctly, we have a chrome document which runs shell.js and contains an <iframe mozbrowser mozapp>. Inside this <iframe> we render the system app. The system app runs in the parent process and so is in the same process as the chrome document. Tim, is that correct? Note that the system app is not really a normal webapp. It's essentially part of the platform. So what I'm about to suggest is not something that we'd do everywhere, but just as a one-off here in order to reduce other one-offs that we have in its place right now. I was wondering if there's some way that we can use wrapper magic to expose the chrome <iframe mozbrowser mozapp> to the system app document that lives inside that iframe. We wouldn't want to enable the system app to get arbitrary chrome privileges though, for example by walking the JS object heap starting at the <iframe> object. So some form of whitelist would probably be needed deciding which properties on the object could be exposed. Is there any wrapper magic that can help us here?

We might be consider moving System app out-of-process someday, if possible. So if the wrapper magic could work cross process, we could use that (and to spare me some effort re-implementing all most all interfaces).

(In reply to Jonas Sicking (:sicking) from comment #3) > If I understand the situation correctly, we have a chrome document which > runs shell.js and contains an <iframe mozbrowser mozapp>. Inside this > <iframe> we render the system app. The system app runs in the parent process > and so is in the same process as the chrome document. > > Tim, is that correct? Explicitly, yes, that's correct, but again we don't want to close the future possibility to move System app out-of-process.

I don't see much reason to move the system app out of process. But if we do decide to do that, we can always change the way that we talk to the browser element then.

(In reply to Jonas Sicking (:sicking) from comment #6) > I don't see much reason to move the system app out of process. But if we do > decide to do that, we can always change the way that we talk to the browser > element then. Thinking aloud, might not be valid: 1. Improve security: isolate add-ons, user content etc. to a content process. 2. Uniform API behavior: API call from content code always goes through IPC boundary -- less bugs to fix for handling special behavior when calling from System app.

(In reply to Jonas Sicking (:sicking) from comment #3) > I was wondering if there's some way that we can use wrapper magic to expose > the chrome <iframe mozbrowser mozapp> to the system app document that lives > inside that iframe. Why do you want to do that? I'm having trouble seeing the value of accessing a non-subsumed framElement (which goes directly against our invariants). It seems like what you're really proposing is some sort of synchronous API between the chrome code and the system app, right? We have the tools to do that for addons and stuff (Cu.exportFunction, etc), but it seems like we should really avoid a synchronous API in this case. (In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #4) > So if the wrapper magic could work cross process It would not.

> 1. Improve security: isolate add-ons, user content etc. to a content process. Possibly. Though I think that there's so much changes that need to happen in order to gain any tangible security benefits that figuring out how to do a different browser proxy is a drop in the bucket. One solution might simply be to put shell.js in the same child process. > 2. Uniform API behavior: API call from content code always goes through IPC > boundary -- less bugs to fix for handling special behavior when calling from > System app. That would indeed be nice, but not enough that we need to worry about it for now. Basically, I think that at some point in the future we might move the system app into a child process. But that will be a huge task and whatever we do here won't affect that much in either direction I think. (In reply to Bobby Holley (:bholley) from comment #8) > (In reply to Jonas Sicking (:sicking) from comment #3) > > I was wondering if there's some way that we can use wrapper magic to expose > > the chrome <iframe mozbrowser mozapp> to the system app document that lives > > inside that iframe. > > Why do you want to do that? I'm having trouble seeing the value of accessing > a non-subsumed framElement (which goes directly against our invariants). Tim can elaborate. Basically the system app wants to manage the resources for the whole OS in various ways, like screen orientation, audio policies, etc. Right now it can do that really well for all apps and web content. But it can't do it for the UI that the system app itself is displaying. > It seems like what you're really proposing is some sort of synchronous API > between the chrome code and the system app, right? We have the tools to do > that for addons and stuff (Cu.exportFunction, etc), but it seems like we > should really avoid a synchronous API in this case. I don't think we should worry about synchronous vs. non-synchronous too much here. Lets do something simple and worry about what would happen if the system app is moved out-of-process later. If we do do that it'll be a huge project and not likely to happen for years.

(In reply to Jonas Sicking (:sicking) from comment #9) > > Why do you want to do that? I'm having trouble seeing the value of accessing > > a non-subsumed framElement (which goes directly against our invariants). > > Tim can elaborate. Basically the system app wants to manage the resources > for the whole OS in various ways, like screen orientation, audio policies, > etc. Right now it can do that really well for all apps and web content. But > it can't do it for the UI that the system app itself is displaying. Sure, I'm disputing that providing a direct script reference to the iframe element is the right way to do that. If you just want to inject a quick-and-dirty non-WebIDL-compliant API, use Cu.exportFunction (or Cu.cloneInto with cloneFunctions: true).

I still think I should complete a full WebIDL-comliant, corss-process API here, although I am hitting some road block in WebIDL.

Trying to bring this patch back. Since there is a bug 1167465 fixed, I will skip the audio channel part in this API.

Patch rebased and have |allowedAudioChannels| points to what's already implemented.

Kanru, per what we have discussed, I added inproc & oop tests that would ensure three types of methods & events are properly forwarded. Could you review this? Jonas, please look at the WebIDL added -- the interface simply implements old Browser API interfaces. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce3d0d0f6699

Re-push to try because of bug 1217622. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1fb8191256be

Comment on attachment 8677846 [details] [diff] [review] bug1196654.patch Review of attachment 8677846 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/installer/package-manifest.in @@ +385,5 @@ > @RESPATH@/components/ConsoleAPIStorage.js > @RESPATH@/components/BrowserElementParent.manifest > @RESPATH@/components/BrowserElementParent.js > +@RESPATH@/components/BrowserElementProxy.manifest > +@RESPATH@/components/BrowserElementProxy.js Don't forget fennec and b2gdroid ::: dom/browser-element/BrowserElementParent.js @@ +74,5 @@ > + MOZBROWSER_EVENT_NAMES: Object.freeze([ > + "loadstart", "loadend", "close", "error", "firstpaint", > + "documentfirstpaint", "audioplaybackchange", > + "contextmenu", "securitychange", "locationchange", > + "iconchange", "scrollareachanged", "titlechange", This is not an exhaustive list right? Add a comment explaining why some are excluded. ::: dom/browser-element/mochitest/browserElement_Proxy.js @@ +10,5 @@ > +function runTest() { > + let path = location.pathname; > + let frameUrl = location.protocol + '//' + location.host + > + path.substring(0, path.lastIndexOf('/')) + > + '/file_empty.html'; I think we could use SimpleTest.getTestFileURL('file_empty.html') ::: dom/browser-element/moz.build @@ +54,5 @@ > ] > > +DIRS += [ > + 'proxy' > +] Any reason this file has to go to a sub-dir? ::: dom/browser-element/proxy/BrowserElementProxy.js @@ +35,5 @@ > + > +function BrowserElementProxy() { > + // Pad the 0th element so that DOMRequest ID will always be a truthy value. > + this._pendingDOMRequests = [ undefined ]; > +} Please write some prose explaining the message flow and how to update the proxy when adding new APIs and events. @@ +136,5 @@ > + data.domRequestId = domRequestId; > + } > + > + Services.obs.notifyObservers( > + this._window, 'browser-element-api:proxy-call', JSON.stringify(data)); If we could get a handle to the proxy forwarder then we don't have to use observers. I'm not sure if it's doable though. ::: dom/webidl/BrowserElementProxy.webidl @@ +7,5 @@ > +[Constructor, > + JSImplementation="@mozilla.org/dom/browser-element-proxy;1", > + NavigatorProperty="mozBrowserElementProxy", > + Pref="dom.mozBrowserFramesEnabled", > + CheckAnyPermissions="browser:embedded-system-app"] We should add AvailableIn="CertifiedApps" but I don't know how to expose that in mochitest..

I've cancelled all the previous try jobs and do -p all instead, just to make sure we are good on Fennec and B2GDroid and etc. https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9078bc93555

Comment on attachment 8677974 [details] [diff] [review] bug1196654.patch Review of attachment 8677974 [details] [diff] [review]: ----------------------------------------------------------------- I'll defer to KanRu here. So if he's happy, I'm happy.

This is a purely internal API between Gecko and Gaia System, but it's probably useful to other vendors...