This is a bug. When no additional apps are selected to connect (through the UI prompt), we still need to do connections for the existing allowed apps. Although we haven't supported the UI prompt yet, we have to correct the potentially wrong logic.

Hi Nikhil, could take the review? I made a function _findAllowedSubAppManifestURLs() to refactorize the common part. If |aAddEntryIfNotFound| is true, this function will return an new entry (empty array) if it's not found, so that we can add newly selected apps to that entry later.

Comment on attachment 8427043 [details] [diff] [review] Patch Review of attachment 8427043 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/InterAppCommService.jsm @@ +460,5 @@ > messagePortIDs: messagePortIDs, > oid: aOuterWindowID, requestID: aRequestID }); > }, > > + _findAllowedSubAppManifestURLs: function(aKeyword, aPubAppManifestURL, Nit: please call this _getAllowedSubAppManifestURLs. Also comment explaining the parameters and what the function does. @@ +469,5 @@ > + if (!aAddEntryIfNotFound) { > + return []; > + } > + > + // Add a new entry for the |aKeyword|; Nit: s/the // and s/;/./ :) @@ +470,5 @@ > + return []; > + } > + > + // Add a new entry for the |aKeyword|; > + allowedPubAppManifestURLs = this._allowedConnections[aKeyword] = {}; I don't think you should be creating pub entries in a function for sub. Don't support the aAddEntryIfNotFound for publishers. If true is only going to be passed at one call site, it would be better to just have two functions, one for creation and one for find. Put assertions as required. Boolean parameters are a readability nightmare. @@ +824,5 @@ > + // Still need to do the connections for the existing allowed apps. > + let allowedSubAppManifestURLs = > + this._findAllowedSubAppManifestURLs(keyword, manifestURL, false); > + > + this._dispatchMessagePorts(keyword, manifestURL, allowedSubAppManifestURLs, This is also done at the end of the function. Could you restructure the function to instead do special stuff if |selectedApps.length > 0| and then always do the dispatchMessagePorts() at the end.

Fix the latest comment. Thanks Nikhil for the review!

Try: https://tbpl.mozilla.org/?tree=Try&rev=a98a4e666767

Comment on attachment 8428202 [details] [diff] [review] Patch, V2 Review of attachment 8428202 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/InterAppCommService.jsm @@ +461,5 @@ > oid: aOuterWindowID, requestID: aRequestID }); > }, > > + /** > + * Fetch the subscribers that used to be allowed to connect. Nit: s/used to be/are currently/

Fixed the comment #5. Thanks for the review! Landing. https://hg.mozilla.org/integration/b2g-inbound/rev/8473f191f232

sorry had to backout for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=40520145&tree=B2g-Inbound

Yeap, thanks Carsten for catching this. It was my fault when rebasing. Relanding. https://hg.mozilla.org/integration/b2g-inbound/rev/bed24d8a34a3