609043 - Add support for Open Web Apps

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[[:fabrice] Fabrice Desré]

Provide support for Open Web Apps (https://wiki.mozilla.org/Labs/Apps), to allow seamless installation from within web pages.

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: m-c patch

Adds a mozApp property to window.navigator. The real implementation is delegated to an XPCOM component.

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: m-b patch

Here we hook up Open Webapp support with the existing platform-dependent components.

We use the following parts of the manifest :
- name
- launch uri
- icon
- capabilities
and support l10n for the name.

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 487622 [details] [diff] [review]
m-c patch



>+++ b/dom/base/nsDOMClassInfo.cpp

You will also want to add classinfo for the class that implements xxx.  Do a grep in this file for GeoGeolocation to see what I mean.  Also add these to the nsDOMClassInfoClasses.h file.




>+
>+NS_IMETHODIMP nsNavigator::GetMozApp(nsIDOMOpenWebapp **aRetVal)
>+{
>+  NS_ENSURE_ARG_POINTER(aRetVal);
>+  *aRetVal = nsnull;
>+  
>+  if (mMozApp) {
>+    NS_ADDREF(*aRetVal = mMozApp);
>+    return NS_OK;
>+  }
>+  
>+  nsresult rv;
>+  mMozApp = do_CreateInstance("@mozilla.org/openwebapp;1", &rv);
>+  if (NS_FAILED(rv)) {
>+    NS_WARNING("No implementation of @mozilla.org/openwebapp;1 found");
>+    return NS_ERROR_NOT_IMPLEMENTED;
>+  }

Drop the rv.  You really aren't using it.  Instead just test for a null mMozApp.


>+#
>+# Contributor(s):
>+#  Fabrice Desré <[e-mail]>
>+#

Is it just my dumb editor, or does your last name look wrong?


>+
>+[scriptable, function, uuid(9032f9d8-0fc2-4b6d-ad5b-a7c371d197b9)]
>+interface nsIOpenWebappCallback : nsISupports {
>+    void completed(in jsval manifest);    
>+};
>+
>+[scriptable, uuid(691d89a7-89b2-459b-ae7e-6bb9668a4b30)]
>+interface nsIDOMOpenWebapp : nsISupports {
>+    void install(in jsval manifest, in nsIOpenWebappCallback onSuccess, in nsIOpenWebappCallback onError);
>+    boolean isInstalled(in jsval manifest);
>+};
>+
>+/**
>+ * Property that extends the navigator object.
>+ */
>+[scriptable, uuid(8959e699-8c6c-451c-a842-581a12c8b35e)]
>+interface nsIDOMNavigatorOpenWebapp : nsISupports
>+{
>+  readonly attribute nsIDOMOpenWebapp mozApp;
>+};


Some people will complain about these 3 interfaces being in the same file.  Not really sure why - maybe some latent java style fetish.

Given that there is no spec, i'd comment wtf these API are suppose to do.

Using nsIOpenWebappCallback is probably wrong.  A better way is to use nsIDOMEventListener.  How much wiggle room do you have with this API.  Maybe I should provide feedback else where?

Otherwise looks fine.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Fabrice - do you have an updated JS-only patch for this?

Comment 5

[Doug Turner (:dougt)]

Also - it looks like the spec has changed from when Fabrice started to implement this.

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to comment #5)
> Also - it looks like the spec has changed from when Fabrice started to
> implement this.

Yes, I'm currently implementing https://developer.mozilla.org/en/OpenWebApps/The_JavaScript_API in JS only, using a sandbox to inject the |apps| object into window.navigator

Comment 7

[[:fabrice] Fabrice Desré]

Attachment: JS only WIP

This patch implements the installation API from https://developer.mozilla.org/en/OpenWebApps/The_JavaScript_API

Each application manifest is saved under $PROFILE/webapps/$APP_ID/manifest.js
The full list of applications is saved to $PROFILE/webapps/webapps.json

It also creates a manifest for webapps installed from the site menu.

Todo:
- implement part of the management API
- fix the "Add shortcut" android feature which is now broken.

Comment 8

[[:fabrice] Fabrice Desré]

A good test page is : https://apps.mozillalabs.com/appdir/

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 549516 [details] [diff] [review]
JS only WIP


>diff --git a/mobile/chrome/content/common-ui.js b/mobile/chrome/content/common-ui.js
> var WebappsUI = {
>   _dialog: null,
>-  _manifest: null,
>   _perms: [],
>-  
>+  _openwebapps: false,
>+  _application: null,
>+
>+  init: function() {
>+    Cu.import("resource:///modules/openwebapps.jsm");
>+    messageManager.addMessageListener("OpenWebapps:Install", this);
>+    messageManager.addMessageListener("OpenWebapps:GetInstalledBy", this);
>+    messageManager.addMessageListener("OpenWebapps:AmInstalled", this);
>+  },
>+
>+  // converts a manifest to an application as expected by openwebapps.install()
>+  convertManifest: function(aData) {
>+    let aManifest = JSON.parse(aData.manifest);
>+    let app = {};
>+
>+    app.manifest = aManifest;
>+    app.installData = aData.installData;
>+    app.storeURI = aData.storeURI;
>+    app.manifestURI = aData.manifestURI;

How about removing aManifest and doing:

      app.manifest = JSON.parse(aData.manifest);

if you really want to keep the local varibale, change "aManifest" to "manifest"


Also, we typically do this now:

      let app = {
        manifest: aManifest,
        installData: aData.installData,
        storeURI: aData.storeURI,
        manifestURI: aData.manifestURI
      };

You use aData.* in the code below, but you could use app.*

>+    let localeRoot;
>+
>+    if (aManifest.locales)

Remove the blank line

>+    if (icon)
>+      app.iconURI = icon;
>+    let root = baseURI.resolve("/").toString();

Add a blank line before "let root..."

>+    app.appURI = aManifest.launch_path ? baseURI.resolve(aManifest.launch_path) : root.substring(0, root.length - 1);

lanuch_path? launchPath seems more JavaScript-ish

>+    app.capabilities = [];
>+    app.successCallback = aData.successCallback;
>+    app.errorCallback = aData.errorCallback;

Couldn't you have added these above?

>+  receiveMessage: function(aMessage) {
>+    let browser = getBrowser();

      let browser = aMessage.target;

Just in case the selected browser did not send the request

>+    switch(aMessage.name) {
>+      case "OpenWebapps:Install":
>+        WebappsUI._openwebapps = true;
>+        WebappsUI.show(WebappsUI.convertManifest(aMessage.json));
>+        break;
>+      case "OpenWebapps:GetInstalledBy":
>+        let apps = OpenWebapps.getInstalledBy(aMessage.json.storeURI);
>+        browser.messageManager.sendAsyncMessage("OpenWebapps:GetInstalledBy:Return",
>+            { apps: apps, successCallback: aMessage.json.successCallback, errorCallback: aMessage.json.errorCallback });

How can you pass the callbacks across JSON? The JSON stringify will destroy and functions. I think you need to hold the functions on the content side.

UPDATE: I see that you do hold them in content and these are IDs. I would consider bundling all callbacks together into a single JS object, not stored individually. Here, you would have something like:

              { apps: apps, callbackID: aMessage.json.callbackID });

The callbackID would be used in content to retrieve a JS object with both success and error callbacks. See below for the content changes.

>     BrowserUI.popPopup(this);
>+    if (this._openwebapps) {
>+      let browser = getBrowser();
>+      browser.messageManager.sendAsyncMessage("OpenWebapps:InstallAborted", { successCallback: this._application.successCallback, errorCallback: this._application.errorCallback });

* Blank line after BrowserUI.popPopup
* Shouldn't you reset _openwebapps = false here too?

>+    this.install();
>+    let browser = getBrowser();
>+    if (willSendMessage)
>+      browser.messageManager.sendAsyncMessage("OpenWebapps:InstallDone", { successCallback: this._application.successCallback, errorCallback: this._application.errorCallback });

* Blank line after this.install()
* I don't like using getBrowser here. Maybe _openwebapps should point to the original aMessage.target and then be used here too.

>diff --git a/mobile/chrome/content/content.js b/mobile/chrome/content/content.js

>+let OpenWebapps = {

>+  getCallbackId: function(aCallback) {
>+    let id = "id" + Math.random();
>+    this._callbacks[id] = aCallback;
>+    return id;
>+  },
>+  
>+  getCallback: function(aId) {
>+    return this._callbacks[aId];
>+  },
>+
>+  removeCallback: function(aId) {
>+    delete this._callbacks[aId];
>+  },
>+
>+  install: function(aStoreURI, aManifestURI, aInstallData, aSuccessCallback, aErrorCallback) {
>+    let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci.nsIXMLHttpRequest);
>+    xhr.open("GET", aManifestURI, true);
>+    xhr.onload = function() {
>+      if (xhr.status == 200) {
>+        try {
>+          let manifest = JSON.parse(xhr.responseText);
>+          if (!OpenWebapps.checkManifest(manifest)) {
>+            if (aErrorCallback)
>+              aErrorCallback({ code: "invalidManifest", message: "Invalid manifest" });
>+          }
>+          else {

} else {

>+            sendAsyncMessage("OpenWebapps:Install", { storeURI: aStoreURI.href, manifestURI: aManifestURI, manifest: xhr.responseText,
>+                             installData: aInstallData, successCallback:  OpenWebapps.getCallbackId(aSuccessCallback), errorCallback: OpenWebapps.getCallbackId(aErrorCallback) });

Since each meaagse knows how many callbacks exist, let's simplify this and do:

            sendAsyncMessage("OpenWebapps:Install", { storeURI: aStoreURI.href, manifestURI: aManifestURI, manifest: xhr.responseText,
                             installData: aInstallData, callbackID: OpenWebapps.getCallbackId({ success: aSuccessCallback, error: aErrorCallback }) });

>+        }
>+        catch(e) {

} catch {

>+      }
>+      else if (aErrorCallback) {

} else if (...) {

>+      }
>+      
>+    }
>+    xhr.onerror = function() {
>+      if (aErrorCallback)
>+        aErrorCallback({ code: "networkError", message: "Unable to retrieve manifest" });
>+    }
>+    xhr.send(null);

* Remove blank line between } }
* Add blank lines between xhr.*

>+  amInstalled: function(aAppURI, aSuccessCallback, aErrorCallback) {
>+    sendAsyncMessage("OpenWebapps:AmInstalled", { appURI: aAppURI, successCallback:  OpenWebapps.getCallbackId(aSuccessCallback), errorCallback: OpenWebapps.getCallbackId(aErrorCallback) });

    sendAsyncMessage("OpenWebapps:AmInstalled", { appURI: aAppURI, callbackID: OpenWebapps.getCallbackId({ success: aSuccessCallback, error: aErrorCallback }) });

>+  getInstalledBy: function(aStoreURI, aSuccessCallback, aErrorCallback) {
>+    sendAsyncMessage("OpenWebapps:GetInstalledBy", { storeURI: aStoreURI.href, successCallback:  OpenWebapps.getCallbackId(aSuccessCallback), errorCallback: OpenWebapps.getCallbackId(aErrorCallback) });

    sendAsyncMessage("OpenWebapps:GetInstalledBy", { storeURI: aStoreURI.href, callbackID: OpenWebapps.getCallbackId({ success: aSuccessCallback, error: aErrorCallback }) });

>+  receiveMessage: function(aMessage) {

Update for new callback style

>+  init: function() {

>+    let observer = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
>+    observer.addObserver(this, "content-document-global-created", false);

Service.obs.addObserver

>+  getInjected: function() {
>+    let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci.nsIXMLHttpRequest);
>+    xhr.open("GET", "chrome://browser/content/injected.js", false);
>+    xhr.overrideMimeType("text/plain");
>+    xhr.send(null);
>+    return xhr.responseText;
>+  },
>+
>+  observe: function(subject, topic, data) {
>+    let sandbox = new Components.utils.Sandbox(Cc["@mozilla.org/systemprincipal;1"].createInstance(Ci.nsIPrincipal));
>+    sandbox.window = subject.wrappedJSObject;
>+
>+    sandbox.importFunction(function(aStoreURI, aManifestURI, aInstallData, aSuccessCallback, aErrorCallback) {
>+        OpenWebapps.install(aStoreURI, aManifestURI, aInstallData, aSuccessCallback, aErrorCallback);
>+        }, "OpenWebapps_install");
>+    
>+    sandbox.importFunction(function(aAppURI, aSuccessCallback, aErrorCallback) {
>+        OpenWebapps.amInstalled(aAppURI, aSuccessCallback, aErrorCallback);
>+        }, "OpenWebapps_amInstalled");
>+    
>+    sandbox.importFunction(function(aStoreURI, aSuccessCallback, aErrorCallback) {
>+        OpenWebapps.getInstalledBy(aStoreURI, aSuccessCallback, aErrorCallback);
>+        }, "OpenWebapps_getInstalledBy");
>+
>+    let toInject = OpenWebapps.getInjected();
>+    Cu.evalInSandbox(toInject, sandbox, "1.8", "chrome://browser/content/injected.js", 1);
>+  }
>+};
>+
>+OpenWebapps.init();

>diff --git a/mobile/chrome/content/injected.js b/mobile/chrome/content/injected.js
>+}
>\ No newline at end of file

Add new line

>diff --git a/mobile/modules/Makefile.in b/mobile/modules/Makefile.in

>   LocaleRepository.jsm \
>   linuxTypes.jsm \
>   video.jsm \
>+  openwebapps.jsm \

openWebapps.jsm ?

>+
>+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>+Cu.import("resource://gre/modules/Services.jsm");
>+
>+XPCOMUtils.defineLazyGetter(this, "NetUtil", function() {
>+  Cu.import("resource://gre/modules/NetUtil.jsm");
>+  return NetUtil;
>+});
>+
>+let OpenWebapps = {

>+  init: function() {

>+        // Read webapps json file into a string
>+        let state = Cc["@mozilla.org/supports-string;1"].createInstance(Ci.nsISupportsString);
>+        state.data = NetUtil.readInputStreamToString(aStream, aStream.available()) || "";
>+        aStream.close();
>+
>+        let data = null;
>+        try {
>+          dump(data + "\n");
>+          data = JSON.parse(state.data);
>+          self.webapps = data;
>+        } catch (ex) {
>+          Cu.reportError("SessionStore: Could not parse JSON: " + ex);

* s/SessionStore/OpenWebappsSupport
* Rework this section. We don't need the nsISupportsString:

          let data = null;
          try {
            data = JSON.parse(NetUtil.readInputStreamToString(aStream, aStream.available()) || "");
            aStream.close();
            self.webapps = data;
          } catch (ex) {
            Cu.reportError("OpenWebappsSupport: Could not parse JSON: " + ex);

>+  _writeFile: function ss_writeFile(aFile, aData) {
>+    let stateString = Cc["@mozilla.org/supports-string;1"].createInstance(Ci.nsISupportsString);
>+    stateString.data = aData;
>+    Services.obs.notifyObservers(stateString, "sessionstore-state-write", "");
>+
>+    // Don't touch the file if an observer has deleted all state data
>+    if (!stateString.data)
>+      return;

Remove this whole block

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Fabrice Desré [:fabrice] from comment #7)

> Each application manifest is saved under $PROFILE/webapps/$APP_ID/manifest.js
> The full list of applications is saved to $PROFILE/webapps/webapps.json

You could save the icon to $PROFILE/webapps/$APP_ID/icon.png
The fielURI to the icon.png could be used in $PROFILE/webapps/webapps.json

> It also creates a manifest for webapps installed from the site menu.

We may not need this, but it's ok for now.

> Todo:
> - implement part of the management API

The docs say this about the Management API:

'The Management API is part of the application repository's API which is privileged, intended to grant access to trusted pages, or "Dashboards".'

I don't know if we have the "privileged" and "trusted pages" parts of this worked yet do we? How does a user give the permission to the page? Do we need this API in the first phase?

Comment 11

[[:fabrice] Fabrice Desré]

Attachment: addressing comments

> >+    app.appURI = aManifest.launch_path ? baseURI.resolve(aManifest.launch_path) : root.substring(0, root.length - 1);
> 
> lanuch_path? launchPath seems more JavaScript-ish

Unfortunately this is part of the manifest defined here : https://developer.mozilla.org/en/OpenWebApps/The_Manifest
 
> UPDATE: I see that you do hold them in content and these are IDs. I would
> consider bundling all callbacks together into a single JS object, not stored
> individually. Here, you would have something like:
> 
>               { apps: apps, callbackID: aMessage.json.callbackID });
> 
> The callbackID would be used in content to retrieve a JS object with both
> success and error callbacks. See below for the content changes.

Nice idea!
 
> >     BrowserUI.popPopup(this);
> >+    if (this._openwebapps) {
> >+      let browser = getBrowser();
> >+      browser.messageManager.sendAsyncMessage("OpenWebapps:InstallAborted", { successCallback: this._application.successCallback, errorCallback: this._application.errorCallback });
> 
> * Shouldn't you reset _openwebapps = false here too?

Useful to help gc, but not needed for logic - we are "protected" by the use of a modal dialog here.


> >+    this.install();
> >+    let browser = getBrowser();
> >+    if (willSendMessage)
> >+      browser.messageManager.sendAsyncMessage("OpenWebapps:InstallDone", { successCallback: this._application.successCallback, errorCallback: this._application.errorCallback });
> * I don't like using getBrowser here. Maybe _openwebapps should point to the
> original aMessage.target and then be used here too.

I moved _openwebapps to be the browser (aMessage.target). I also removed the willSendMessage variable since we can now directly check if browser is null.
 

I also added implementation of 3 management methods :
list, launch and uninstall, since they are useful to implement a web dashboard.

To fully test, use the store at https://apps.mozillalabs.com/appdir/ and the test dashboard at http://people.mozilla.org/~fdesre/openwebapps/dashboard.html

I will change the name of property to navigator.mozApps instead of navigator.apps before pushing.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 552939 [details] [diff] [review]
addressing comments


>diff --git a/mobile/chrome/content/common-ui.js b/mobile/chrome/content/common-ui.js

> var WebappsUI = {

>+  show: function show(aApplication) {
>+    if (!aApplication) {
>+      this._openwebapps = null;
>       // Try every way to get an icon

nit: add a blank line before the comment

>+  install: function(aIconURI) {
>       } catch(e) {
>         Cu.reportError(e);
>       }
>     }
>     image.onerror = function() {
>       // can't load the icon (bad URI) : fallback to the default one from chrome

add a blank line above "image.onerror"

>+      self.install("chrome://browser/skin/images/favicon-default-30.png");
>     }
>-    image.src = aIcon;
>+    image.src = aIconURI || this._application.iconURI;

add a blank line above "image.src=..."

>diff --git a/mobile/chrome/content/content.js b/mobile/chrome/content/content.js
>+  install: function(aStoreURI, aManifestURI, aInstallData, aSuccessCallback, aErrorCallback) {

>+      if (xhr.status == 200) {
>+        try {
>+          let manifest = JSON.parse(xhr.responseText);
>+          if (!OpenWebapps.checkManifest(manifest)) {
>+            if (aErrorCallback)
>+              aErrorCallback({ code: "invalidManifest", message: "Invalid manifest" });
>+          }
>+          else {
>+            sendAsyncMessage("OpenWebapps:Install", { storeURI: aStoreURI.href, manifestURI: aManifestURI, manifest: xhr.responseText,
>+                             installData: aInstallData, callbackID:  OpenWebapps.getCallbackId({ success: aSuccessCallback, error: aErrorCallback }) });
>+          }
>+        }
>+        catch(e) {
>+          if (aErrorCallback)
>+            aErrorCallback({ code: "manifestParseError", message: "Unable to parse the manifest" });
>+        }
>+      }
>+      else if (aErrorCallback) {
>+        aErrorCallback({ code: "networkError", message: "Unable to retrieve manifest" });
>+      }      
>+    }

nit: use "} else {" and "} catch(e) {"

are the error codes ("networkError" for example) spec'ed? or are we supposed to just keep them understandable?


>diff --git a/mobile/chrome/content/injected.js b/mobile/chrome/content/injected.js

I know this file is temporary, but it is using a 4 space indent. Make it use 2 spaces

>diff --git a/mobile/components/Makefile.in b/mobile/components/Makefile.in

>@@ -76,7 +76,6 @@ EXTRA_COMPONENTS = \
>         LoginManager.js \
>         LoginManagerPrompter.js \
>         BlocklistPrompt.js \
>-        WebappsSupport.js \
>         $(NULL)

There is an IDL file (WebappSupport.idl) in here now too. You need to remove it too (MXR is out of date)

>diff --git a/mobile/modules/openWebapps.jsm b/mobile/modules/openWebapps.jsm

>+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>+Cu.import("resource://gre/modules/Services.jsm");
>+Cu.import("resource://gre/modules/Services.jsm");

Services.jsm is listed twice

>+let OpenWebapps = {

>+  install: function(aApplication) {
>+    let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
>+    let id = uuidGenerator.generateUUID().toString();
>+    let dir = this.appsDir.clone();
>+    dir.append(id);
>+    dir.create(Ci.nsIFile.DIRECTORY_TYPE, 0700);
>+    
>+    let manFile = dir.clone();
>+    manFile.append("manifest.json");
>+    this._writeFile(manFile, JSON.stringify(aApplication.manifest));
>+    
>+    this.webapps[id] = {
>+      title: aApplication.title,
>+      storeURI: aApplication.storeURI,
>+      appURI: aApplication.appURI,
>+      iconData: aApplication.iconData,
>+      installData: aApplication.installData,
>+      installTime: (new Date()).getTime(),
>+      manifest: aApplication.manifest
>+    };

Do we need to check for an existing webapp here? the UUID used for the folder will be fine (no conflicts), but the appURI that is used to compare applications in "amInstalled" is not checked here and could cause more than one app to be installed with the same appURI.

Should we check amInstalled first?

r+ but fix the nits, and think about the amInstalled question

Comment 13

[[:fabrice] Fabrice Desré]

Nits fixed, and added a check to not install twice an application.

http://hg.mozilla.org/mozilla-central/rev/f085bbca2ee9

Comment 14

[Daniel Holbert [:dholbert]]

This appears to have triggered perma-orange on Android browser-chrome tests.  Backing out...

Comment 15

[Daniel Holbert [:dholbert]]

Backed out:  http://hg.mozilla.org/mozilla-central/rev/37dedb70a68a
(I also backed out 679194, because it layers on top of this, as noted in that bug)

Details: Android b-c has was mostly (entirely?) green on m-c until this landed.  Since this landed, it's been perma-orange, with each cycle experiencing a timeout in all or some of these four tests:
chrome://mochitests/content/browser/mobile/chrome/tests/browser_addons.js
chrome://mochitests/content/browser/mobile/chrome/tests/browser_scrollbar.js
chrome://mochitests/content/browser/mobile/chrome/tests/browser_tabs.js
chrome://mochitests/content/browser/mobile/chrome/tests/browser_thumbnails.js

Sample log:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1313444106.1313444625.12412.gz

Similarly, Android b-c was green on m-i up until the push that merged this over to m-i.  From that point on, m-i has had the same issue.

So, the above strongly points to this cset causing the timouts that turned Android b-c permaorange.

Comment 16

[[:fabrice] Fabrice Desré]

Attachment: updated patch

Updated patch with the following changes:
- use an xpcom component to implement the navigator property
- move the xpcom and js module to /toolkit
- implemented a watchUpdates() and clearWatch() API calls (documented at https://developer.mozilla.org/en/OpenWebApps/The_JavaScript_API)
- cleaned up code we don't need anymore

Comment 17

[[:fabrice] Fabrice Desré]

Attachment: new updated patch

Fixed a bug in getInstalledBy where the JS apps array was not converted to an nsIOpenWebappsApplication array

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 562050 [details] [diff] [review]
new updated patch

Looks good. The changes from the last full review are mostly limited to the new IDL & impl, plus moving to toolkit.

This is currently only used by mobile, but we plan to go through a simialr review/update when desktop starts to use the code. Putting the code in toolkit from the beginning just seemed like a good idea.

Comment 19

[[:fabrice] Fabrice Desré]

pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/282cc7688390

Comment 20

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/282cc7688390