963234 - Notifications are broken in mulet

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Alexandre Poirot [:ochameau]]

That's because b2g/components/AlertsService.js gets the top level window,
and then interacts directly with AlertsHelper object.
That's the only one code I've seen so far using direct references to JS objects within shell.js. Ideally we would only send events to shell.js, via the observer service, or something similar.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: abstract AlertsServices.js from shell.js

That's one way to make it work on the mulet, but there is some alternatives.
One alternative, that would introduce a bigger patch, is to move this AlertsHelper to the xpcom.
Most other components listen to mozContentEvent and send mozChromeEvent from the xpcom/jsm.

Comment 2

[[:fabrice] Fabrice Desré]

Comment on attachment 8364562 [details] [diff] [review]
abstract AlertsServices.js from shell.js

Review of attachment 8364562 [details] [diff] [review]:
-----------------------------------------------------------------

Alex, I think I like the "move this AlertsHelper to the xpcom" option better.

Comment 3

[:gerard-majax]

Since I've been playing with notifications for a while now, taking. In which XPCOM should we move this ? AlertsService ?

Comment 4

[Alexandre Poirot [:ochameau]]

We should move everything alerts-related from shell.js to b2g/components/AlertsService.js

Then I tend to think the only necessary thing you will need by doing that,
is using the SystemApp.jsm component I implemented in bug 963239 (which is in the git branch).
To replace shell.send*Event()/shell.contentBrowser.addEventListener by SystemApp.send*Event()/SystemApp.addEventListener().

Once you have a patch, I can either cherry-pick it from your branch, or you can open a pull request so that I include it in the git branch. That to ease rebasing the branch over last upstream master.

Comment 5

[:gerard-majax]

This has been easier than expected. I have a patch that complies to fabrice's suggestions and that is working fine on B2G Desktop. Let's confirm on device tomorrow :).

Comment 6

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #5)
> This has been easier than expected. I have a patch that complies to
> fabrice's suggestions and that is working fine on B2G Desktop. Let's confirm
> on device tomorrow :).

Nevermind, this was completely broken on OOP builds

Comment 7

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Moving AlertsHelper out of shell.js

Comment 8

[:gerard-majax]

As far as I could test, it works on device and in b2g desktop. I still have to address the SystemAppProxy.addEventListener() issue and finish implementing showAlertNotification().

Comment 9

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

After a careful look at other codes using SystemAppProxy, it was just the content-start observer that helped :)

Comment 10

[Alexandre Poirot [:ochameau]]

The whole point of _pendingListeners in SystemAppProxy is to prevent having to wait for content-start...
so there is something wrong with this list!

Comment 11

[:gerard-majax]

(In reply to Alexandre Poirot (:ochameau) from comment #10)
> The whole point of _pendingListeners in SystemAppProxy is to prevent having
> to wait for content-start...
> so there is something wrong with this list!

Okay, I'll continue investigating this. From what I saw yesterday, the content being used when performing the failing addEventListener() was in a strange shape: innerWidth/innerHeight were 0, for example.

Comment 12

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Asking feedback for the removal of content-start.

Comment 13

[:gerard-majax]

> $ git revert 05f96a18fa3479e3e37232aed41ae466258f3f21
> [detached HEAD 1adff20] Revert "Bug 963234 - abstract AlertsServices.js from shell.js r=fabrice"
>  2 files changed, 7 insertions(+), 19 deletions(-)
> $ git cherry-pick bug963234 
> [detached HEAD 6848f99] Bug 963234 - Moving AlertsHelper out of shell.js
>  5 files changed, 275 insertions(+), 206 deletions(-)
>  create mode 100644 b2g/components/AlertsHelper.jsm

So far, it works as expected.

Comment 14

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Every bits should be in place now.

Comment 15

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Comment 16

[:gerard-majax]

It looks like try is being green: https://tbpl.mozilla.org/?tree=Try&rev=ceee271dbd2c

Comment 17

[[:fabrice] Fabrice Desré]

Comment on attachment 8412643 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Review of attachment 8412643 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/AlertsHelper.jsm
@@ +38,5 @@
> +let AlertsHelper = {
> +
> +  _listeners: {},
> +
> +  init: function alert_init() {

here and everywhere else: no need to name functions anymore.

@@ +42,5 @@
> +  init: function alert_init() {
> +    ppmm.addMessageListener('app-notification-send', this);
> +    ppmm.addMessageListener('alert-notification-send', this);
> +    ppmm.addMessageListener('alert-notification-close', this);
> +    SystemAppProxy.addEventListener('mozContentEvent', this);

these are never removed. They should be in an xpcom-shutdown observer for instance.

@@ +55,5 @@
> +      case kNotificationClick:
> +      case kNotificationClose:
> +        this.handleNotificationEvent(detail);
> +        break;
> +    }

so, we're getting all the mozContentEvent(s) ? Can we switch to use a NotificationEvent instead?

@@ +121,5 @@
> +
> +  registerAppListener: function alert_registerAppListener(uid, listener) {
> +    this._listeners[uid] = listener;
> +
> +    let app = DOMApplicationRegistry.getAppByManifestURL(listener.manifestURL);

using directly DOMApplicationRegistry is discouraged. Switch to the AppsService if possible.

@@ +181,5 @@
> +    // If we have a manifest URL, get the icon and title from the manifest
> +    // to prevent spoofing.
> +    let app = DOMApplicationRegistry.getAppByManifestURL(manifestURL);
> +    DOMApplicationRegistry.getManifestFor(manifestURL).then((aManifest) => {
> +      let helper = new ManifestHelper(aManifest, app.origin);

you can use manifestURL instead of app.origin and not do the getAppByManifestURL() call.

Comment 18

[:gerard-majax]

(In reply to Fabrice Desré [:fabrice] from comment #17)
> Comment on attachment 8412643 [details] [diff] [review]
> 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch
> 
> Review of attachment 8412643 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/components/AlertsHelper.jsm
> @@ +38,5 @@
> > +let AlertsHelper = {
> > +
> > +  _listeners: {},
> > +
> > +  init: function alert_init() {
> 
> here and everywhere else: no need to name functions anymore.

Fixed.

> 
> @@ +42,5 @@
> > +  init: function alert_init() {
> > +    ppmm.addMessageListener('app-notification-send', this);
> > +    ppmm.addMessageListener('alert-notification-send', this);
> > +    ppmm.addMessageListener('alert-notification-close', this);
> > +    SystemAppProxy.addEventListener('mozContentEvent', this);
> 
> these are never removed. They should be in an xpcom-shutdown observer for
> instance.

Right, fixed.

> 
> @@ +55,5 @@
> > +      case kNotificationClick:
> > +      case kNotificationClose:
> > +        this.handleNotificationEvent(detail);
> > +        break;
> > +    }
> 
> so, we're getting all the mozContentEvent(s) ? Can we switch to use a
> NotificationEvent instead?

This is something Alexandre pointed at but was not sure as far as I remember. It means we have to change it also on Gaia side, but if you think it's better, then let's do it :).

And yes, I saw that we get all mozContentEvent that are sent.

Comment 19

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Addressing a first set of remarks.

Comment 20

[:gerard-majax]

Attachment: Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Gaia pull request to handle the new mozNotificationEvent.

Comment 21

[:gerard-majax]

(In reply to Fabrice Desré [:fabrice] from comment #17)
> Comment on attachment 8412643 [details] [diff] [review]
> 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch
> 
> Review of attachment 8412643 [details] [diff] [review]:
> -----------------------------------------------------------------
> 

[...]

> 
> @@ +121,5 @@
> > +
> > +  registerAppListener: function alert_registerAppListener(uid, listener) {
> > +    this._listeners[uid] = listener;
> > +
> > +    let app = DOMApplicationRegistry.getAppByManifestURL(listener.manifestURL);
> 
> using directly DOMApplicationRegistry is discouraged. Switch to the
> AppsService if possible.
> 
> @@ +181,5 @@
> > +    // If we have a manifest URL, get the icon and title from the manifest
> > +    // to prevent spoofing.
> > +    let app = DOMApplicationRegistry.getAppByManifestURL(manifestURL);
> > +    DOMApplicationRegistry.getManifestFor(manifestURL).then((aManifest) => {
> > +      let helper = new ManifestHelper(aManifest, app.origin);
> 
> you can use manifestURL instead of app.origin and not do the
> getAppByManifestURL() call.

Both are fixed, but I had to expose getManifestFor() in AppsService.

Comment 22

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Updated patch that fixes all of the remarks.

Comment 23

[:gerard-majax]

Pending try: https://tbpl.mozilla.org/?tree=Try&rev=d0919c65e4d5

Comment 24

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Moving registerFrame earlier, as suggested on IRC.

Comment 25

[:gerard-majax]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

This is the Gaia part patch that enables the use of mozNotificationEvent.

Comment 26

[Alexandre Poirot [:ochameau]]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

I'm not reviewer for such gaia patch.
I think you will have to keep supporting both old and new events and may be cleanup after a bit, otherwise you will have hard time landing these patches without breaking tests!

Comment 27

[:gerard-majax]

(In reply to Alexandre Poirot (:ochameau) from comment #26)
> Comment on attachment 8413176 [details] [review]
> Link to Github https://github.com/mozilla-b2g/gaia/pull/18709
> 
> I'm not reviewer for such gaia patch.
> I think you will have to keep supporting both old and new events and may be
> cleanup after a bit, otherwise you will have hard time landing these patches
> without breaking tests!

Obviously. There was a thread regarding this on dev-b2g back in november 2013, and one conclusion I just recovered was that the majority of people were in favor of:
 1. land a first gaia patch that disable the tests which will get broken
 2. land the gecko change
 3. once gecko is fjxed, re-enable the tests.

Comment 28

[Alexandre Poirot [:ochameau]]

Comment on attachment 8413771 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Review of attachment 8413771 [details] [diff] [review]:
-----------------------------------------------------------------

I took a brief look at alert modification, but the SystemAppProxy modification looks good (Thanks having tracked down this issue!).
Works great on device and mulet when having both gecko/gaia patches.
Same thing than gaia patch, I'm not reviewer of this code.

::: b2g/components/AlertsHelper.jsm
@@ +97,5 @@
> +      topic = 'alertclickcallback';
> +    } else if (detail.type === kNotificationShow) {
> +      topic = 'alertshow';
> +    } else {
> +      /* desktop-notification-close */

nit: I would have put kNotificationClose instead of string value

@@ +145,5 @@
> +    this._listeners[uid] = listener;
> +
> +    let manifest = appsService.getManifestFor(listener.manifestURL);
> +    if (manifest) {
> +      let helper = new ManifestHelper(manifest, listener.manifestURL);

ManifestHelper expects the origin as second argument, but I don't know how much it can break it.

::: b2g/components/FxAccountsMgmtService.jsm
@@ +51,5 @@
>      Services.obs.addObserver(this, ONLOGIN_NOTIFICATION, false);
>      Services.obs.addObserver(this, ONVERIFIED_NOTIFICATION, false);
>      Services.obs.addObserver(this, ONLOGOUT_NOTIFICATION, false);
> +    SystemAppProxy.addEventListener("mozFxAccountsContentEvent",
> +                                        FxAccountsMgmtService);

nit: indentation.

Comment 29

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Addressing nits from feedback+ from :ochameau.

Comment 30

[[:fabrice] Fabrice Desré]

Comment on attachment 8413893 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch

Review of attachment 8413893 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/AlertsHelper.jsm
@@ +11,5 @@
> +const Cc = Components.classes;
> +
> +Cu.import('resource://gre/modules/XPCOMUtils.jsm');
> +Cu.import('resource://gre/modules/Services.jsm');
> +Cu.import("resource://gre/modules/AppsUtils.jsm");

unused.

@@ +35,5 @@
> +}
> +
> +const kNotificationShow  = "desktop-notification-show";
> +const kNotificationClick = "desktop-notification-click";
> +const kNotificationClose = "desktop-notification-close";

double quotes here, single quotes elsewhere... We use double quotes in chrome JS in general.

@@ +76,5 @@
> +      case kNotificationShow:
> +      case kNotificationClick:
> +      case kNotificationClose:
> +        this.handleNotificationEvent(detail);
> +        break;

nit: you could log the default case of getting an unexpected event type.

@@ +143,5 @@
> +
> +  registerAppListener: function(uid, listener) {
> +    this._listeners[uid] = listener;
> +
> +    let manifest = appsService.getManifestFor(listener.manifestURL);

getManifestFor() returns a Promis<manifest>. How does that code work?

::: dom/apps/src/AppsService.js
@@ +40,5 @@
>    },
>  
> +  getManifestFor: function getManifestFor(aManifestURL) {
> +    debug("getManifestFor(" + aManifestURL + ")");
> +    return DOMApplicationRegistry.getManifestFor(aManifestURL);

that will only work in the parent process, but nothing prevents this code to run in the child.

::: dom/interfaces/apps/nsIAppsService.idl
@@ +21,5 @@
>  interface nsIAppsService : nsISupports
>  {
>    mozIApplication getAppByManifestURL(in DOMString manifestURL);
>  
> +  nsISupports getManifestFor(in DOMString manifestURL);

document what the nsISupports actually is.

Comment 31

[[:fabrice] Fabrice Desré]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Looks fine, but you need to ask review to a system app peer.

Comment 32

[:gerard-majax]

(In reply to Fabrice Desré [:fabrice] from comment #30)
> Comment on attachment 8413893 [details] [diff] [review]
> 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js.patch
> 
> Review of attachment 8413893 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/components/AlertsHelper.jsm
> @@ +11,5 @@
> > +const Cc = Components.classes;
> > +
> > +Cu.import('resource://gre/modules/XPCOMUtils.jsm');
> > +Cu.import('resource://gre/modules/Services.jsm');
> > +Cu.import("resource://gre/modules/AppsUtils.jsm");
> 
> unused.

Which one ? We need AppsUtils for ManifestHelper

> 
> @@ +35,5 @@
> > +}
> > +
> > +const kNotificationShow  = "desktop-notification-show";
> > +const kNotificationClick = "desktop-notification-click";
> > +const kNotificationClose = "desktop-notification-close";
> 
> double quotes here, single quotes elsewhere... We use double quotes in
> chrome JS in general.

Sorry, fixed.

> 
> @@ +76,5 @@
> > +      case kNotificationShow:
> > +      case kNotificationClick:
> > +      case kNotificationClose:
> > +        this.handleNotificationEvent(detail);
> > +        break;
> 
> nit: you could log the default case of getting an unexpected event type.

Done.

> 
> @@ +143,5 @@
> > +
> > +  registerAppListener: function(uid, listener) {
> > +    this._listeners[uid] = listener;
> > +
> > +    let manifest = appsService.getManifestFor(listener.manifestURL);
> 
> getManifestFor() returns a Promis<manifest>. How does that code work?

I'm as surprised as you, at first I used it as a Promise and I kept getting errors about lacking .then(). I'll investigate what happens, thanks for confirming that we should be getting a Promise.

> 
> ::: dom/apps/src/AppsService.js
> @@ +40,5 @@
> >    },
> >  
> > +  getManifestFor: function getManifestFor(aManifestURL) {
> > +    debug("getManifestFor(" + aManifestURL + ")");
> > +    return DOMApplicationRegistry.getManifestFor(aManifestURL);
> 
> that will only work in the parent process, but nothing prevents this code to
> run in the child.

Should we make it work in the child or protect against child calling us?

> 
> ::: dom/interfaces/apps/nsIAppsService.idl
> @@ +21,5 @@
> >  interface nsIAppsService : nsISupports
> >  {
> >    mozIApplication getAppByManifestURL(in DOMString manifestURL);
> >  
> > +  nsISupports getManifestFor(in DOMString manifestURL);
> 
> document what the nsISupports actually is.

Done.

Comment 33

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #32)
> (In reply to Fabrice Desré [:fabrice] from comment #30)
> > Comment on attachment 8413893 [details] [diff] [review]

[...]

> > @@ +143,5 @@
> > > +
> > > +  registerAppListener: function(uid, listener) {
> > > +    this._listeners[uid] = listener;
> > > +
> > > +    let manifest = appsService.getManifestFor(listener.manifestURL);
> > 
> > getManifestFor() returns a Promis<manifest>. How does that code work?
> 
> I'm as surprised as you, at first I used it as a Promise and I kept getting
> errors about lacking .then(). I'll investigate what happens, thanks for
> confirming that we should be getting a Promise.

It seems that returning a jsval instead of nsISupports makes it behaving as a Promise as expected.

Comment 34

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Updated patch that should fix all previous remarks.

Comment 35

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Fixing some style. For some reason, I get a failure in gaia marionette test: apps/system/test/marionette/desktop_notification_test.js

Comment 36

[:gerard-majax]

Comment on attachment 8414404 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Gaia integration tests now passes correctly locally, and all breakages seems to be gone also on Travis.

Comment 37

[:gerard-majax]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Alive and Michael, I'd like that you review the Gaia part of those changes. I'm adding mozNotificationEvent handling with the current mozChromeEvent. The plan is that this way, Gaia will work as expected during the transition. Bug 1003063 is already filed to take care of removing mozChromeEvent, hence we will just have to want for its PR to be travis green.

Comment 38

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #37)
> Comment on attachment 8413176 [details] [review]
> Link to Github https://github.com/mozilla-b2g/gaia/pull/18709
> 
> Alive and Michael, I'd like that you review the Gaia part of those changes.
> I'm adding mozNotificationEvent handling with the current mozChromeEvent.
> The plan is that this way, Gaia will work as expected during the transition.
> Bug 1003063 is already filed to take care of removing mozChromeEvent, hence
> we will just have to want for its PR to be travis green.

Gaia integration tests should be fixed and passing before and after the Gecko patch: I checked on my system and everything seems to be fine.

Comment 39

[[:fabrice] Fabrice Desré]

Comment on attachment 8414404 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Review of attachment 8414404 [details] [diff] [review]:
-----------------------------------------------------------------

r- just for the changes to getManifestFor().

::: b2g/components/AlertsHelper.jsm
@@ +197,5 @@
> +    // If we have a manifest URL, get the icon and title from the manifest
> +    // to prevent spoofing.
> +    appsService.getManifestFor(manifestURL).then((manifest) => {
> +      let helper = new ManifestHelper(manifest, manifestURL);
> +      send(helper.name, helper.iconURLForSize(128));

nit: use a constant defined at the beginning of this file instead of '128'

::: dom/apps/src/AppsService.js
@@ +40,5 @@
>    },
>  
> +  getManifestFor: function getManifestFor(aManifestURL) {
> +    debug("getManifestFor(" + aManifestURL + ")");
> +    return DOMApplicationRegistry.getManifestFor(aManifestURL);

returning null for the promise will lead to weird errors. I would be ok with either throwing or returning a rejected promise when called in a child process.

::: dom/apps/src/AppsServiceChild.jsm
@@ +75,5 @@
>  
> +  getManifestFor: function getManifestFor(aManifestURL) {
> +    debug("getManifestFor() not yet supported on child!");
> +    return null;
> +  },

remove that one.

::: dom/interfaces/apps/nsIAppsService.idl
@@ +22,5 @@
>  {
>    mozIApplication getAppByManifestURL(in DOMString manifestURL);
>  
>    /**
> +   * Returns a Promise for the manifest for a given manifestURL.

Add: * This is only supported in the parent process.

Comment 40

[:gerard-majax]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Sorry, messed up events: mozChromeEvent/mozContentEvent should become mozChromeNotificationEvent/mozContentNotificationEvent rather than just mozNotificationEvent.

Comment 41

[:gerard-majax]

Comment on attachment 8414404 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Sorry, messed up events: mozChromeEvent/mozContentEvent should become mozChromeNotificationEvent/mozContentNotificationEvent rather than just mozNotificationEvent.

Comment 42

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Updating patch: fixing latest comments reported by fabrice and changing mozChrome/mozContent events in a saner manner. PR has been updated accordingly.

Pending try: https://tbpl.mozilla.org/?tree=Try&rev=53412d4455c9
Pending travis: https://travis-ci.org/mozilla-b2g/gaia/builds/24081668

Comment 43

[:gerard-majax]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Making use of {mozChrome,mozContent}NotificationEvent all tests are passing, travis is green.

Comment 44

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

r=me

Comment 45

[[:fabrice] Fabrice Desré]

Comment on attachment 8415066 [details] [diff] [review]
0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Review of attachment 8415066 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit fixed.

::: dom/apps/src/AppsService.js
@@ +48,5 @@
> +    } else {
> +      deferred.reject(
> +        new Error("Calling getManifestFor() from child is not supported"));
> +    }
> +    return deferred.promise;

why not just return DOMApplicationRegistry.getManifestFor(aManifestURL) when in the parent and create the new promise that we reject only in the child?

::: dom/interfaces/apps/nsIAppsService.idl
@@ +23,5 @@
>    mozIApplication getAppByManifestURL(in DOMString manifestURL);
>  
>    /**
> +   * Returns a Promise for the manifest for a given manifestURL.
> +   * This is only supported in the parent process.

: the promise will be rejected in content processes.

Comment 46

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8413176 [details] [review]
Link to Github https://github.com/mozilla-b2g/gaia/pull/18709

Nice interim solution.

Comment 47

[:gerard-majax]

Thanks, gaia part merged: https://github.com/mozilla-b2g/gaia/commit/1b2f9f11c5cd3a39d1c677190fe9bffa059569bf

I'll address latest fabrice comment, update the patch and mark it as checkin needed :)

Comment 48

[:gerard-majax]

Attachment: 0001-Bug-963234-Moving-AlertsHelper-out-of-shell.js-r-fab.patch

Fixing latest nits reported by fabrice.

Comment 49

[:gerard-majax]

Pending try: https://tbpl.mozilla.org/?tree=Try&rev=e75f17f6c243

Comment 50

[:gerard-majax]

Try is green, except one unrelated test https://tbpl.mozilla.org/?tree=Try&rev=e75f17f6c243

Comment 51

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/caa1de36d1ad

Comment 52

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/caa1de36d1ad