1098664 - [Midori 2.0][HOMO][o2 Germany] webapps.json for firefox hello(loop) is not properly configured

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect, P2)

Description

[sync-1]

FFOS2.0 baseline BuildID: 20140916000205
 
 
 [o2 Germany] webapps.json is not properly configured
 
 As requested in comment #3 at Bug 828715 the webapps.json file under 
 
 /system/b2g/webapps/webapps.json
 
 
 needs to be configured properly as this document:
 
 https://docs.google.com/document/d/1vM0zJqV7pQfcE5sh6gwLfEvryctHuOvNmpj-faIpnOo/edit
 
 "origin": "app://{242c75fd-7ea7-46b6-82ce-317eec093cb8}",
 
 it should be like this:
 
 "origin": "app://loop.services.mozilla.com",
 
 Please fix this problem asap as this blocks homo.

Comment 1

[Lingling Zhao]

Hi Gary,

For loop

Comment 2

[Lingling Zhao]

Hi Gary,

As you said FxOS uses uuid to generate folder for each app in 1095843, but for loop, there is a special requirement.

Comment 3

[Lingling Zhao]

This bug is related to  Bug 1095345.

Comment 4

[Wesly Huang (EPM)]

Hi Lingling:

In your description "...comment #3 at Bug 828715..." seems unrelated to this one, would you double confirm?

Comment 5

[Wesly Huang (EPM)]

Lingling, also have you follow the instruction mentioned in comment#4 of bug 1095345?

Comment 6

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Lingling Zhao,
   Could you provide more detail about loop requirement? Per comment https://bugzilla.mozilla.org/show_bug.cgi?id=1095345#c4, you should follow their instructions for pre-build, could you try that first?

Comment 7

[Viga]

(In reply to GaryChen [:GaryChen][:PYChen][:陳柏宇] from comment #6)
> Hi Lingling Zhao,
>    Could you provide more detail about loop requirement? Per comment
> https://bugzilla.mozilla.org/show_bug.cgi?id=1095345#c4, you should follow
> their instructions for pre-build, could you try that first?

Hi Gary, What I'm interested in is what's the strategy while generate 3rd-apps 's origin?
In what situation uuid would come out.

Comment 8

[Lingling Zhao]

Attachment: cambios en la configuración de apps.docx

Please see the attachment. I'm not sure whether he refers to this doc.
Please see "app://loop.services.mozilla.com" on the right.

Comment 9

[Wesly Huang (EPM)]

Hi Beatriz:

Sorry for bothering but would like to get your advise here, about the problem to pre-load Hello app in SW.

Seems this is related to bug 1095345, is it related to the instruction mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1095345#c4?

Comment 10

[Daniel Coloma:dcoloma]

(In reply to Wesly Huang from comment #9)
> Hi Beatriz:
> 
> Sorry for bothering but would like to get your advise here, about the
> problem to pre-load Hello app in SW.
> 
> Seems this is related to bug 1095345, is it related to the instruction
> mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1095345#c4?

Bug 1095345 is a consequence of this bug, the webapps.json does not include the right value for Hello origin and because of that the authentication does not work.

Comment 11

[Wesly Huang (EPM)]

Hi Daniel:

Thanks for your input, are you suggesting there is something wrong during the pre-load procedure of Hello, causing problem of webapps.json? Is there something need to be specially taken care w/ Hello? We/TCL follow same way to preload other apps and it just works fine, only Hello meet this issue per my understanding.

Comment 12

[Daniel Coloma:dcoloma]

(In reply to Wesly Huang from comment #11)
> Hi Daniel:
> 
> Thanks for your input, are you suggesting there is something wrong during
> the pre-load procedure of Hello, causing problem of webapps.json? 

Yes: The build system should allow an application to specify the domain. It seems this is not working or Mozilla/TCL are not making it working. 

> Is there something need to be specially taken care w/ Hello? 

As said above, the build system should honour the domain specified by apps in the manifest

> We/TCL follow same way to preload other apps and it just works fine, only Hello meet this issue per
> my understanding.

The issue is that keeping the specified domain is only important for some apps such as Hello, that is why it might only affect Hello.

Comment 13

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Alex,
   Would you mind take a look on comment 12 ?
   In technical, I think it is doable, but I am not sure is it reasonable to allow an application to specify the domain.

   Thanks.

Comment 14

[Alexandre Poirot [:ochameau]]

Where is this app source code?
There is some ways to specify a custom origin for apps but it depends if it is packaged or hosted.

Comment 15

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Alexandre Poirot [:ochameau] from comment #14)
> Where is this app source code?
> There is some ways to specify a custom origin for apps but it depends if it
> is packaged or hosted.

Oh!!! How to make it?!!?
I think hello(loop) is a packaged app, but I am not sure where can we get source code.

Hi, Danie:
Could you provide the source code of hello(loop) for testing?
thanks.

Comment 16

[Daniel Coloma:dcoloma]

The app is packaged and available at https://github.com/mozilla-b2g/firefoxos-loop-client

Comment 17

[Alexandre Poirot [:ochameau]]

I'm not sure I'm following what is missing here.
The app works fine when sideloaded.
The grunt script works fine: grunt build and the app seems to be functional.

Are you trying to land this app into main gaia repo and make it work with gaia build system?
So should I just try to move the app folder into apps/loop and make it somehow work?

Comment 18

[Daniel Coloma:dcoloma]

(In reply to Alexandre Poirot [:ochameau] from comment #17)
> I'm not sure I'm following what is missing here.
> The app works fine when sideloaded.
> The grunt script works fine: grunt build and the app seems to be functional.
>

Correct
 
> Are you trying to land this app into main gaia repo and make it work with
> gaia build system?

No, but it might something we'd like to do in the future.

The problem was that when partners generated the build, the origin of the app was replaced with a random one, it seems they managed to solve that issue (not sure how).

> So should I just try to move the app folder into apps/loop and make it
> somehow work?

We made it work by putting the app into the outoftree_apps folder, not sure if that would work in the apps folder

Comment 19

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 26332

We only allows specifying custom origin for PRIVILEGED apps,
here is a patch to also allow that for apps without any particular privilege.

With this patch, you just have to put:
  "origin": "app://loop.services.mozilla.com"
in manifest.webapp file, which is already the case.

Then, in order to integrate into gaia build system you just need:
 - metadata.json
 - application.zip
in the following folder:
  outoftree_apps/loop.services.mozilla.com/

The metadata.json is going to allow you specifying a custom manifestURL/installOrigin in order to ensure the app will be updatable.

I verified what I just suggested, and with this additional gaia patch I got hello to work (verified my phone number with a pin code sent by sms) while being installed during `make reset-gaia`.
But for some reason, I had to manually fix some exception on this line:
  https://github.com/mozilla-b2g/firefoxos-loop-client/blob/master/app/js/screens/settings.js#L231
That... for some unexplained reason, doesn't happen to break the app when being pushed via `grunt build`.

Comment 20

[Daniel Coloma:dcoloma]

(In reply to Alexandre Poirot [:ochameau] from comment #19)
> Created attachment 8526005 [details] [review]
> Pull request 26332
> 
> We only allows specifying custom origin for PRIVILEGED apps,
> here is a patch to also allow that for apps without any particular privilege.
> 

To be on the safe side, what you describe below worked for Hello (at least in 2.0 branch) without any patch at all. Please note that Hello is a privileged app already, so that might be the reason.

> With this patch, you just have to put:
>   "origin": "app://loop.services.mozilla.com"
> in manifest.webapp file, which is already the case.
> 
> Then, in order to integrate into gaia build system you just need:
>  - metadata.json
>  - application.zip
> in the following folder:
>   outoftree_apps/loop.services.mozilla.com/
> 
> The metadata.json is going to allow you specifying a custom
> manifestURL/installOrigin in order to ensure the app will be updatable.
> 
> I verified what I just suggested, and with this additional gaia patch I got
> hello to work (verified my phone number with a pin code sent by sms) while
> being installed during `make reset-gaia`.
> But for some reason, I had to manually fix some exception on this line:
>  
> https://github.com/mozilla-b2g/firefoxos-loop-client/blob/master/app/js/
> screens/settings.js#L231
> That... for some unexplained reason, doesn't happen to break the app when
> being pushed via `grunt build`.

The reason is because 'grunt build' adds a file app/js/version.js with the github commit hash so the app version could be easily identified. We need (Hello team) to add a grunt task to generate the flow to make it work with a make reset-gaia.

Comment 21

[Ricky Chien [:rickychien] (inactive)]

Because I'm not familiar with this part, so I need George's feedback.

Comment 22

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Alex, as you said in comment 19, we need to have below items inside that app folder.
* application.zip (containing manifest.webapp and origin)
* metadata.json

AFAK, they already have defined its type as "privileged", so, with/without your commit, the app's origin should not be uuid unless the app folder is not compressed into application.zip due to ...
https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L256

Comment 23

[Viga]

(In reply to George Duan [:gduan] [:喬智] from comment #22)
> Hi Alex, as you said in comment 19, we need to have below items inside that
> app folder.
> * application.zip (containing manifest.webapp and origin)
> * metadata.json
> 
> AFAK, they already have defined its type as "privileged", so, with/without
> your commit, the app's origin should not be uuid unless the app folder is
> not compressed into application.zip due to ...
> https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L256

https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L256
isn't called at build time.

Actually, this uuid is generated here:
https://github.com/mozilla-b2g/gaia/blob/v2.0/build/webapp-manifests.js#L51
It reads from webapp, which is a object of metadata.json

It doesn't come from application.zip

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

hmmm...
Let me make it more cleared.

1. webapp object is generated by 
https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L303

2. pckManifest of webapp object is generated by 
https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L347
which is readZipManifest
https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L252

3. if zipFile doesn't exist, we get null webapp.pckManifest
https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L257

4. pckManifest is null, so isPackaged is false
https://github.com/mozilla-b2g/gaia/blob/v2.0/build/webapp-manifests.js#L55

5. Since webapp.metadata.origin is undefined
https://github.com/mozilla-b2g/firefoxos-loop-client/blob/master/metadata.json#L7
this origin would be set to webappTargetDirName which is 'app://' + uuid
https://github.com/mozilla-b2g/gaia/blob/v2.0/build/webapp-manifests.js#L80

Please correct me if I may misunderstood.

if application is compressed to zip and has defined origin inside manifest.webapp ,
then we should get it from here.
https://github.com/mozilla-b2g/gaia/blob/v2.0/build/webapp-manifests.js#L72

Comment 25

[Alexandre Poirot [:ochameau]]

Comment on attachment 8526005 [details] [review]
Pull request 26332

Right, I didn't realized the app was privileged.
That's because in fillExternalAppManifest, webapp.appStatus was reporting 'web' instead of 'privileged'!
I updated the pull request to fix that.
That's because the code look for webapp type in metadata.json and default to 'web' if nothing is given.
So instead of defaulting to 'web', in this patch we now defauts to the manifest.app in the package.

Also I kept a cleanup in fillExternalAppManifest in order to set an item in `uuidMapping` only when strictly needed.

Comment 26

[Viga]

gduan, Your logic is right. But application.zip is find, the root is what Alexandred find.
The type can't be get in metadata.json.
Please review his patch.

Comment 27

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8526005 [details] [review]
Pull request 26332

r=gduan, it makes sense to me.

Comment 28

[Viga]

Any updates? Can it be merged?

Comment 29

[Alexandre Poirot [:ochameau]]

Sorry, I'm struggling to get a green try.
I just pushed another revision to the pull request in order to spawn another try run.

Comment 30

[Alexandre Poirot [:ochameau]]

Try is finally running and green!

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/e3ab9dceeb7db02e57c9b44558cc54fb552a297b

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Reverted for blowing up emulator mochitests.
Master: https://github.com/mozilla-b2g/gaia/commit/d785ba47ff90e1735bfdbdb7c6cadeef001a8eda

https://treeherder.mozilla.org/logviewer.html#?job_id=953356&repo=b2g-inbound

Comment 33

[Alexandre Poirot [:ochameau]]

:/

Let's verify with a try run:
  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=37ddec0e35bd

Comment 34

[Viga]

(In reply to Alexandre Poirot [:ochameau] from comment #25)
> Comment on attachment 8526005 [details] [review]
> Pull request 26332
> 
> Right, I didn't realized the app was privileged.
> That's because in fillExternalAppManifest, webapp.appStatus was reporting
> 'web' instead of 'privileged'!
> I updated the pull request to fix that.
> That's because the code look for webapp type in metadata.json and default to
> 'web' if nothing is given.
> So instead of defaulting to 'web', in this patch we now defauts to the
> manifest.app in the package.
> 
> Also I kept a cleanup in fillExternalAppManifest in order to set an item in
> `uuidMapping` only when strictly needed.

Hello I tried the patch, seems Not OK.
What we need get is webapp.appStatus, which should get from the manifest inside application.zip.
You add webapp.manifest.type, which still not OK

Comment 35

[Alexandre Poirot [:ochameau]]

Ryan, it looks like I got green emulator mochitests on try, wasn't that failing because of another patch?

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=37ddec0e35bd

Comment 36

[Ryan VanderMeulen [:RyanVM]]

It certainly greened up post-backout. Needs-clobber or something?

Comment 37

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 26721

George, I don't get how I got the previous patch to work locally. If we have a metadata.json, webpapp.manifest should be null as there is no manifest.webapp file in the app folder, but only in the application.zip, which is refered by webapp.pckManifest.

Also, I'm wondering why we are introduced 'pckManifest' it looks misleading, wouldn't be easier if it was also webapp.manifest for external apps?? (anyway that's not something we should address in this bug!)

Comment 38

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8535041 [details] [review]
Pull request 26721

r=gduan
based on comment 34, r+ this patch.
 
Indeed, we should have better naming for webapp.manifest(update.webapp) and webapp.pckManifest(manifest.webapp of application.zip) for external app. or it might be misleading.

In fact, I'm not quite sure the relation of failure from comment 32 with this modification.

Comment 39

[Alexandre Poirot [:ochameau]]

Just rebased and fixed tests error.
webapp.pckManifest isn't always available. We also have external hosted apps, with just manifest.webapp and metadata.json files.

Comment 40

[Alexandre Poirot [:ochameau]]

Green except what looks like an intermitent:
  apps/system/test/marionette/software_home_attention_window_test.js | Software Home Button - Attention window "before each" hook

Comment 41

[Alexandre Poirot [:ochameau]]

weijia, could you confirm that everything is fine with the last patch?

Comment 42

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/2b696f097884262a1dbffe1b031ad7646f8fc8b3

Comment 43

[Ryan VanderMeulen [:RyanVM]]

This blew up mochitests on B2G again. Don't know what to tell you. Reverted.
Master: https://github.com/mozilla-b2g/gaia/commit/d01fa939f14846f3e4823ec7f39e27bbf58acc17

Comment 44

[Alexandre Poirot [:ochameau]]

Retrying a try run... hopefully this one will fail, otherwise I'm lost :x

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8d6e1b3b184f

Comment 45

[Viga]

(In reply to Alexandre Poirot [:ochameau] from comment #41)
> weijia, could you confirm that everything is fine with the last patch?

I test it and it works fine!

Comment 46

[Alexandre Poirot [:ochameau]]

Ok , I'm lost.

Is this correct?
diff --git a/b2g/config/gaia.json b/b2g/config/gaia.json
--- a/b2g/config/gaia.json
+++ b/b2g/config/gaia.json
@@ -1,9 +1,9 @@
 {
     "git": {
         "git_revision": "", 
-        "remote": "", 
-        "branch": ""
+        "remote": "https://github.com/ochameau/gaia",
+        "branch": "customOrigin2"
     }, 
     "revision": "f741dca42f03f16982c026484be42126a60a8adc", 
     "repo_path": "integration/gaia-central"
 }

Am I really running test against my branch with such patch?

Comment 47

[Ricky Chien [:rickychien] (inactive)]

yo! I think you're correct and remember you have "hg pull && hg update" to latest version before you push to try. :)

I usually run into such annoying things from treeherder. Patch passed on try but failed on b2g-inbound. IMO, it's possible that a performance issue since different running speed between try and b2g-inbound.
Otherwise, I suspect that there is a bug on try that succeed with errors.

Comment 48

[Alexandre Poirot [:ochameau]]

I'm wondering if bug 1102972 is going to fix a possible race in this codepath...

Comment 49

[Alexandre Poirot [:ochameau]]

Good news!
Bug 1102972 landed without being backed out even if it was modifying the exact same codepath.
It may have fixed the race, I'll rebase the patch and retry to land it.

Comment 50

[Alexandre Poirot [:ochameau]]

Attachment: pull request 27200

Carry over r+ as this is just a rebase.

Comment 51

[Alexandre Poirot [:ochameau]]

"Careful checkin needed"
See comment 43, a previous version of this patch, while having green try, broke mochitests.
Since then a refactoring occured and hopefully fixed some races.

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=4e5742e10b1b

Comment 52

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/4721890b22deb04def5596853ba47cdcac2339b4

Comment 53

[Ryan VanderMeulen [:RyanVM]]

Nope, broke mochitests again.

Master: https://github.com/mozilla-b2g/gaia/commit/d4dac29613076bdba3cb8adc217deadb08a2ac20

Comment 54

[Alexandre Poirot [:ochameau]]

I'm not sure it is a race anymore given that it isn't even intermittent.
I'm starting to think there is something really different in the way we run test on gaia try and gecko CI,
different mochitest harness app setup, different env variable set, ...

Here is the log of the failure:
https://tbpl.mozilla.org/php/getParsedLog.php?id=55919838&tree=B2g-Inbound#error4

Comment 55

[Alexandre Poirot [:ochameau]]

Unassigning given last project updates.