919496 - [app manager] once an app has been installed, it doesn't show up in the "installed app" tab

Status: VERIFIED FIXED

(DevTools Graveyard :: WebIDE, defect, P1)

Comment 1

[Paul Rouget [:paul]]

Attachment: Patch v1

Comment 2

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Comment on attachment 809778 [details] [diff] [review]
Patch v1

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

Looks good, but we can easily write a test for the new actor function.
See getAll test:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/apps/tests/unit/test_webappsActor.js#36

You can run this test on firefox desktop by enabling the test:
  http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/apps/tests/unit/xpcshell.ini
and commenting two icon related tests.

::: browser/devtools/app-manager/webapps-store.js
@@ +10,5 @@
>  const dbgClient = Cu.import("resource://gre/modules/devtools/dbg-client.jsm");
>  dbgClient.UnsolicitedNotifications.appOpen = "appOpen";
>  dbgClient.UnsolicitedNotifications.appClose = "appClose"
> +dbgClient.UnsolicitedNotifications.appInstall = "appInstall";
> +dbgClient.UnsolicitedNotifications.appUninstall = "appUninstall";

In bug 919981's patch, I'm registering these notifications, and also appOpen/Close in dbg-client.jsm, so I think we no longer need them here.

@@ +224,5 @@
> +    };
> +
> +    client.request(request, (res) => {
> +      if (res.error) {
> +        return deferred.reject(res.error);

There is no `deferred` here. we should throw or console.error this message.

::: toolkit/devtools/server/actors/webapps.js
@@ +551,5 @@
> +    }
> +
> +    let deferred = promise.defer();
> +    let reg = DOMApplicationRegistry;
> +    let app = reg.getAppByManifestURL(manifestURL);

getAppByManifestURL isn't consistent with getAll. We miss the app manifest. app.manifest should be the app manifest, as a JSON object, but here, it's undefined.

We can fetch it like this:
  let deferred = promise.defer();
  reg.getManifestFor(manifestURL, function (manifest) {
    app.manifest = manifest;
    deferred.resolve({app: app});
  });
  return deferred.promise;

Comment 3

[Paul Rouget [:paul]]

Attachment: Patch v2

Comment 4

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Comment on attachment 810511 [details] [diff] [review]
Patch v2

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

Works great!

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a2601d3f3214

Comment 6

[Ed Morley [:emorley]]

Backed out for assertions, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=28407575&tree=Fx-Team

remote:   https://hg.mozilla.org/integration/fx-team/rev/a9ed5d97225f

Comment 7

[Paul Rouget [:paul]]

Attachment: Patch V2.1

Comment 8

[Paul Rouget [:paul]]

Attachment: Patch V3

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/daf000728e1e

Comment 10

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/daf000728e1e

Comment 11

[Paul Rouget [:paul]]

Comment on attachment 812710 [details] [diff] [review]
Patch V3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature (app manager)
User impact if declined: list of installed apps not updated
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e37fc2634e46

Comment 13

[Petruta Horea [:phorea], Desktop Test Engineering]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0

Verified as fixed on Fx 26 beta 3 (20131107161719) and latest Aurora (20131108004000) using Firefox OS 1.2 or 1.3 Simulator.

Comment 14

[Rezonansowy]

See also: Bug 986979