781379 - getNotInstalled should be under mgmt and return apps from all origins

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Anant Narayanan [:anant]]

In bug 768276 the getNotInstalled function was added, but directly under navigator.mozApps. It should be under navigator.mozApps.mgmt, which signifies APIs that are "privileged".

Both getNotInstalled and getAll fall under the privileged category, while getInstalled can stay directly under mozApps since it only returns apps that were installed from the caller's origin.

Comment 1

[(not reading bugmail) Nick Desaulniers [:\n]]

Code is functional (I think, but I'll be more confident after unit tests), and I've fixed regressions with the other tests.  Now just to write up a chrome mochitest (not a browser mochitest)!

test with:
TEST_PATH=dom/tests/mochitest/webapps/ make -C objdir/ mochitest-chrome

Comment 2

[(not reading bugmail) Nick Desaulniers [:\n]]

Tests are green.  Rebasing, and running tests again to double check everything, then submitting patch for review.

Comment 3

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: getNotInstalled should be under mgmt and return apps from all origins

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 653436 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

Review of attachment 653436 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/tests/mochitest/webapps/test_getNotInstalled.xul
@@ +17,5 @@
> +
> +let _isLaunchable;
> +let steps = [
> +  cleanUp, installApp, monkeyPatchDOMApplicationRegistry, getNotInstalled,
> +  cleanUp, unmonkeyPatchDOMApplicationRegistry, tearDown

I believe you need to undo the monkey patching before calling cleanUp, otherwise the app you installed won't be uninstalled because the getAll used in uninstallAll will skip it

Comment 5

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: getNotInstalled should be under mgmt and return apps from all origins

Comment 6

[(not reading bugmail) Nick Desaulniers [:\n]]

(In reply to :Felipe Gomes from comment #4)
> Comment on attachment 653436 [details] [diff] [review]
> getNotInstalled should be under mgmt and return apps from all origins
> 
> Review of attachment 653436 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/tests/mochitest/webapps/test_getNotInstalled.xul
> @@ +17,5 @@
> > +
> > +let _isLaunchable;
> > +let steps = [
> > +  cleanUp, installApp, monkeyPatchDOMApplicationRegistry, getNotInstalled,
> > +  cleanUp, unmonkeyPatchDOMApplicationRegistry, tearDown
> 
> I believe you need to undo the monkey patching before calling cleanUp,
> otherwise the app you installed won't be uninstalled because the getAll used
> in uninstallAll will skip it

Good catch, I reordered the steps in the test.

Comment 7

[Anant Narayanan [:anant]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e952e6793ae

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 653509 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

Review of attachment 653509 [details] [diff] [review]:
-----------------------------------------------------------------

Please file a follow-up or backout this patch.

::: dom/interfaces/apps/nsIDOMApplicationRegistry.idl
@@ +54,5 @@
>    /**
> +   * the request will return the applications acquired from all origins but
> +   * which are not launchable (e.g. by not being natively installed), or null.
> +   */
> +  nsIDOMDOMRequest getNotInstalled();

You must change the UUID of the interface.

@@ -95,5 @@
>    /**
> -   * the request will return the applications acquired from this origin but which
> -   * are not launchable (e.g. by not being natively installed), or null.
> -   */
> -  nsIDOMDOMRequest getNotInstalled();

And this one also.

Comment 9

[Anant Narayanan [:anant]]

Comment on attachment 653509 [details] [diff] [review]
getNotInstalled should be under mgmt and return apps from all origins

Bug 784245 is the follow-up.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3e952e6793ae