Proposal to get rid of some of the shell.js <-> System app custom events, a.k.a. mozChromeEvent

[Tim Guan-tin Chien]

This is a general idea on our Device API evolution. I want to bring
this up and make sure it's a workable idea.

TLDR: Let's move mozChromeEvent to it's respective origin APIs and
expose them under |navigator.mozFoo.mgmt| with a special
|foo-manage-system| permission that we will only grant to System app.

1) What are mozChromeEvents

mozChromeEvent and mozContentEvent are ad-hoc event name we give for
custom events dispatches between shell.js (w/ chrome privilege) and
System app (an packaged app with permissions listed in it's manifest).
We use this because some of the Gecko APIs rely on System app to
actually act on it on the UI layer. For example,

* mozApps and mozSystemMessage rely on System app (specifically,
window management) to actually create and show the app frame to launch
an app.
* mozInputMethod rely on System app input mgmt to respond to focus
event and show the keyboard
* WebNotification API rely on System app to actually show the
notifications, and respond to user interaction on the notifications.

2) How they are implemented

If you look at shell.js you would see basically everyone dump a few
line of observers, and convert Gecko-internal observer message into
the event. There are other alternatives like import "SystemAppProxy"
into your jsm and call the event sending method.

3) It works, how bad is it

3.1) mozChromeEvent poses a performance problem, since a lot of
components in System app need to listen to it, we ended up having 30x
event listeners installed and only one of these callbacks will
actually act on it.
3.2) Spaghetti in shell.js is in no way testable other than Gaia
integration tests, but in fact they are part of APIs that should have
been testable in dom/*/mochitests/
3.3) By allowing 3.1 and 3.2 to continue live in the tree we are not
delivering the best experience we could. We also deny the mission of
B2G on figuring out what is needed to construct an OS with Web
technologies.
3.4) It only works in-process. I don't know if that's desirable but if
it's possible to run System app in non-root process, we should
definitely do it.

4) Proposed evolution direction

I proposed for applicable mozChromeEvent or mozContentEvents use
cases, we should move them back to their respective APIs, and only
expose these method/events with a special |-system| permission. For
example, the open-app event is dispatched whenever Gecko need Gaia
System to create an app frame, it could be replaced with an interface
like this (DOMApplicationsManager is exposed under
|navigator.mozApps.mgmt|)

partial interface DOMApplicationsManager : EventTarget {
[CheckAnyPermissions="webapps-manage-system"]
attribute EventHandler onlaunchrequest
}

And Gaia System window management will act on this event.

5) Possible alternatives

There will always be some System level UIs and we will always need
some place to host it. The only alternative I could think of would be
granting chrome privilege to System app, but even with that we will be
only fixing 3.1 and not 3.2-4.

6) What's not applicable

The one big exception here is the Browser API proxies. Sometimes
System app would need to access the Browser API methods exposed to
it's own embedder. Unless we implement some kind of
|navigator.mozControlMyOwnBrowserFrame| API to host these methods, we
would be still rely on shell.js to use them, for now. I can send out
another proposal if people are interested.

==

Specifically call out to :pault, :sicking, :fabrice, and :vivien for
response on this, as you are the owners of security and B2G
runtime-related matter. If there is no strong objection I will begin
to work on this.

Thanks!


Tim

[Michael Henretty]

On Thu, Jul 23, 2015 at 12:32 AM, Tim Guan-tin Chien <timd...@mozilla.com> wrote:

The one big exception here is the Browser API proxies. Sometimes
System app would need to access the Browser API methods exposed to
it's own embedder. Unless we implement some kind of
|navigator.mozControlMyOwnBrowserFrame| API to host these methods, we
would be still rely on shell.js to use them, for now. I can send out
another proposal if people are interested.

Can you explain this a little more? I'm not sure what this means.

[Tim Guan-tin Chien]

Take AudioChannel management for instance. The latest reversion will
allow System app to mute/pause audio from a given app frame, and the
methods to do so are attached on the mozbrowser iframe instance.

Given the design, System are prevented to manage it's own audio from
various components. The current fix here is to allow System to access
the audio managing methods of it's own hosting frame by wiring them to
mozChromeEvents/mozContentEvents.


Tim

[Jonas Sicking]

Hi Tim,

Generally speaking, I completely agree with you that our "chrome
events" design is shitty.

I think it's great if we can replace the events with "real" APIs like
mozApps.mgmt.onlaunchrequest.

However I also want to be mindful of that it might be non-trivial to
implement these things in Gecko. And given that we will have exactly
one consumer, the system app, I don't want to spend too much time on
implementing the perfect API.

But absolutely, in places where it's easy to add methods on the
appropriate API, I definitely think we should do exactly that. For
example I would imagine that we already have APIs on the browser API
which is exposed only to the system app. So adding more features to
the browser API which is only exposed to the system app shouldn't be
much work I would imagine?

Regarding permissions, we could simply add a "system-app" permission
which is only given to the system app. That way we could pretty easily
add WebIDL annotations which takes care of security checks. No need to
invent specific permissions for specific APIs.

This matches what we do for the "system principal" in Gecko, where we
different pages have different principals, but where all Gecko
internals just run with a single shared "system principal" which is
given a whole host of capabilities.


To capture the APIs where for whatever reason it wouldn't be easy to
build a "real" API, we could do something like

callback SystemAppMessageHandler = void (any messageBody);

interface SystemAppMessages {
addMessageHandler((sequence<DOMString> or DOMString) messageType,
SystemAppMessageHandler handler);
sendMessage(DOMString messageType, any messageBody);
}

So the system app could register different handlers for different
message types. And on the Gecko side we can have a generic way to send
and receive simple strings and structured clones.


How does that sound? I'd definitely like to hear Fabrice's thoughts too.

/ Jonas

On Thu, Jul 23, 2015 at 12:32 AM, Tim Guan-tin Chien
<timd...@mozilla.com> wrote:

> The one big exception here is the Browser API proxies. Sometimes
> System app would need to access the Browser API methods exposed to
> it's own embedder. Unless we implement some kind of
> |navigator.mozControlMyOwnBrowserFrame| API to host these methods, we
> would be still rely on shell.js to use them, for now. I can send out
> another proposal if people are interested.
>

[Fabrice Desré]

Hi all,

Sorry for the late reply...

Let's do a bit or b2g archeology... and go back to this discussion:
https://groups.google.com/forum/#!topic/mozilla.dev.webapi/2ykFnFyOJSc

I don't think much has changed in terms of proposal. As you can see, we
didn't come to a consensus, hence the FIXME that I put more than 3 years
ago in
http://hg.mozilla.org/mozilla-central/annotate/afa67b6957bb/b2g/chrome/content/shell.js#l751

I would be very happy if you could get rid of that!

Some more comments inline.

On 07/23/2015 12:32 AM, Tim Guan-tin Chien wrote:

> 3.1) mozChromeEvent poses a performance problem, since a lot of
> components in System app need to listen to it, we ended up having 30x
> event listeners installed and only one of these callbacks will
> actually act on it.

We are using not just "mozChromeEvent" but different "mozChromeXXX"
event names to not pay this performance penalty.

> 3.2) Spaghetti in shell.js is in no way testable other than Gaia
> integration tests, but in fact they are part of APIs that should have
> been testable in dom/*/mochitests/

I'm not sure about the testability issue. We can write mocks for all the
uses we have I believe, but we still have debt here.

> 3.4) It only works in-process. I don't know if that's desirable but if
> it's possible to run System app in non-root process, we should
> definitely do it.

That would help, yes, though we could inject our event passing mechanism
as a frame script in the system app too and remote the events.

> 4) Proposed evolution direction
>
> I proposed for applicable mozChromeEvent or mozContentEvents use
> cases, we should move them back to their respective APIs, and only
> expose these method/events with a special |-system| permission. For
> example, the open-app event is dispatched whenever Gecko need Gaia
> System to create an app frame, it could be replaced with an interface
> like this (DOMApplicationsManager is exposed under
> |navigator.mozApps.mgmt|)
>
> partial interface DOMApplicationsManager : EventTarget {
> [CheckAnyPermissions="webapps-manage-system"]
> attribute EventHandler onlaunchrequest
> }
>
> And Gaia System window management will act on this event.

I'm not a fan of adding an embedding part to "normal" web apis. I'd
rather have the embedding api stand on its own.

> 5) Possible alternatives
>
> There will always be some System level UIs and we will always need
> some place to host it. The only alternative I could think of would be
> granting chrome privilege to System app, but even with that we will be
> only fixing 3.1 and not 3.2-4.

Granting chrome privileges to the system app is not an option. It's a
sure road to evil things.

Overall, that will be quite some work to fully remove these events. We
have to be careful with the return on investment here. As much as I'd
like that to happen, I also have several other projects that need
similar skills for people that have cycles left!

Fabrice
--
Fabrice Desré
b2g team
Mozilla Corporation

[Jonas Sicking]

On Fri, Jul 31, 2015 at 6:04 PM, Fabrice Desré <fab...@mozilla.com> wrote:
> Hi all,
>
> Sorry for the late reply...
>
> Let's do a bit or b2g archeology... and go back to this discussion:
> https://groups.google.com/forum/#!topic/mozilla.dev.webapi/2ykFnFyOJSc
>
> I don't think much has changed in terms of proposal. As you can see, we
> didn't come to a consensus, hence the FIXME that I put more than 3 years
> ago in
> http://hg.mozilla.org/mozilla-central/annotate/afa67b6957bb/b2g/chrome/content/shell.js#l751
>
> I would be very happy if you could get rid of that!

If a decision is needed, I'd say we should do the "flat" proposal
since that seems to require less boilerplate code in the
implementation. I.e. a mozEmbed object with all the necessary

But really, I'm happy to let whoever writes the patch implement
whatever approach they want.

>> 3.2) Spaghetti in shell.js is in no way testable other than Gaia
>> integration tests, but in fact they are part of APIs that should have
>> been testable in dom/*/mochitests/
>
> I'm not sure about the testability issue. We can write mocks for all the
> uses we have I believe, but we still have debt here.

I *really* like the idea of being able to write mochitests which
ensure that calling various DOM APIs fire the expected callbacks in
the system app.

But is this really harder when we're using DOM events?

>> 4) Proposed evolution direction
>>
>> I proposed for applicable mozChromeEvent or mozContentEvents use
>> cases, we should move them back to their respective APIs, and only
>> expose these method/events with a special |-system| permission. For
>> example, the open-app event is dispatched whenever Gecko need Gaia
>> System to create an app frame, it could be replaced with an interface
>> like this (DOMApplicationsManager is exposed under
>> |navigator.mozApps.mgmt|)
>>
>> partial interface DOMApplicationsManager : EventTarget {
>> [CheckAnyPermissions="webapps-manage-system"]
>> attribute EventHandler onlaunchrequest
>> }
>>
>> And Gaia System window management will act on this event.
>
> I'm not a fan of adding an embedding part to "normal" web apis. I'd
> rather have the embedding api stand on its own.

I kind'a agree. If nothing else it's easier to document when the two
are cleanly separated.

But I don't feel strongly.

>> 5) Possible alternatives
>>
>> There will always be some System level UIs and we will always need
>> some place to host it. The only alternative I could think of would be
>> granting chrome privilege to System app, but even with that we will be
>> only fixing 3.1 and not 3.2-4.
>
> Granting chrome privileges to the system app is not an option. It's a
> sure road to evil things.

Yeah. One big problem with granting the system app chrome privileges
is that it makes it much more fragile to allow addons to inject into
the system app.

I'd also be worried about partners modifying the system app using chrome code.

> Overall, that will be quite some work to fully remove these events. We
> have to be careful with the return on investment here. As much as I'd
> like that to happen, I also have several other projects that need
> similar skills for people that have cycles left!

Definitely agree. But if the people working on the system app feel
like it makes it possible for them to reduce technical debt, then I
definitely think that they should submit patches to gecko. This part
of gecko is all JS I think, so shouldn't be too different from
modifying Gaia.

/ Jonas

[Tim Guan-tin Chien]

On Sun, Aug 2, 2015 at 9:45 AM, Jonas Sicking <jo...@sicking.cc> wrote:
> On Fri, Jul 31, 2015 at 6:04 PM, Fabrice Desré <fab...@mozilla.com> wrote:
>> Hi all,
>>
>> Sorry for the late reply...
>>
>> Let's do a bit or b2g archeology... and go back to this discussion:
>> https://groups.google.com/forum/#!topic/mozilla.dev.webapi/2ykFnFyOJSc
>>
>> I don't think much has changed in terms of proposal. As you can see, we
>> didn't come to a consensus, hence the FIXME that I put more than 3 years
>> ago in
>> http://hg.mozilla.org/mozilla-central/annotate/afa67b6957bb/b2g/chrome/content/shell.js#l751
>>
>> I would be very happy if you could get rid of that!
>
> If a decision is needed, I'd say we should do the "flat" proposal
> since that seems to require less boilerplate code in the
> implementation. I.e. a mozEmbed object with all the necessary
>
> But really, I'm happy to let whoever writes the patch implement
> whatever approach they want.
>

I hope we don't couple different APIs too much. Creating an API that
has no purpose on it's own other than bridging calls from other APIs,
sound like collecting mozChromeEvents in a nicer way and not actually
get rid of it in concept.

Especially when we have already know these events will not be handled
by one single component in System UIs.

I would opt for an embedded API only if we have strong need to finish
off this on the short term, e.g. if we want to move System app our of
the root process soon. That API would still be an FIXME though :'(

>>> 3.2) Spaghetti in shell.js is in no way testable other than Gaia
>>> integration tests, but in fact they are part of APIs that should have
>>> been testable in dom/*/mochitests/
>>
>> I'm not sure about the testability issue. We can write mocks for all the
>> uses we have I believe, but we still have debt here.
>
> I *really* like the idea of being able to write mochitests which
> ensure that calling various DOM APIs fire the expected callbacks in
> the system app.
>
> But is this really harder when we're using DOM events?
>

The plain bloody fact is, not single event from shell.js to System app
is covered in any tests other than end-to-end tests like Gij or Gip.

You simply can't test shell.js in Firefox Desktop mochitests because
they are not packaged in these builds.

I assume by mocking, Fabrice means mocking |shell.sendChromeEvent| to
ensure it is being called from the API code. My questions for this
approach would be (1) is that the test we want and we could to live
with and (2) if it's doing able why isn't it's already being done.
Forgive me for not understand Gecko JSM imports enough to tell if it's
actually doable without seeing one example.

Other than Fx Desktop, we don't have Chrome mochitests in B2G so we
couldn't import shell.js directly and test it.

We are indeed testing APIs in B2G mochitests. However, I want to call
out bug 1094055, which makes mochitest frames impossible to get be run
in-proc when running on B2G, let alone being hosted by shell.html
directly and catch these events.

These are conclusions I have on our current test infra, gathered while
working on mozInputMethod API. Correct me if I am wrong and please
please tell me some good news.

>>> 4) Proposed evolution direction
>>>
>>> I proposed for applicable mozChromeEvent or mozContentEvents use
>>> cases, we should move them back to their respective APIs, and only
>>> expose these method/events with a special |-system| permission. For
>>> example, the open-app event is dispatched whenever Gecko need Gaia
>>> System to create an app frame, it could be replaced with an interface
>>> like this (DOMApplicationsManager is exposed under
>>> |navigator.mozApps.mgmt|)
>>>
>>> partial interface DOMApplicationsManager : EventTarget {
>>> [CheckAnyPermissions="webapps-manage-system"]
>>> attribute EventHandler onlaunchrequest
>>> }
>>>
>>> And Gaia System window management will act on this event.
>>
>> I'm not a fan of adding an embedding part to "normal" web apis. I'd
>> rather have the embedding api stand on its own.
>
> I kind'a agree. If nothing else it's easier to document when the two
> are cleanly separated.
>
> But I don't feel strongly.
>

We already have mozApps.mgmt. What's conceptually different between
management methods and embedder, system-only methods?

(I already spoken about standalone embedding API above)

>>> 5) Possible alternatives
>>>
>>> There will always be some System level UIs and we will always need
>>> some place to host it. The only alternative I could think of would be
>>> granting chrome privilege to System app, but even with that we will be
>>> only fixing 3.1 and not 3.2-4.
>>
>> Granting chrome privileges to the system app is not an option. It's a
>> sure road to evil things.
>
> Yeah. One big problem with granting the system app chrome privileges
> is that it makes it much more fragile to allow addons to inject into
> the system app.
>
> I'd also be worried about partners modifying the system app using chrome code.
>
>> Overall, that will be quite some work to fully remove these events. We
>> have to be careful with the return on investment here. As much as I'd
>> like that to happen, I also have several other projects that need
>> similar skills for people that have cycles left!
>
> Definitely agree. But if the people working on the system app feel
> like it makes it possible for them to reduce technical debt, then I
> definitely think that they should submit patches to gecko. This part
> of gecko is all JS I think, so shouldn't be too different from
> modifying Gaia.
>
> / Jonas

That's what I have been doing and I plan to continue to do so, if
there are management support.

[Fabrice Desré]

On 08/02/2015 08:01 AM, Tim Guan-tin Chien wrote:
> I hope we don't couple different APIs too much. Creating an API that
> has no purpose on it's own other than bridging calls from other APIs,
> sound like collecting mozChromeEvents in a nicer way and not actually
> get rid of it in concept.

I don't see anything wrong with with concept of having an embedding api.
But we can agree that we disagree on that ;)

> Especially when we have already know these events will not be handled
> by one single component in System UIs.

But even with a single mozEmbedding api, nothing would prevent different
components of the system app to provide their feature, right? Eg,
mozEmbedding.onlaunchapp = ... in some apps module and
mozEmbedding.onchoseactivity = ... in the activity module.

> The plain bloody fact is, not single event from shell.js to System app
> is covered in any tests other than end-to-end tests like Gij or Gip.

Yes, Gi tests are the equivalent of mochitest-chrome in firefox desktop,
where you need the full UI to run the tests.

> You simply can't test shell.js in Firefox Desktop mochitests because
> they are not packaged in these builds.
>
> I assume by mocking, Fabrice means mocking |shell.sendChromeEvent| to
> ensure it is being called from the API code. My questions for this
> approach would be (1) is that the test we want and we could to live
> with and (2) if it's doing able why isn't it's already being done.
> Forgive me for not understand Gecko JSM imports enough to tell if it's
> actually doable without seeing one example.

We usually test web apis using plain mochitests that don't involve
product specific UI. That means that we bypass the UI hooks during tests
at the api level (for instance we don't wait for the installation
confirmation dialog in mozApps tests).

I agree that we currently have a gray zone in b2g where the b2g glue in
shell.js/SystemAppProxy.jsm is only exercised by Gi tests. But since
this is where the coupling between gecko and gaia happens, this seemed
acceptable to me to catch regressions.

I also agree that it would be even better to have more tests than what
we currently have in
https://mxr.mozilla.org/mozilla-central/source/b2g/components/test/mochitest/

> Other than Fx Desktop, we don't have Chrome mochitests in B2G so we
> couldn't import shell.js directly and test it.
>
> We are indeed testing APIs in B2G mochitests. However, I want to call
> out bug 1094055, which makes mochitest frames impossible to get be run
> in-proc when running on B2G, let alone being hosted by shell.html
> directly and catch these events.
>
> These are conclusions I have on our current test infra, gathered while
> working on mozInputMethod API. Correct me if I am wrong and please
> please tell me some good news.

No good news, sorry. Looks like we still need to fix this bug :(

> We already have mozApps.mgmt. What's conceptually different between
> management methods and embedder, system-only methods?
>
> (I already spoken about standalone embedding API above)

The difference to me is that mozApps.mgmt methods can be used
legitimately by more apps than the embedding api. More generally, there
is also the case of features that don't have an existing content facing
api to hook on, like system messages or activities (they just have a
public constructor).