1181930 - Refactoring: move the message broadcaster out of Webapps.jsm

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: apps-broadcaster.patch

That's a small and safe refactoring, but I need it for another patch I'm working on.

Comment 2

[[:fabrice] Fabrice Desré]

try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f769cb5ba50

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: apps-broadcaster.patch

With the new module actually added to the patch...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e9556029e34

Comment 4

[[:fabrice] Fabrice Desré]

Attachment: apps-broadcaster.patch

Now passing tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fae526a85600

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8639891 [details] [diff] [review]
apps-broadcaster.patch

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

Nice!

::: dom/apps/MessageBroadcaster.jsm
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +"use strict";
> +
> +const { utils: Cu, classes: Cc, interface: Ci } = Components;

None of these constants are used.

@@ +12,5 @@
> +this.EXPORTED_SYMBOLS = ["MessageBroadcaster"];
> +
> +this.MessageBroadcaster = {
> +  appGetter: null,
> +  children: [],

Maybe "childrens"

@@ +21,5 @@
> +    }
> +    this.appGetter = aAppGetter;
> +  },
> +
> +  addMessageListener: function(aMsgNames, aApp, aMm) {

Maybe add a comment explaining that listeners are ref counted.

@@ +22,5 @@
> +    this.appGetter = aAppGetter;
> +  },
> +
> +  addMessageListener: function(aMsgNames, aApp, aMm) {
> +    aMsgNames.forEach(function (aMsgName) {

Use arrow functions here and in the rest of the patch, please.

@@ +23,5 @@
> +  },
> +
> +  addMessageListener: function(aMsgNames, aApp, aMm) {
> +    aMsgNames.forEach(function (aMsgName) {
> +      let man = aApp && aApp.manifestURL;

s/man/manifestURL/g, please

@@ +46,5 @@
> +
> +      // If the state reported by the registration is outdated, update it now.
> +      if ((aMsgName === 'Webapps:FireEvent') ||
> +          (aMsgName === 'Webapps:UpdateState')) {
> +        if (man) {

if (man && ((aMsgName === 'Webapps:FireEvent') || (aMsgName === 'Webapps:UpdateState')))

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/939e1009ebaf

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/939e1009ebaf