Since bug 814581, we've extracted frame messaging utility functions into |gMessageManager| in RadioInterfaceLayer.js[1]. To enhance modulation even more, functions of the same topic will be moved out of RadioInterfaceLayer.js. For modules still use frame messages rather than IPDL, they will certainly need the same messaging helper. In bug 873351, we're going to move SMS code out of RIL, but we need some method to emit a voicemail specific signal to content process. We'd either depends on IPDL move of voicemail in bug 833229, or we can simply share |gMessageManager| for now and deprecate it later. [1]: http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#255

1) new JS module "RilMessageManager.jsm" is created. Maybe a better naming? 2) all topic registrations are now managed by message manager itself, so those message names are removed from RIL_IPC_*_MSG_NAMES. But we still have to register what we're interested in though.

Can you split into a 2-part patch? It's not easy to look the patch back and forth. Part 1: Move some functions to another JSM. Part 2: your modified code.

Have a standalone _updateDebugFlag() instead.

Comment on attachment 794477 [details] [diff] [review] part 1/3: move to RilMessageManager.jsm Review of attachment 794477 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RilMessageManager.jsm @@ +90,5 @@ > + } > + ppmm = null; > + }, > + > + _registerMessageTarget: function _registerMessageTarget(topic, msg) { s/msg/target/ ? @@ +100,5 @@ > + list.push(topic); > + } > + } > + > + let target = msg.target; rm

Comment on attachment 794477 [details] [diff] [review] part 1/3: move to RilMessageManager.jsm Review of attachment 794477 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +252,5 @@ > } > }); > > XPCOMUtils.defineLazyGetter(this, "gMessageManager", function () { > + let ns = {}; What does the abbreviation 'ns' stand for? ::: dom/system/gonk/RilMessageManager.jsm @@ +90,5 @@ > + } > + ppmm = null; > + }, > + > + _registerMessageTarget: function _registerMessageTarget(topic, msg) { Agree with yoshi's comment. @@ +100,5 @@ > + list.push(topic); > + } > + } > + > + let target = msg.target; Agree with yoshi's comment.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #8) > > XPCOMUtils.defineLazyGetter(this, "gMessageManager", function () { > > + let ns = {}; > > What does the abbreviation 'ns' stand for? Name space I gues. The same could be found in http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#195 .

Address comment 7

Address comment 7.

update r=yoshi only.

Comment on attachment 795309 [details] [diff] [review] part 1/3: move to RilMessageManager.jsm : v2 Already reviewed by Yoshi & HsinYi.

Rebase after bug 864485

Rebase after bug 864485

rebase after bug 864485

https://hg.mozilla.org/integration/b2g-inbound/rev/0b45d07ccdee https://hg.mozilla.org/integration/b2g-inbound/rev/0a1f354d13a0 https://hg.mozilla.org/integration/b2g-inbound/rev/21b97e0e5a10

Needs clobber on Windows, followup to touch CLOBBER file: remote: https://hg.mozilla.org/integration/b2g-inbound/rev/679f571996e8

(In reply to Ed Morley [:edmorley UTC+1] from comment #18) > Needs clobber on Windows, followup to touch CLOBBER file: > remote: https://hg.mozilla.org/integration/b2g-inbound/rev/679f571996e8 Looks like this clobber belongs to bug 873351 instead.

Backed out for conflicts with the backout of bug 864485: remote: https://hg.mozilla.org/integration/b2g-inbound/rev/57a3b3ea1f5e remote: https://hg.mozilla.org/integration/b2g-inbound/rev/84ee1aab7cd8 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/d486094c1c6f

Full try for bug 864485, bug 907585, and bug 873351: https://tbpl.mozilla.org/?tree=Try&rev=2b850251174f

https://hg.mozilla.org/integration/b2g-inbound/rev/9464e393a501 https://hg.mozilla.org/integration/b2g-inbound/rev/9039460e09c5 https://hg.mozilla.org/integration/b2g-inbound/rev/8020b3013618

https://hg.mozilla.org/mozilla-central/rev/9464e393a501 https://hg.mozilla.org/mozilla-central/rev/9039460e09c5 https://hg.mozilla.org/mozilla-central/rev/8020b3013618

The phone doesn't boot any more with these patches: I/Gecko (11179): Security problem: Content process does not have `icc'. It will be killed.

Backed out at gwagner's request because apparently we like it when phones are able to boot. Who knew. https://hg.mozilla.org/integration/b2g-inbound/rev/dc4758d44b11

Comment on attachment 799318 [details] [diff] [review] part 2/3: re-implement message listener registration : v3 Review of attachment 799318 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +175,5 @@ > > function RadioInterfaceLayer() { > + let callback = this._receiveMessage.bind(this); > + gMessageManager.registerMessageListeners("icc", > + RIL_IPC_ICCMANAGER_MSG_NAMES, Now it requests for permission "icc", which doesn't exists at all. ::: dom/system/gonk/RilMessageManager.jsm @@ -196,5 @@ > - } > - return null; > - } > - } else if (RIL_IPC_ICCMANAGER_MSG_NAMES.indexOf(msg.name) != -1) { > - if (!msg.target.assertPermission("mobileconnection")) { Here! All ICC events are originally registered as "mobileconnection".

https://hg.mozilla.org/integration/b2g-inbound/rev/3d41fb8801aa https://hg.mozilla.org/integration/b2g-inbound/rev/4390128c42bf https://hg.mozilla.org/integration/b2g-inbound/rev/76a3fe16c9e2

Backout: https://hg.mozilla.org/integration/b2g-inbound/rev/97570ed1ad80

Created bug 926277 to enforce RIL OOP tests.

Please this into backlog.

Its functionality has been replaced by bug 833229 so it's no longer valid.