+++ This bug was initially created as a clone of Bug #715814 +++ Core support and b2g specific support landed in bug 715814

Moving to web apps, as this focuses on web apps in FF Android.

This is an early WIP to save my place. I created a new interface, nsINativeAppService that has two methods: jsval getApps(in nsIDOMMozActivityOptions options); void launch(in jsval appData, in nsIDOMMozActivityOptions options, in nsIActivityUIGlueCallback onresult); Still haven't quite figured out how I'll correctly return things to the apps. Android gives us a content provider URI, which can then be used to access the real data. I haven't seen a clear spec for what sort of data apps expect in return, and I imagine there isn't one, so I'll just have to have some special case code to reformat Android apps results into the equivalent Gaia app ones? That sounds painful... The opposite path will have to be done with synthetic apks, but has similar problems getting the data returned from Gecko into a ContentProvider. Still thinking my way through this....

(In reply to Wesley Johnston (:wesj) from comment #3) > Still haven't quite figured out how I'll correctly return things to the > apps. Android gives us a content provider URI, which can then be used to > access the real data. I haven't seen a clear spec for what sort of data apps > expect in return, and I imagine there isn't one, so I'll just have to have > some special case code to reformat Android apps results into the equivalent > Gaia app ones? That sounds painful... There's no spec indeed. Each activity is free to define its own data transfer format.

Josh said he's interested in this.

We need to enable System Message on Android as well, which is the back-end mechanism of Activities.

(In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #6) > We need to enable System Message on Android as well, which is the back-end > mechanism of Activities. Is there a bug for this already?

A WIP based off of wesj's previous patch. Added a mapping mechanism within NativeAppService to map the Web Activity name, type, and data to the equivalent (or as best as we can) Android intent action, type, and extras. Have not completed the mappings yet, but would like feedback on the way I am doing it.

Can test by launching a MozActivity in the main process in Fennec: new MozActivity({ name: "send", data: { type:"plain/text", text:"testing" }});

New WIP with a simplified implementation that uses Android's intent chooser for picking the appropriate activity to handle the WebActivity. This assumes that all webapps will have registered with intent filters with a synthesized APK. All that is needed from here is determining rest of the mappings from Web Activities to Intents. wesj / myk, do you know if there is something like this in the synthesized APK work? We need to make sure that the mappings are equal in both (and can share the same code to do it).

(In reply to Josh Dover from comment #7) > (In reply to Gene Lian [:gene] (PTO Dec. 25 ~ Jan. 5) from comment #6) > > We need to enable System Message on Android as well, which is the back-end > > mechanism of Activities. > > Is there a bug for this already? Sorry for the delayed response. Please see bug 895689.

Comment on attachment 8361405 [details] [diff] [review] Simplified Implementation using Android Intent chooser I'm busy. Brian has been doing some dom api stuff. Maybe he can look at this faster than me?

(In reply to Josh Dover from comment #11) > All that is needed from here is determining rest of the mappings from Web > Activities to Intents. wesj / myk, do you know if there is something like > this in the synthesized APK work? We need to make sure that the mappings are > equal in both (and can share the same code to do it). I think jhugman has done some work to determine these mappings, or at least a subset of them. cc:ing him and asking him for info on that.

Comment on attachment 8361405 [details] [diff] [review] Simplified Implementation using Android Intent chooser Review of attachment 8361405 [details] [diff] [review]: ----------------------------------------------------------------- I'm not too familiar with web activities, so I think Wes should give another pass at this. The part I'm most unsure about is how we decided which intents to support, which I commented on below. ::: mobile/android/base/GeckoAppShell.java @@ +1028,5 @@ > > + static Intent getIntentForWebActivity(String action, > + @OptionalGeneratedParameter String uri, > + @OptionalGeneratedParameter String mime, > + @OptionalGeneratedParameter JSONObject extras) { Are these optionalGeneratedParameter annotations necessary? I thought these were only used with JNI. @@ +1049,5 @@ > + } > + } > + > + // Setup URIs > + if (uri != null && !uri.isEmpty()) { String#isEmpty() was introduced in Android SDK 9, but we support as low as 8. Instead, use TextUtils#isEmpty (which will also cover the null check for you). @@ +1054,5 @@ > + intent.setData(Uri.parse(uri)); > + } > + > + // Setup type > + if (mime != null && !mime.isEmpty()) { Same here. ::: mobile/android/components/NativeAppService.js @@ +8,5 @@ > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > +Cu.import("resource://gre/modules/Services.jsm"); > +Cu.import("resource://gre/modules/HelperApps.jsm"); > + > +let activityMap = { Please order these keys alphabetically. @@ +13,5 @@ > + "pick": { > + action: "android.intent.action.GET_CONTENT", > + mime: data => processType(data.type), > + }, > + "activity": { OK, so here is where I start to get confused. A few of these correspond to the Firefox OS activities Wes linked me to (https://developer.mozilla.org/en-US/docs/WebAPI/Web_Activities#Firefox_OS_activities). But most of them -- activity, createShortcut, delete, edit, insert, main, paste, run, search, send, setWallpaper -- are not. How are we choosing the the activities that get listed here? The list of all ACTIONs at http://developer.android.com/reference/android/content/Intent.html is quite large. The ones here are just a small subset of these, so I'm curious why these were picked. Is there some other list somewhere outlining the functionality we want to support? Or were these picked arbitrarily? @@ +58,5 @@ > + } > +}; > + > +function mapActivity(name, data) { > + let mappedData = activityMap[name]; We should probably check `name in activityMap` before doing this, and if that condition evaluates to false, throw an error with a clear message. Since this is an API that web pages will be using, we'll want to avoid cryptic exceptions. @@ +61,5 @@ > +function mapActivity(name, data) { > + let mappedData = activityMap[name]; > + > + return { > + webactivity: true, What is this property used for? @@ +71,5 @@ > +} > + > +function processType(type) { > + // Android only supports one MIME type per intent. > + return type instanceof Array ? type[0] : type; What does FxOS do if multiple MIME types are given? We should probably do our best to make sure behavior matches between platforms as much as possible so apps don't exhibit different behaviors on different platforms. In this case, we might need to iterate through the array, firing a separate request for each, until Android gives us a result back.

From the Android/synthesized APK pov: We are able to generate IntentFilters from WebActivities declared in the app's manifest (we don't yet, but it's relatively easy). Because it is the synthesized APKs that are catching the Android intents, we won't need to provide an Activity chooser of our own – we just use Android's. This is exactly as intended. However, a couple of things bother me at the moment: * we should provide a standard set of WebActivities, in the same way Android does. Lessons from the Android world would suggest that the community hasn't been very good at adopting community lead standards (see the http://www.openintents.org/intentstable for a repo of intents that no-one uses). My first port of call to find these out would be finding out what use Firefox OS makes of them. WesJ's link is a good start. * WebActivities kinda sorta work like the Intent system in Android, but we'll be unable to support anything but things that invoke a UI. i.e. Android Activities only. AFAIK, headless Gecko doesn't exist yet, and until it does, and is well understood, it's going to be difficult respond to things like ContentResolvers and Broadcast intents. Think: no headless access to data provided by other apps, no responding to phone events, e.g. geo-fencing. The Android Intent docs do provide a bunch of constants, but many of them are not Activity actions. * The way we've talked about implementing the synth APK WebActivities mostly depends on the developer registering them in the manifest.json. This does not take into consideration: a) programmatic registration, b) registration by web /sites/. This second point is most difficult, but I don't think this is the right bug to say why. * Calling out to WebActivities should not preclude Android Intents: i.e. the mapping should be bi-directional. * Whatever mappings we chose should be kept in sync with the APK Factory used to generate APKs from webapp manifests. * Especially with this issue, we have an enormous impedance mismatches going on: FirefoxOS/Android. Personally, I think the intents system and putting it at the heart of the OS is one of the few things that Android gets completely right. More specific feedback for this patch: * I see no reason why the mapping needs to be in Javascript.

(In reply to James Hugman [:jhugman] [@jhugman] from comment #16) > We are able to generate IntentFilters from WebActivities declared in the > app's manifest (we don't yet, but it's relatively easy). Is there code anywhere for where this mapping will happen? If not, is this something that should be worked on in this bug, or are you planning on doing this? I think we could at least propose an idl for this mapping service. > * we should provide a standard set of WebActivities, in the same way > Android does. Lessons from the Android world would suggest that the > community hasn't been very good at adopting community lead standards (see > the http://www.openintents.org/intentstable for a repo of intents that > no-one uses). My first port of call to find these out would be finding out > what use Firefox OS makes of them. WesJ's link is a good start. Is there a bug/etherpad/wiki to determine these? > * Calling out to WebActivities should not preclude Android Intents: i.e. > the mapping should be bi-directional. Great, this is what I assumed. > * Whatever mappings we chose should be kept in sync with the APK Factory > used to generate APKs from webapp manifests. Ideally, we should be able to use the same code to do this, so nothing gets out of sync. > * I see no reason why the mapping needs to be in Javascript. I don't either, but whatever we build should be easily used by both Fennec and the APK synthesizer.

(In reply to Josh Dover from comment #17) > (In reply to James Hugman [:jhugman] [@jhugman] from comment #16) > > We are able to generate IntentFilters from WebActivities declared in the > > app's manifest (we don't yet, but it's relatively easy). > > Is there code anywhere for where this mapping will happen? If not, is this > something that should be worked on in this bug, or are you planning on doing > this? I think we could at least propose an idl for this mapping service. No. The IntentFilters can either be registered programmatically in Java (but would need to be done in the synthesized APK), or via the APK's AndroidManifest.xml, in, ahem, XML. That's JS => XML, Java, defining the /filter/. > > My first port of call to find these out would be finding out > > what use Firefox OS makes of them. WesJ's link is a good start. > > Is there a bug/etherpad/wiki to determine these? Not that I know of. Would suggest a Wiki page. > > * Whatever mappings we chose should be kept in sync with the APK Factory > > used to generate APKs from webapp manifests. > > Ideally, we should be able to use the same code to do this, so nothing gets > out of sync. Yes, that would be nice. Conceptually this is: /Intent/ => WebActivityHandler > > > * I see no reason why the mapping needs to be in Javascript. > > I don't either, but whatever we build should be easily used by both Fennec > and the APK synthesizer. From an implementation details p.o.v. I'd be wary of premature code sharing – it isn't clear to me that it's possible in the general case: if we see that the two systems are both systems of locks and keys, then we are mapping lock1->lock2, key2->lock1, key1->key2 i.e. * in APK-Factory we're mapping WebActivity filters (lock1) to IntentFilters (lock2). * in SynthesizedAPK, we're mapping WebActivity filters to IntentFilters again, but this time we must use Java. * for incoming intents, in Fennec, we're mapping Intents (key2) that match IntentFilters (lock2) to WebActivity filters (lock1). It's not immediately obvious that Android can tell us which IntentFilter a received Intent matched. * for outgoing webactivities, in Fennec, we're mapping MozActivity (key1) to Intent (key2) It should be noted that none of the behaviour described in this comment is implemented.

Comment on attachment 8361405 [details] [diff] [review] Simplified Implementation using Android Intent chooser Review of attachment 8361405 [details] [diff] [review]: ----------------------------------------------------------------- Talked to josh about this, and I agree that trying to share this is probably premature. I think we're just going to do the initial mapping here. We should try to make it easy to extend/change. A wiki page to detail things would be nice as well. You can just start one somewhere inside https://wiki.mozilla.org/Mobile/Fennec/ . Once we got some basics checked in and the WebApps/Android team are both happy, we can move it over to somewhere near https://developer.mozilla.org/en-US/docs/Web/API/MozActivity.

A mappings table has been added to the project wiki page at [1]. This will serve as a reference spec and place for discussion on: 1) which actions should be supported 2) how they (and their associated data) should be translated to Android [1] https://wiki.mozilla.org/Mobile/Projects/API:_Web_activities#MozActivity_Mapping

Fixed bug in JSON parsing

Comment on attachment 8393095 [details] [diff] [review] 01 - Move intent handling to IntentHelper Review of attachment 8393095 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/IntentHelper.java @@ +23,5 @@ > + > +public final class IntentHelper implements GeckoEventListener { > + private static final String LOGTAG = "GeckoIntentHelper"; > + private static final String[] EVENTS = > + { "Intent:GetHandlers", "Intent:Open", "Intent:OpenForResult" }; Put these on separate lines. r.e.: EVENTS = { EVENT1, EVENT2 } @@ +57,5 @@ > + } > + > + @Override > + public void handleMessage(String event, JSONObject message) { > + Log.d(LOGTAG, "recieved message: " + message.toString()); Remove logging. @@ +72,5 @@ > + } > + } > + > + private void getHandlers(JSONObject message) throws JSONException { > + final Intent intent = GeckoAppShell.getOpenURIIntent(activity, message.optString("url"), I like this IntentHelper class. Hopefully we can move these methods in here and cleanup GeckoAppShell some more. Separate bug :) Maybe file and assign to yourself. @@ +75,5 @@ > + private void getHandlers(JSONObject message) throws JSONException { > + final Intent intent = GeckoAppShell.getOpenURIIntent(activity, message.optString("url"), > + message.optString("mime"), message.optString("action"), message.optString("title")); > + final String[] handlers = GeckoAppShell.getHandlersForIntent(intent); > + final List<String> appList = Arrays.asList(handlers); Add a blank line here. I hate that we have so many variables here. Maybe this would be nicer if we put these in one line. final List<String> appList = Arrays.asList(GeckoAppShell.getHandlersForIntent(intent)); @@ +84,5 @@ > + > + private void open(JSONObject message) throws JSONException { > + GeckoAppShell.openUriExternal(message.optString("url"), > + message.optString("mime"), message.optString("packageName"), > + message.optString("className"), message.optString("action"), message.optString("title")); This is hard to read. If we have to break lines, lets just put each argument on its own line. @@ +89,5 @@ > + } > + > + private void openForResult(final JSONObject message) throws JSONException { > + final Intent intent = GeckoAppShell.getOpenURIIntent(activity, message.optString("url"), > + message.optString("mime"), message.optString("action"), message.optString("title")); Same here @@ +97,5 @@ > + > + final JSONObject originalMessage = message; > + ActivityHandlerHelper.startIntentForActivity(activity, > + intent, > + new ActivityResultHandler() { Ideally I think we might pull this into an inner class and then call ActivityHandlerHelper.startIntentForActivity(activity, intent, new Handler(message));

Comment on attachment 8393096 [details] [diff] [review] 02 - Send web activities to Java Review of attachment 8393096 [details] [diff] [review]: ----------------------------------------------------------------- I think we'll probably need to rework this a bit. Lets ping the people who can help tell us how. Fabrice, are they you, or do you know who can help us here? ::: dom/activities/src/ActivitiesService.jsm @@ +208,2 @@ > if (aResults.options.length === 0) { > + let nativeAppService = Cc["@mozilla.org/native-application-service;1"].createInstance(Ci.nsINativeAppService); I think we need to move this up into the "find" function above: http://mxr.mozilla.org/mozilla-central/source/dom/activities/src/ActivitiesService.jsm#124 but that's a bit tricky since its using an indexedDB herlper to send results back. However, with the new Synth APK's stuff, we might not need to use the IndexedDB stuff at all. i.e. Any webapps that handle intents will register for them in their APK's, so this whole thing is now un-necessary. We just replace the ActivitiesDB on Android with our own implementation and not use the WebApp DB at all... If this was XPCom that wouldn't be too hard. Its not thugh. We might need to pull the ActivitesDB into its own file that we can override on Android. Pinging fabrice for advice here. ::: mobile/android/components/NativeAppService.js @@ +6,5 @@ > +const Cu = Components.utils; > + > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > +Cu.import("resource://gre/modules/Services.jsm"); > +Cu.import("resource://gre/modules/HelperApps.jsm"); You're not using this anymore. @@ +10,5 @@ > +Cu.import("resource://gre/modules/HelperApps.jsm"); > + > +let _callbacks = {}; > + > +function NativeAppService() { } If we can override ActivitiesDB, I'd like to remove this file and do everything there instead. @@ +45,5 @@ > + } > + }, > + > + _sendMessage: function(data) { > + return Services.androidBridge.handleGeckoMessage(JSON.stringify(data)); I'm pretty biased, but use Messaging.jsm here :)

Comment on attachment 8393114 [details] [diff] [review] 03 - Map and launch web activities as Android intents Review of attachment 8393114 [details] [diff] [review]: ----------------------------------------------------------------- This seems usable. I'd like to try and clean up the mapping a little bit so that, at least for basic use cases, its really simple to add new mappings. ::: mobile/android/base/IntentHelper.java @@ +128,5 @@ > + response.put("extras", bundleToJSON(data.getExtras())); > + } > + > + response.put("resultCode", resultCode); > + EventDispatcher.sendResponse(originalMessage, response); I forgot about these returns in my previous review. Getting the result back may mean that we can't just override the database... ::: mobile/android/base/util/WebActivityMapper.java @@ +25,5 @@ > + this.name = name; > + this.mapping = mapping; > + } > + > + public static WebActivityMapping fromName(String name) { The enum isn't really that helpful if you do things this way. You could remove the name and then use: WebActivityMapping mapping = Enum.valueOf(Mapping.class, name.toUpperCase()).mapping; or a HashTable of some sort would probably work just as well: HashMap<String, WebActivityMapping> map = new HashMap<String, WebActivityMapping>() {{ put("dial", new DialMapping()); put("send", new SendMapping()); }}; @@ +49,5 @@ > + return null; > + } > + > + public void putExtras(JSONObject data, Intent intent) throws JSONException { > + return; I'm debating the merits of this over a more basic getIntent(JSONObject) class, but I'm willing to try this out I think... @@ +78,5 @@ > + > + private static class DialMapping extends WebActivityMapping { > + @Override > + public String getAction() { > + return "android.intent.action.DIAL"; Intent.ACTION_DIAL @@ +87,5 @@ > + return "tel:" + data.getString("number"); > + } > + } > + > + private static class SendMapping extends WebActivityMapping { I almost wonder if we're better to abstract this implementation a bit. i.e. the Constructor could take the action to return, and maybe an array of extras... Just trying to figure out if there's a base implementation we can reuse for multiple web activity types. @@ +90,5 @@ > + > + private static class SendMapping extends WebActivityMapping { > + @Override > + public String getAction() { > + return "android.intent.action.SEND"; Intent.ACTION_SEND @@ +100,5 @@ > + } > + > + @Override > + public void putExtras(JSONObject data, Intent intent) throws JSONException { > + optPutExtra("text", "android.intent.extra.TEXT", data, intent); Intent.EXTRA_TEXT, etc.

(In reply to Wesley Johnston (:wesj) from comment #26) > Comment on attachment 8393096 [details] [diff] [review] > 02 - Send web activities to Java > > Review of attachment 8393096 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think we'll probably need to rework this a bit. Lets ping the people who > can help tell us how. Fabrice, are they you, or do you know who can help us > here? > > ::: dom/activities/src/ActivitiesService.jsm > @@ +208,2 @@ > > if (aResults.options.length === 0) { > > + let nativeAppService = Cc["@mozilla.org/native-application-service;1"].createInstance(Ci.nsINativeAppService); > > I think we need to move this up into the "find" function above: > http://mxr.mozilla.org/mozilla-central/source/dom/activities/src/ > ActivitiesService.jsm#124 > > but that's a bit tricky since its using an indexedDB helper to send results > back. However, with the new Synth APK's stuff, we might not need to use the > IndexedDB stuff at all. i.e. Any webapps that handle intents will register > for them in their APK's, so this whole thing is now un-necessary. We just > replace the ActivitiesDB on Android with our own implementation and not use > the WebApp DB at all... > > If this was XPCom that wouldn't be too hard. Its not thugh. We might need to > pull the ActivitesDB into its own file that we can override on Android. > Pinging fabrice for advice here. > Still need advice on this part. It seems to me that it would make more sense to override Activities so that all startActivity() did was send the MozActivity options to Java. > > @@ +45,5 @@ > > + } > > + }, > > + > > + _sendMessage: function(data) { > > + return Services.androidBridge.handleGeckoMessage(JSON.stringify(data)); > > I'm pretty biased, but use Messaging.jsm here :) This greatly simplified things :)

(In reply to Wesley Johnston (:wesj) from comment #27) > Comment on attachment 8393114 [details] [diff] [review] > 03 - Map and launch web activities as Android intents > > @@ +49,5 @@ > > + return null; > > + } > > + > > + public void putExtras(JSONObject data, Intent intent) throws JSONException { > > + return; > > I'm debating the merits of this over a more basic getIntent(JSONObject) > class, but I'm willing to try this out I think... I looked at the different web activities used by Gaia and found that 'type' and 'uri'/'url' were used quite often, so I made a BaseMapping class to handle these situations which I used to add more mappings really easily. Having base classes like this was the motivation for doing a less generic method like getIntent(JSONObject). Let me know what you think!

Comment on attachment 8395820 [details] [diff] [review] 01 - Move intent handling to IntentHelper Review of attachment 8395820 [details] [diff] [review]: ----------------------------------------------------------------- Nice :)

Comment on attachment 8395815 [details] [diff] [review] 02 - Send web activities to Java Review of attachment 8395815 [details] [diff] [review]: ----------------------------------------------------------------- We need to get some tests written for all of this code, since its getting quite complex. There's a meeting tomorrow for some JS test stuff. This will be good as a use case... ::: dom/activities/src/ActivitiesService.jsm @@ +208,3 @@ > if (aResults.options.length === 0) { > + let nativeAppService = Cc["@mozilla.org/native-application-service;1"].createInstance(Ci.nsINativeAppService); > + if (nativeAppService) { We can try leaving this in. Lets just move it down below this successCb, before we even do the match: http://mxr.mozilla.org/mozilla-central/source/dom/activities/src/ActivitiesService.jsm#275 If the NativeService is defined, we can jump the entire line here... @@ +208,5 @@ > if (aResults.options.length === 0) { > + let nativeAppService = Cc["@mozilla.org/native-application-service;1"].createInstance(Ci.nsINativeAppService); > + if (nativeAppService) { > + nativeAppService.launchNativeAppPicker(aMsg.options, { > + handleEvent: function(result) { This result will have a bunch of things in it that content probably shouldn't know about. We should clean it up, or maybe it should just contain data that we'll send to the page in a separate object. i.e Java returns: { guid: something, type: "Prompt:Return", data: results } and you'd just pass the data back to the page.

Moved call to NativeAppService. Need JS testing.

Comment on attachment 8398259 [details] [diff] [review] 02 - Send web activities to Java Review of attachment 8398259 [details] [diff] [review]: ----------------------------------------------------------------- I'm changing my mind a bit (sorry), but lets get fabrice's feedback before we do anything. ::: dom/activities/src/ActivitiesService.jsm @@ +276,5 @@ > }; > > + // If there is a native app service available, skip the ActivitiesDB, delegating to the platform. > + let nativeAppService = > + Cc["@mozilla.org/native-application-service;1"].createInstance(Ci.nsINativeAppService); Sorry to go back and forth here. I think we should wait to get some feedback from fabrice, but after thinking about this a bit, I think we should do this inside the sucessCb, and pass the results we found there to the nativeAppService. I want to make this a bit general so that it could work on other platforms. Those platforms might not have something like the synthetic APK, so this would let them choose to use or ignore the webapps that were found. Right now they basically have to ignore them (or duplicate this database code...) ::: mobile/android/app/mobile.js @@ +837,5 @@ > // which is a test server that always reports all apps as having updates. > pref("browser.webapps.updateCheckUrl", "https://controller.apk.firefox.com/app_updates"); > > +// System messages needed for Web Activity support > +pref("dom.sysmsg.enabled", true); We'll probably lock this to non-release builds initially.

Comment on attachment 8398259 [details] [diff] [review] 02 - Send web activities to Java Review of attachment 8398259 [details] [diff] [review]: ----------------------------------------------------------------- I need to understand better the android specifics! ::: dom/activities/interfaces/nsINativeAppService.idl @@ +3,5 @@ > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +#include "nsISupports.idl" > + > +interface nsIDOMMozActivityOptions; this has been removed when switching all the activities code to use webidl. Use jsval instead. @@ +5,5 @@ > +#include "nsISupports.idl" > + > +interface nsIDOMMozActivityOptions; > +interface nsIActivityUIGlueCallback; > + this file needs comments. What is it for? ::: dom/activities/src/Activities.manifest @@ +14,5 @@ > contract @mozilla.org/dom/activities/options;1 {ee983dbb-d5ea-4c5b-be98-10a13cac9f9d} > + > +component {d5691dc4-cb8b-4f57-bf3f-31215fd5a3ca} NativeAppService.js > +contract @mozilla.org/native-application-service;1 {d5691dc4-cb8b-4f57-bf3f-31215fd5a3ca} > +contract @mozilla.org/dom/activities/ui-glue;1 {d5691dc4-cb8b-4f57-bf3f-31215fd5a3ca} this component is not is this directory. Please remove that. ::: dom/activities/src/ActivitiesService.jsm @@ +287,5 @@ > + }); > + } > + }); > + } > + It's not 100% clear to me how all this works, but by doing that you remove all chances to use activities registered from manifests. Is that what we want on Android? Are you converting them to native intents while building the apks? ::: mobile/android/components/NativeAppService.js @@ +6,5 @@ > +const Ci = Components.interfaces; > +const Cu = Components.utils; > + > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > +Cu.import("resource://gre/modules/Services.jsm"); unused.

Some background on how this works with synth-APKs on Android: All open web apps are packaged up as native APKs (installable Android apps) that have all of their web activities translated into Android intents (this work is separate from this bug). Because of this, we do not use the ActivitiesDB that B2G uses to find eligible apps, so all of our inter-app communications happen via Android's native intent system. This also allows webapps to talk to regular native Android apps as well (yay!). What's going on in these patches: My strategy with the modifications to nsIActivitiesUIGlue is to allow an implementer of the interface to have the option to handle the activity by providing all of MozActivity options (rather than just the name) to the implementer as well as a callback to deliver the results (if applicable). This is great for Android and could be applicable to Windows too [1] in the future. Of course for B2G we can safely ignore this new callback. I tried to comment as much as possible, but definitely point out any areas where it's unclear what I am doing. [1] Windows app contracts and extensions - http://msdn.microsoft.com/en-us/library/windows/apps/hh464906.aspx

Comment on attachment 8403689 [details] [diff] [review] 03 - Add onresult callback to nsIActivityUIGlue Review of attachment 8403689 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/activities/interfaces/nsIActivityUIGlue.idl @@ +38,5 @@ > * @param activities A json blob which is an array of { "title":"...", "icon":"..." }. > + * @param onchoose The callback to send the index of the choosen activity. > + Send '-1' if no choice is made. Send 'undefined' if activities.length is 0 > + and not using alternate activity system. > + * @param onresult The callback to send the result of an externally launched activity. could we keep a single callback, with a signature like: handleEvent(in long resultType, in jsval result) where resultType could be nsIActivityUIGlueChooseCallback.WEBAPPS_ACTIVITY or nsIActivityUIGlueChooseCallback.NATIVE_ACTIVITY

Comment on attachment 8403690 [details] [diff] [review] 04 - Pass web activities to Java Review of attachment 8403690 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/components/ActivitiesGlue.js @@ +8,5 @@ > +const Ci = Components.interfaces; > +const Cu = Components.utils; > + > +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > +Cu.import("resource://gre/modules/Services.jsm"); unused.

(In reply to Fabrice Desré [:fabrice] from comment #40) > could we keep a single callback, with a signature like: > > handleEvent(in long resultType, in jsval result) > > where resultType could be nsIActivityUIGlueChooseCallback.WEBAPPS_ACTIVITY > or nsIActivityUIGlueChooseCallback.NATIVE_ACTIVITY Done, let me know if the comments aren't clear enough.

Made some small modifications to the building of the result JSON to send back to JS: - If there are no extras, that key will not be present rather than having an empty JS object - Added the uri to the result (especially important for intents like ACTION_PICK)

Comment on attachment 8403688 [details] [diff] [review] 02 - Pass activity options instead of name through nsIActivityUIGlue This should be ready for review without any changes.

Comment on attachment 8405048 [details] [diff] [review] 03 - Allow nsIActivityUIGlueCallback to handle native and web activities Review of attachment 8405048 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/components/ActivitiesGlue.js @@ +70,5 @@ > }); > Services.tm.currentThread.dispatch(this, Ci.nsIEventTarget.DISPATCH_NORMAL); > }, > > + classID: Components.ID("{de25076e-f986-4a6d-bf5a-cda27b9e3ad8}"), no need to change the classID ::: b2g/components/B2GComponents.manifest @@ +21,5 @@ > category xpcom-directory-providers browser-directory-provider @mozilla.org/browser/directory-provider;1 > > # ActivitiesGlue.js > +component {de25076e-f986-4a6d-bf5a-cda27b9e3ad8} ActivitiesGlue.js > +contract @mozilla.org/dom/activities/ui-glue;1 {de25076e-f986-4a6d-bf5a-cda27b9e3ad8} no need to change the classID

Comment on attachment 8405049 [details] [diff] [review] 04 - Pass web activities to Java Review of attachment 8405049 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, but I know next to nothing about the android code, so wesj should also take a look.

Comment on attachment 8405049 [details] [diff] [review] 04 - Pass web activities to Java Review of attachment 8405049 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/app/mobile.js @@ +837,5 @@ > // To test updates, set this to http://apk-update-checker.paas.allizom.org, > // which is a test server that always reports all apps as having updates. > pref("browser.webapps.updateCheckUrl", "https://controller.apk.firefox.com/app_updates"); > > +pref("dom.sysmsg.enabled", true); Pull this out of the synth-apks ifdef and instead lets wrap it in a #ifdef NIGHTLY_BUILD. I think we'll eventually want to ensure that (at least for the web, maybe not apps), this shows a prompt to the user. ::: mobile/android/components/ActivitiesGlue.js @@ +17,5 @@ > + QueryInterface: XPCOMUtils.generateQI([Ci.nsIActivityUIGlue]), > + classID: Components.ID("{e4deb5f6-d5e3-4fce-bc53-901dd9951c48}"), > + > + // Ignore aActivities results on Android, go straight to Android intents. > + chooseActivity: function ap_chooseActivity(aOptions, aActivities, aCallback) { Nice. Good working finding this :) @@ +22,5 @@ > + sendMessageToJava({ > + type: "WebActivity:Open", > + activity: { name: aOptions.name, data: aOptions.data } > + }, (result) => { > + aCallback.handleEvent(Ci.nsIActivityUIGlueCallback.NATIVE_ACTIVITY, result); This actually could be the result from a web activity at some point, but we'll deal with that when its possible :)

Comment on attachment 8405051 [details] [diff] [review] 05 - Map and launch web activities as Android intents Review of attachment 8405051 [details] [diff] [review]: ----------------------------------------------------------------- Lets land this :) ::: mobile/android/base/util/WebActivityMapper.java @@ +133,5 @@ > + @Override > + public String getMime(JSONObject data) { > + // MozActivity adds a type 'url' here, we don't want to set the MIME to 'url'. > + String type = data.optString("type", null); > + if (TextUtils.equals(type, "url") || TextUtils.equals(type, "uri")) { "url".equals(type) will also get you around any null problems.

Before landing, I am running a try build to make sure that nothing in B2G was broken by these changes: https://tbpl.mozilla.org/?tree=Try&rev=8371bd506269

Rebased onto fx-team

Rebased onto fx-team

Rebased onto fx-team

Rebased onto fx-team

Rebased onto fx-team

https://hg.mozilla.org/integration/fx-team/rev/33bcd13d52f5 https://hg.mozilla.org/integration/fx-team/rev/f8c51b2092ca https://hg.mozilla.org/integration/fx-team/rev/2ada01517980 https://hg.mozilla.org/integration/fx-team/rev/ec54368243da https://hg.mozilla.org/integration/fx-team/rev/46b4833e84c3

sorry had to backed this out for a test failure like https://tbpl.mozilla.org/php/getParsedLog.php?id=37826592&tree=Fx-Team

Ongoing discussion on dev.platform: https://groups.google.com/forum/#!topic/mozilla.dev.platform/xy0lPhgQCCo

rebased onto fx-team

rebased onto fx-team

rebased onto fx-team

rebased onto fx-team

This only exposes MozActivity in webapps running on a nightly build.

Comment on attachment 8413039 [details] [diff] [review] 06 - Pref off MozActivity for only WebappRT Review of attachment 8413039 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/WebappRT.js @@ +41,5 @@ > pref("dom.mozTCPSocket.enabled", true), > // Don't check for updates in webapp processes to avoid duplicate notifications. > pref("browser.webapps.checkForUpdates", 0), > + > + #ifdef NIGHTLY_BUILD No need for the nightly build stuff even. For webapps, we'll let this ride the trains.

Passed on try, marking for checkin! https://tbpl.mozilla.org/?tree=Try&rev=4343e2943d21

https://hg.mozilla.org/integration/fx-team/rev/95742dfbee49 https://hg.mozilla.org/integration/fx-team/rev/c15e4803607d https://hg.mozilla.org/integration/fx-team/rev/025898d5577c https://hg.mozilla.org/integration/fx-team/rev/fac267a41e5c https://hg.mozilla.org/integration/fx-team/rev/774890574009 https://hg.mozilla.org/integration/fx-team/rev/8d7327159983

https://hg.mozilla.org/mozilla-central/rev/95742dfbee49 https://hg.mozilla.org/mozilla-central/rev/c15e4803607d https://hg.mozilla.org/mozilla-central/rev/025898d5577c https://hg.mozilla.org/mozilla-central/rev/fac267a41e5c https://hg.mozilla.org/mozilla-central/rev/774890574009 https://hg.mozilla.org/mozilla-central/rev/8d7327159983

Dev docs added: https://developer.mozilla.org/en-US/docs/WebAPI/Web_Activities#Firefox_for_Android_activities