Patch 1 was written by nalexander.

Part 2: Distinguish between OrderedBroadcasts with null and default/undefined permissions.

Comment on attachment 769989 [details] [diff] [review] part-2-OrderedBroadcast-permissions.patch Review of attachment 769989 [details] [diff] [review]: ----------------------------------------------------------------- Address tests, r=nalexander. ::: mobile/android/base/OrderedBroadcastHelper.java @@ +78,5 @@ > > + // A missing (undefined) permission means the intent will be limited > + // to the current package. A null means no permission, so any > + // package can receive the intent. > + final String permission = message.has("permission") Does this look better with isNull ? optString(..., PER_ANDROID_PACKAGE_PERMISSION)? ::: mobile/android/modules/OrderedBroadcast.jsm @@ +79,5 @@ > type: "OrderedBroadcast:Send", > action: action, > responseEvent: responseEvent, > token: { callbackId: callbackId, data: token || null }, > + permission: permission, This should break the tests. If it doesn't, something is seriously wrong. Please verify and update the tests.

Green try tests: https://tbpl.mozilla.org/?tree=Try&rev=23111a39ccd9

(In reply to Chris Peterson (:cpeterson) from comment #3) > Green try tests: > https://tbpl.mozilla.org/?tree=Try&rev=23111a39ccd9 Fair enough. Looking at the tests again, I see how this happens. Bonus points for testing the new functionality, but you'd need to add an additional permission protected test receiver and I'm not that concerned about it. This is mostly for calling Fennec receivers anyway, and JS callers will quickly find out if their permissions are incorrect.

(In reply to Nick Alexander :nalexander from comment #2) > Does this look better with isNull ? optString(..., PER_ANDROID_PACKAGE_PERMISSION)? I don't think this code can be simplified with optString(). If a permission value exists and is null, optString() will return the string "null" instead of null. isNull() will return null if a value exists and is null or if a value does not exist (undefined), so we still need to check has("permission") to differentiate between null and undefined values.

Comment on attachment 769989 [details] [diff] [review] part-2-OrderedBroadcast-permissions.patch Review of attachment 769989 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/OrderedBroadcastHelper.java @@ +80,5 @@ > + // to the current package. A null means no permission, so any > + // package can receive the intent. > + final String permission = message.has("permission") > + ? (message.isNull("permission") ? null : message.getString("permission")) > + : GlobalConstants.PER_ANDROID_PACKAGE_PERMISSION; Nit: prefer operators at end of line, like the rest of the file. ::: mobile/android/modules/OrderedBroadcast.jsm @@ +84,1 @@ > }); For the record, this direct attributing works because of rule JO#8 in <http://es5.github.io/#x15.12.3>, which specifies that undefined-valued attributes aren't serialized... because JSONObject will parse them as the string "undefined", which would get treated as a permission. Glad I checked that :P

https://hg.mozilla.org/integration/mozilla-inbound/rev/f43a6971bb1b https://hg.mozilla.org/integration/mozilla-inbound/rev/b6bf318e6a90 Fixed operators at end of line noted in comment 6.

https://hg.mozilla.org/mozilla-central/rev/f43a6971bb1b https://hg.mozilla.org/mozilla-central/rev/b6bf318e6a90