850218 - Launching the packaged app stub in a PRODUCTION=1 DOGFOOD=1 build launches to a FTP directory, not the app launch path

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Jason Smith [:jsmith]]

Build: B2G 18 3/12/2013
Device: Unagi

I've seen this "FTP" issue enough to make me think something is going wrong. I've seen it randomly in production builds too.

My solid STR though requires a DOGFOOD=1 and PRODUCTION=1 build with make reset-gaia:

1. Flash the latest build
2. Pull master and run make DOGFOOD=1 PRODUCTION=1 reset-gaia
3. Try launching the packaged app stub

Expected - we should launch to the launch_path of the app

Actual - we launch to a FTP directory view of the app with the app:// seen

Comment 1

[Jason Smith [:jsmith]]

Fabrice - Any ideas on why the random FTP directory is sometimes seen on initial launch of certain packaged apps in certain situations? My STR seems to reproduce this consistently, although I've seen it happen in production builds randomly like was David saw in bug 850173.

Comment 2

[Julien Wajsberg [:julienw]]

The launch_path needs to have the actual html file (eg "/index.html"), not the directory (eg: "/").

We don't have a "default" index file as there is on HTTP servers.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> The launch_path needs to have the actual html file (eg "/index.html"), not
> the directory (eg: "/").

Right. Except in this case, the launch_path is specified correctly:

  "launch_path": "/index.html",

> 
> We don't have a "default" index file as there is on HTTP servers.

I'm starting to think that might not be a good idea. For a packaged app, opening up a directory by default with no launch_path doesn't really make a whole lot of sense. If we can't have a default to go to, then we should be requiring the launch_path on packaged apps.

Comment 4

[Julien Wajsberg [:julienw]]

(In reply to Jason Smith [:jsmith] from comment #3)
> (In reply to Julien Wajsberg [:julienw] from comment #2)
> > The launch_path needs to have the actual html file (eg "/index.html"), not
> > the directory (eg: "/").
> 
> Right. Except in this case, the launch_path is specified correctly:
> 
>   "launch_path": "/index.html",

Right, then I don't know at all.

I checked the application.zip that is on the device and it seems to be correct. Maybe fabrice will have an idea.

Comment 5

[[:fabrice] Fabrice Desré]

There's no manifest.webapp at /data/local/webapps/packstubtest, which is why we use a default launch_path. It looks like the bug is in gaia's build system (the manifest.webapp file is also missing in profile/webapps/packstubtest when running make DOGFOOD=1 PRODUCTION=1 profile).

Comment 6

[Julien Wajsberg [:julienw]]

Problem is, the packstubtest app is stored in external-dogfood-apps whereas I believe it should be in dogfood_apps and specified in build/applications-data.js instead.

I think that was Bug 833945 right ?

Comment 7

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Problem is, the packstubtest app is stored in external-dogfood-apps whereas
> I believe it should be in dogfood_apps and specified in
> build/applications-data.js instead.
> 
> I think that was Bug 833945 right ?

There's a separate bug talking about directory organization I believe that's become polluted in Gaia.

Comment 8

[Julien Wajsberg [:julienw]]

Fabrice, in [1], the comment clearly indicates that we don't want to copy the file manifest.webapp. You initially wrote this code, so I'd like to make it crystal clear that now we really want to copy it. And if it's missing, we should report an error.

Or should we extract the file from the zip as we do in [2] ? In that case, could using the "charset" option of |readInputStreamToString| (see [3]) work instead of instanciating a converter for UTF-8 ?

[1] https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-manifests.js#L123
[2] http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#2366
[3] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/NetUtil.jsm#261

Comment 9

[Julien Wajsberg [:julienw]]

Jason, tell me if I'm right: until this bug is fixed there is no way to ship an external packaged app and make it work correctly. If that's real I think this should really be a blocker.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> Jason, tell me if I'm right: until this bug is fixed there is no way to ship
> an external packaged app and make it work correctly. If that's real I think
> this should really be a blocker.

From what I've seen, generally the partner customized packaged apps have not been generating the error described in this bug, but there have been some one off instances where I've seen this bug happen on a random production build here and there, including a partner build at one point.

We probably need to nail down what's causing this error to happen and then use that analysis to conclude whether that's causing the random one-off errors we see in this bug with packaged apps in partner customizations. If it's causing the one-off errors that can happen in partner builds, then yeah, we should nom this block.

Comment 11

[Julien Wajsberg [:julienw]]

jason: I say that because I've seen this behaviour with all packaged apps in https://github.com/telefonicaid/firefoxos-gaia-spain/tree/master/external-apps (eg: game pack).

However, that's right we don't see this with our external-apps in https://github.com/mozilla-b2g/gaia/tree/master/external-apps.

Fabrice, from what I see in [1] we're supposed to extract the manifest.webapp from application.zip when we load the new external app on the device. Could that not work if the metadata.json is wrong ?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#246

Comment 12

[[:fabrice] Fabrice Desré]

We don't need the metadata.json file in gecko. I still don't understand what's going on here... Julien, can you check the manifest.webapp from game pack once it's instsalled?

Comment 13

[Julien Wajsberg [:julienw]]

(In reply to Fabrice Desré [:fabrice] from comment #12)
> We don't need the metadata.json file in gecko. 

I don't understand this ;) I've never proposed to copy metadata.json...

Comment 14

[Julien Wajsberg [:julienw]]

(In reply to Fabrice Desré [:fabrice] from comment #12)
>  Julien, can you check the manifest.webapp from game
> pack once it's instsalled?

It does not have any manifest.webapp, just like you said in comment 5. Same cause, same behaviour.

Comment 15

[Jason Smith [:jsmith]]

Okay, I've now confirmed my comment in comment 10. This happening periodically with partner builds - I saw this happen with Calculator and YouTube. Which also means it's not necessarily a bug that happens only with packaged apps, either.

Comment 16

[Jason Smith [:jsmith]]

Logcat isn't showing anything interesting.

Comment 17

[Jason Smith [:jsmith]]

Attachment: Broken YouTube

Comment 18

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #15)
> Okay, I've now confirmed my comment in comment 10. This happening
> periodically with partner builds - I saw this happen with Calculator and
> YouTube. Which also means it's not necessarily a bug that happens only with
> packaged apps, either.

Actually, disregard the part with the hosted apps. This was with the packaged app version of YouTube on Ikura build. We also saw this happen with the Calculator app too.

Comment 19

[Jason Smith [:jsmith]]

Attachment: Broken Calculator

Comment 20

[Daniel Coloma:dcoloma]

From the latest comments, I conclude the DOGFOOD=1 is not relevant anymore, right? If so, it might be that the bug is in the customisation code (it seems all the affected apps are part of the customisation)?

Comment 21

[Jason Smith [:jsmith]]

(In reply to Daniel Coloma:dcoloma from comment #20)
> From the latest comments, I conclude the DOGFOOD=1 is not relevant anymore,
> right? If so, it might be that the bug is in the customisation code (it
> seems all the affected apps are part of the customisation)?

The build information stated in the bug isn't really relevant to the issue here.

If I had to guess, the bug is happening probably either in the build JS code for Gaia or the gecko layer that lies beneath it.

Comment 22

[Jason Smith [:jsmith]]

In response to your question - yes, this only happens with apps in the external-apps directory, which is part of a customization.

Comment 23

[Julien Wajsberg [:julienw]]

The bug is clearly in the build customization code. I plan to fix this this week (if nobody fixes this before).

Comment 24

[Daniel Coloma:dcoloma]

Adding Yuren as he developed the customization code.

Comment 25

[Julien Wajsberg [:julienw]]

Sorry if I was not clear, this is not in the customization code done by yuren, but in the build code that takes the custom apps as input; that code was done by Fabrice ;)

Comment 26

[Daniel Coloma:dcoloma]

(In reply to Julien Wajsberg [:julienw] from comment #25)
> Sorry if I was not clear, this is not in the customization code done by
> yuren, but in the build code that takes the custom apps as input; that code
> was done by Fabrice ;)

Thanks!

The key question for me is whether that code is used when generating custom builds, and it seems so. If that is not the case, please unblock.

Comment 27

[Jason Smith [:jsmith]]

I don't understand why you unset the tef+ flag here. The comments still imply that this is a bug in gaia build code.

Comment 28

[Daniel Coloma:dcoloma]

So, just to be crystal clear:

 - If it affects the builds our partners are creating for commercial devices, we should block on this.
 - If this does not affect those builds I don't care.

Comment 29

[Jason Smith [:jsmith]]

(In reply to Daniel Coloma:dcoloma from comment #28)
> So, just to be crystal clear:
> 
>  - If it affects the builds our partners are creating for commercial
> devices, we should block on this.
>  - If this does not affect those builds I don't care.

This does affect partner builds. The two screenshots in the bug above are from ZTE partner builds on Ikura devices.

Comment 30

[Julien Wajsberg [:julienw]]

The bug is either in gaia build or in the webapps.jsm code.

Sorry for messing up with the flag, I just saw that I was the one removing it.

Comment 31

[[:fabrice] Fabrice Desré]

Do we have a reproducible case? Like, an app I could put in external-apps and that fails?

Comment 32

[Jason Smith [:jsmith]]

(In reply to Fabrice Desré [:fabrice] from comment #31)
> Do we have a reproducible case? Like, an app I could put in external-apps
> and that fails?

It's going to be tough to get a reproducible case here, but I do still have that phone that ended up in this state in Mt View. I've seen this happen sporadically with certain builds - like the Inari build I showed you with YouTube. Carlos hit this with a different Inari build with the calculator app.

Comment 33

[Julien Wajsberg [:julienw]]

fabrice, I think any packaged app will trigger this.

Try for example https://github.com/telefonicaid/firefoxos-gaia-spain/tree/master/external-apps/calculator

Comment 34

[[:fabrice] Fabrice Desré]

(In reply to Julien Wajsberg [:julienw] from comment #33)
> fabrice, I think any packaged app will trigger this.
> 
> Try for example
> https://github.com/telefonicaid/firefoxos-gaia-spain/tree/master/external-
> apps/calculator

I installed this customization, no problems with the calculator or other packaged apps.

Comment 35

[Julien Wajsberg [:julienw]]

I use an older version of gaia-spain external apps (back when metadata.json was quite wrong) so maybe this bug occurs only when metadata.json is wrong.

Comment 36

[:gerard-majax]

I'm unable to reproduce it on Keon with gaia master. Is there any magic trick to reproduce ?

Comment 37

[Julien Wajsberg [:julienw]]

FYI I work on Bug 862418 these days, which hopefully should help having good and sane metadata.json for our customization partners.

So my wild guess here is that this bug is triggered by bad metadata.json files, and once Bug 862418 is fixed (I hope today) we won't have this bug anymore, providd that our partners use a master gaia to customize their devices.

Comment 38

[Jason Smith [:jsmith]]

Okay. I was trying to reproduce this yesterday, except I accidentally bricked by device by accident. Will try again, but if it's the bad metadata.json issue, then I should not be able to reproduce this then when I test this

Comment 39

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #38)
> Okay. I was trying to reproduce this yesterday, except I accidentally
> bricked by device by accident. Will try again, but if it's the bad
> metadata.json issue, then I should not be able to reproduce this then when I
> test this

Or actually let me reword this - using customization repos, I shouldn't be able to reproduce this. I'll try messing with the metadata.json to see if that causes this bug.

Comment 40

[Julien Wajsberg [:julienw]]

I've just seen this again with a valid app (the packaged notes from Bug 866720). So definitely not fixed and I'll make this my new priority. However, won't probably be able to work on this before next Tuesday so if someone (Fabrice ?) wants to take it, feel free.

I got it with the new packaged Notes app, and also from the Here Maps app that is in the external-apps directory in Gaia repository. I've also noticed that updating the app solves the bug.

Comment 41

[Jason Smith [:jsmith]]

Pulling qawanted because we've got another confirmed reproduction of this per comment 40.

Comment 42

[[:fabrice] Fabrice Desré]

(In reply to Julien Wajsberg [:julienw] from comment #40)
> I've just seen this again with a valid app (the packaged notes from Bug
> 866720). So definitely not fixed and I'll make this my new priority.
> However, won't probably be able to work on this before next Tuesday so if
> someone (Fabrice ?) wants to take it, feel free.

The notes app there is not correctly set up to be preloaded...

> I got it with the new packaged Notes app, and also from the Here Maps app
> that is in the external-apps directory in Gaia repository. I've also noticed
> that updating the app solves the bug.

Was this the "standard" maps app from the gaia repo, or the one from the spain customization?

Comment 43

[Jason Smith [:jsmith]]

(In reply to Fabrice Desré [:fabrice] from comment #42)
> (In reply to Julien Wajsberg [:julienw] from comment #40)
> > I've just seen this again with a valid app (the packaged notes from Bug
> > 866720). So definitely not fixed and I'll make this my new priority.
> > However, won't probably be able to work on this before next Tuesday so if
> > someone (Fabrice ?) wants to take it, feel free.
> 
> The notes app there is not correctly set up to be preloaded...

See my comment over in the other bug. I'm not following that comment exactly - all of the appcache references & resources was removed in the PR to my understanding.

Comment 44

[[:fabrice] Fabrice Desré]

Forget my last comment. I need to re-init my repos.

Comment 45

[[:fabrice] Fabrice Desré]

I still can't reproduce Comment 40. I triead on 1.0.1, 1.1 and current master and on different devices. If someone in MV can reproduce, give me your device...

Comment 46

[Julien Wajsberg [:julienw]]

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

> Was this the "standard" maps app from the gaia repo, or the one from the
> spain customization?

the one from the gaia repo...

Comment 47

[Julien Wajsberg [:julienw]]

I'm using gecko b2g18 + gaia master, which is not a valid combination afaik, but is still used by lots of dev.

Jason, what configuration are you using ?

Will try with a v1-train tree soon.

Comment 48

[Julien Wajsberg [:julienw]]

same with v1-train;

To be clear, here is what I do :

  make clean reset-gaia

And here maps is wrong after this.

Comment 49

[Julien Wajsberg [:julienw]]

  rm -rf profile && make production

leads to a working here maps.

What this means is that that should not happen on normal builds, and that should happen only on eng builds.

Therefore I think it's not a blocker anymore, even if imho it should still be fixed.

Comment 50

[[:fabrice] Fabrice Desré]

make clean reset-gaia produces an eng gaia build. And in this case we don't add the manifest.webapp file for external apps in the /data/local/webapps/m.here.com/ directory. We then default to a empty launch path value, and display the directory listing.

The fix has to be done in gaia's build system.

Comment 51

[Daniel Coloma:dcoloma]

(In reply to Julien Wajsberg [:julienw] from comment #49)
>   rm -rf profile && make production
> 
> leads to a working here maps.
> 
> What this means is that that should not happen on normal builds, and that
> should happen only on eng builds.
> 
> Therefore I think it's not a blocker anymore, even if imho it should still
> be fixed.

is the notes application also working for you after wiping your profile?

Comment 52

[Julien Wajsberg [:julienw]]

(In reply to Fabrice Desré [:fabrice] from comment #50)
> make clean reset-gaia produces an eng gaia build. And in this case we don't
> add the manifest.webapp file for external apps in the
> /data/local/webapps/m.here.com/ directory. We then default to a empty launch
> path value, and display the directory listing.
> 
> The fix has to be done in gaia's build system.

are you sure ?

Do we want to extract the manifest.webapp file from application.zip when building an eng build, but not when building a user build ? Seems strange to me...

(In reply to Daniel Coloma:dcoloma from comment #51)
> (In reply to Julien Wajsberg [:julienw] from comment #49)
> >   rm -rf profile && make production
> > 
> > leads to a working here maps.
> > 
> > What this means is that that should not happen on normal builds, and that
> > should happen only on eng builds.
> > 
> > Therefore I think it's not a blocker anymore, even if imho it should still
> > be fixed.
> 
> is the notes application also working for you after wiping your profile?

Can't try now, keeping the NI to try tomorrow.

Comment 53

[Jason Smith [:jsmith]]

Keeping this tef? sounds right to me at this point. We do know this issue will happen with engineering builds no matter what.

However, there's still the issue of why this was seen on two partner production builds. Though this scenario happens rarely however - I've only seen this issue happen twice throughout all production builds I've used.

Comment 54

[Jason Smith [:jsmith]]

I'm going to clear the nom though here until we get a better picture if this actually reproduces again on a production build.

We'll be doing test passes soon as partner customizations again, so if this reproduces again, we'll notice it.

Comment 55

[Julien Wajsberg [:julienw]]

Fabrice, why do we need to put preinstalled apps in /data/local/webapps instead of /system/b2g/webapps for eng build ? If we change this, this would fix this bug.

Another way to fix this would be to check in /data/local/webapps in the error case in [1] instead of returning. If I am not wrong this should work because we execute this only at first run. Or is the name "isFirstRun" in [2] misleading ?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#284
[2] http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#453

Comment 56

[[:fabrice] Fabrice Desré]

(In reply to Julien Wajsberg [:julienw] (PTO 30th May -> 10th June with no access to my bugmail) from comment #55)
> Fabrice, why do we need to put preinstalled apps in /data/local/webapps
> instead of /system/b2g/webapps for eng build ? If we change this, this would
> fix this bug.

I don't know if this is still true, but we did that because the /system partition used to be to small to fit all the test apps - some where really big because of embedded media files for instance.

> Another way to fix this would be to check in /data/local/webapps in the
> error case in [1] instead of returning. If I am not wrong this should work
> because we execute this only at first run. Or is the name "isFirstRun" in
> [2] misleading ?

That code is executed at first run or after a gecko update. If we do what you propose, we'll need to only run the part at https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#338

Currently the gaia build system does the right thing with gaiamobile.org apps in all cases (ENG and USER builds). Why not fix it to also do the right thing with 3rd party preloaded apps?

Comment 57

[Julien Wajsberg [:julienw]]

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

> Currently the gaia build system does the right thing with gaiamobile.org
> apps in all cases (ENG and USER builds). Why not fix it to also do the right
> thing with 3rd party preloaded apps?

That's completely right, I'll try to go this way.

Thanks !

Comment 58

[Julien Wajsberg [:julienw]]

So, it's quite different for gaiamobile.org apps and 3rd party apps.

gaiamobile.org apps have a manifest.webapp ready in their directory, and have no update.webapp file.
external apps have no manifest.webapp, instead it's inside application.zip. They do have an update.webapp to enable updates.

If we want to make things work the same for 3rd party apps and for gaiamobile apps, then we need to remove that code from Webapps.jsm and move it to the build, both for production builds and eng builds. Is that the path we should take ?

Comment 60

[[:fabrice] Fabrice Desré]

I would argue that gaia apps are the special case here. Devs only have to submit a package (and eventually an update.webapp), they don't have to extract the manifest from that package.

We need to ship the manifest.webapp for gaia apps because there's no reason to move them out of the system partition and waste space on /data, but /system is read-only.

So what are the options?
1- extract the manifest.webapp from 3rd party apps in ENG builds only. Gaia only patch.
2- extract the manifest.webapp from 3rd party apps in both ENG and USER builds. Needs gaia changes and gecko changes.

I think doing 2 is slightly better (that will make preloaded apps also work in desktop builds where they are probably broken for the same reason as ENG builds).

Comment 61

[Karen Ward [:kward]]

Jason - Should this app be closed?

Comment 62

[Jason Smith [:jsmith]]

No.

Comment 63

[Julien Wajsberg [:julienw]]

unassigning, I think this is still happening but not sure (and is it really important ?)

Comment 64

[Karen Ward [:kward]]

I think this bug should be resolved.  It is either fixed or won't fix. In either case I don't think it is relevant any longer (at least for Marketplace apps)

Comment 65

[BMO Automation]

Firefox OS is not being worked on