If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

REOPENED
Unassigned

Status

Firefox OS
General
P2
normal
REOPENED
3 years ago
7 months ago

People

(Reporter: sync-1, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S3 (9jan)

Firefox Tracking Flags

(b2g-v2.2 affected)

Details

Attachments

(2 attachments, 2 obsolete attachments)

14.38 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document
Details
46 bytes, text/x-github-pull-request
ochameau
: review+
Details | Review | Splinter Review
(Reporter)

Description

3 years ago
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

3 years ago
Hi Gary,

For loop
Summary: [Midori 2.0][HOMO][o2 Germany] webapps.json for firefox hell0(loop) is not properly configured → [Midori 2.0][HOMO][o2 Germany] webapps.json for firefox hello(loop) is not properly configured

Comment 2

3 years ago
Hi Gary,

As you said FxOS uses uuid to generate folder for each app in 1095843, but for loop, there is a special requirement.
Flags: needinfo?(gchen)

Comment 3

3 years ago
This bug is related to  Bug 1095345.
Hi Lingling:

In your description "...comment #3 at Bug 828715..." seems unrelated to this one, would you double confirm?
Flags: needinfo?(zhaolingling)
Lingling, also have you follow the instruction mentioned in comment#4 of bug 1095345?
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?
Flags: needinfo?(gchen)

Comment 7

3 years ago
(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

3 years ago
Created attachment 8522759 [details]
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.
Flags: needinfo?(zhaolingling)
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?
Flags: needinfo?(beatriz.rodriguezgomez)
See Also: → bug 1095345
(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.
Flags: needinfo?(beatriz.rodriguezgomez)
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.
Flags: needinfo?(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.
Flags: needinfo?(dcoloma)
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.
Flags: needinfo?(poirot.alex)
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.
Flags: needinfo?(poirot.alex)
(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.
Flags: needinfo?(dcoloma)
The app is packaged and available at https://github.com/mozilla-b2g/firefoxos-loop-client
Flags: needinfo?(dcoloma)
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?
(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
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.

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`.
Assignee: nobody → poirot.alex
Attachment #8526005 - Flags: review?(ricky060709)
(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.
Because I'm not familiar with this part, so I need George's feedback.
Flags: needinfo?(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
Flags: needinfo?(gduan)

Comment 23

3 years ago
(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
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 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.
Attachment #8526005 - Flags: review?(ricky060709) → review?(gduan)

Comment 26

3 years ago
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 on attachment 8526005 [details] [review]
Pull request 26332

r=gduan, it makes sense to me.
Attachment #8526005 - Flags: review?(gduan) → review+

Comment 28

3 years ago
Any updates? Can it be merged?
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.
Try is finally running and green!
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/e3ab9dceeb7db02e57c9b44558cc54fb552a297b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.2: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S1 (5dec)
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
Status: RESOLVED → REOPENED
status-b2g-v2.2: fixed → affected
Resolution: FIXED → ---
Target Milestone: 2.2 S1 (5dec) → ---
:/

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

Comment 34

3 years ago
(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
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
Flags: needinfo?(ryanvm)
It certainly greened up post-backout. Needs-clobber or something?
Flags: needinfo?(ryanvm)
Created attachment 8535041 [details] [review]
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!)
Attachment #8535041 - Flags: review?(gduan)
Attachment #8526005 - Attachment is obsolete: true
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.
Attachment #8535041 - Flags: review?(gduan) → review+
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.
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
Keywords: checkin-needed
weijia, could you confirm that everything is fine with the last patch?
Flags: needinfo?(liweijia)
Master: https://github.com/mozilla-b2g/gaia/commit/2b696f097884262a1dbffe1b031ad7646f8fc8b3
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-b2g-v2.2: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S2 (19dec)
This blew up mochitests on B2G again. Don't know what to tell you. Reverted.
Master: https://github.com/mozilla-b2g/gaia/commit/d01fa939f14846f3e4823ec7f39e27bbf58acc17
Status: RESOLVED → REOPENED
status-b2g-v2.2: fixed → affected
Resolution: FIXED → ---
Target Milestone: 2.2 S2 (19dec) → ---
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

3 years ago
(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!
Flags: needinfo?(liweijia)
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?
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.
I'm wondering if bug 1102972 is going to fix a possible race in this codepath...
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.
Created attachment 8545251 [details] [review]
pull request 27200

Carry over r+ as this is just a rebase.
Attachment #8535041 - Attachment is obsolete: true
Attachment #8545251 - Flags: review+
"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
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/4721890b22deb04def5596853ba47cdcac2339b4
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-b2g-v2.2: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
Nope, broke mochitests again.

Master: https://github.com/mozilla-b2g/gaia/commit/d4dac29613076bdba3cb8adc217deadb08a2ac20
Status: RESOLVED → REOPENED
status-b2g-v2.2: fixed → affected
Resolution: FIXED → ---
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
Unassigning given last project updates.
Assignee: poirot.alex → nobody
Blocks: 1258639
You need to log in before you can comment on or make changes to this bug.