814140 - Expose test-container app's permissions to mochitests which are running on a separate domain

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Comment 1

[Andrew Halberstadt [:ahal]]

(Accidentally hit enter)

The B2G mochitests are running at http://mochi.tests:8888 which means that none of the permissions exposed to the test-container app are available to them. We need to fix this.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: WIP non-working attempt

This was my original idea to fix this problem. Currently I'm getting a "MANIFEST_PARSE_ERROR" at the mozApps.install() step (though as far as I can tell there is nothing wrong with the json in the manifest).

I don't have any other ideas at the moment.

Comment 3

[Andrew Halberstadt [:ahal]]

Excerpt from email thread between myself and David:

> Andrew,
>
> I think there is some genuine confusion as to who was moving forward with this.
> I had assumed it was you or jon griffin who would be working on this, as some of this
> has to do with the general guts of mochitest.
> As in i'm not sure i know the best way to fix some of these issues without working
> closely with someone. 
>
>-David

What we are trying to do in this bug spans three teams, QA with permissions testing, A-team with automation for running mochitests in B2G, devs with an app permissions model. In our situation each party doesn't know anything about what the other two parties are doing, so we're all going to have to get our hands dirty and dig into code that we're not familiar with.

I'll do the best I can, but if you are expecting me to own and drive this bug to completion on my own, you might be waiting awhile (we have a hard Dec. 10th B2G automation deadline). Though maybe I have my priorities mixed up.

Comment 4

[Andrew Halberstadt [:ahal]]

(In reply to Andrew Halberstadt [:ahal] from comment #2)
> Currently I'm getting a
> "MANIFEST_PARSE_ERROR" at the mozApps.install() step (though as far as I can
> tell there is nothing wrong with the json in the manifest).

Nvm, stupid mistake on my part. The actual error at the install step is "INVALID_SECURITY_LEVEL" which isn't documented on mdn. I'm guessing it's because it's running from a context that doesn't have the embed-apps permission.

Comment 5

[[:fabrice] Fabrice Desré]

No, it means that your application is requesting to be ceritified but can't. A workaround is to set the "dom.mozApps.dev_mode" to true to bypass the check.

Comment 6

[Martijn Wargers (dead)]

What does this "dom.mozApps.dev_mode" pref do exactly?
I tried to use this on Fennec and Firefox desktop:
http://competitie-app.kantjils.nl/
I tried to use cross-site xmlhttprequest with that pref turned on.
I added "systemXHR": "true" (and "network-http": "true" fwiw) to the manifest.webapp:
http://competitie-app.kantjils.nl/manifest.webapp

Comment 7

[Andrew Halberstadt [:ahal]]

Some thoughts:
1) I tried to get around the installation wizard but haven't been able to get a handle of it from marionette. I'm guessing it's a popup?
2) :mdas said she thought there was someone who was looking into getting around the install dialog, does anyone know who might know more?
3) I think this needs to be a 'web' app or else we need to zip up every mochitest.. or can it be an empty privileged app and then just having the tests on the same domain would be good enough?
4) I don't think we can add this to gaia/test_apps as then it would be hosted on a separate domain

Comment 8

[David Chan [:dchan]]

(In reply to Andrew Halberstadt [:ahal] from comment #7)
> Some thoughts:
> 1) I tried to get around the installation wizard but haven't been able to
> get a handle of it from marionette. I'm guessing it's a popup?

You can make a change to runtestsb2g.py to automatically accept app install requests. This is what I did to get bug 813814 working

window.addEventListener('mozChromeEvent', function (e) {
  if (e.detail.type != 'webapps-ask-install') {
    return;
  }

  var detail = {
    type: 'webapps-install-granted',
    id: e.detail.id,
  }
  // breaking abstraction and call eventhandler directly
  window.CustomEventManager.handleEvent({'detail': detail});
});

The code was added to 
self._automation.test_script

> 2) :mdas said she thought there was someone who was looking into getting
> around the install dialog, does anyone know who might know more?
> 3) I think this needs to be a 'web' app or else we need to zip up every
> mochitest.. or can it be an empty privileged app and then just having the
> tests on the same domain would be good enough?

I would recommend using the mozApps.dev_mode workaround and a SJS. See webapp. sjs in bug 811141. The sjs takes a query string with the app type and permissions that should be in the manifest. I strongly recommend going through the install workflow to set permissions since they don't map 1-1, e.g. 'contacts' is expanded to contacts-read, contacts-create and/or contacts-write in the DB


> 4) I don't think we can add this to gaia/test_apps as then it would be
> hosted on a separate domain

Comment 9

[[e-mail] [:onecyrenus]]

Andrew I am attaching a patch here that fixes the problem, but takes a slightly different approach than David Chan's approach above. 

#1) install mochi.test webapp using the gaia install process. 
I am doing this by defining mochi.test as an external app inside gaia. ( The release team will find a way to segment this when they are parsing out marketplace staging / marketplace dev)

#2) slight modification to runtestsb2g / test-container app. 

So there are two sets of changes gecko / gaia. 
your patch above was 90% of the way there

Comment 10

[[e-mail] [:onecyrenus]]

Attachment: Gecko patch

This removes some of the unneeded pieces of installing mochi.test.

Comment 11

[[e-mail] [:onecyrenus]]

Attachment: gaia patch

This patch installs mochi.test correctly. as well as fixes the launching of mochi.test from test-container.

Comment 12

[Andrew Halberstadt [:ahal]]

Comment on attachment 688810 [details] [diff] [review]
gaia patch

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

Awesome! I can review the gecko patch if it's ready.

::: test_apps/test-container/index.html
@@ +12,4 @@
>          </style>
>      </head>
>      <body>
> +       <iframe id="test-container" mozbrowser mozapp="http://mochi.test:8888/manifest.webapp" </iframe>

Would it still work if we set 'mozapp' in runtestsb2g.py instead? I was thinking the test-container app would be more of a generic app that any test frameworks could make use of if they needed to. If it has to be this way, then that's fine too though.

Comment 13

[Andrew Halberstadt [:ahal]]

I can get the gecko patch landed for you. As for the gaia one, you'll need to submit a pull request after getting it reviewed. Pull requests normally take 2-5 days, but if you go to #gaia and bring up that it's a change needed for testing you can probably get it landed much quicker

Comment 14

[Andrew Halberstadt [:ahal]]

Comment on attachment 688809 [details] [diff] [review]
Gecko patch

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

So this looks good to me, but like I said in my previous comment, if setting the mozapp attribute programmatically in runtestsb2g.py works (like my obsoleted patch), then I'd appreciate it if we did that instead of making the test-container app mochitest specific. If it doesn't work, then I'm fine leaving everything as is.

Comment 15

[[e-mail] [:onecyrenus]]

Attachment: second patch, basically define mozapp in runtestsb2g.py

Comment 16

[[e-mail] [:onecyrenus]]

Attachment: second patch remote changes to test-container

Comment 17

[Andrew Halberstadt [:ahal]]

Comment on attachment 688962 [details] [diff] [review]
second patch, basically define mozapp in runtestsb2g.py

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

Looks good!

Comment 18

[Andrew Halberstadt [:ahal]]

Comment on attachment 688963 [details] [diff] [review]
second patch remote changes to test-container

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

I'm going to re-direct this to fabrice as I have no idea what an "external-app" even is (or what the origin file is for). You might be able to just attach the patch to a pull request to https://github.com/mozilla-b2g/gaia and get someone to review it there.

Comment 19

[[e-mail] [:onecyrenus]]

https://github.com/mozilla-b2g/gaia/pull/6862

Comment 20

[Andrew Halberstadt [:ahal]]

I pushed the gecko patch to try to see if it's ok to land already: https://tbpl.mozilla.org/?tree=Try&rev=ab3d9df84f17

Once the gaia change lands we'll need to update the snapshot that buildbot uses to make the builds.

Comment 21

[[e-mail] [:onecyrenus]]

https://github.com/mozilla-b2g/gaia/pull/6867

I have changed the gaia patch slightly so that mozapp is set inside the test-container. 

Unfortunately this is unchangeable, and due to the way iframes are loaded.

Comment 22

[[:fabrice] Fabrice Desré]

Comment on attachment 688963 [details] [diff] [review]
second patch remote changes to test-container

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

The origin file is changing format in bug 812198 (it's remplaced by a metadata.json file that would be { "origin": "http://..." }

Comment 23

[Andrew Halberstadt [:ahal]]

Try run looked good, pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a8ba2e0ed05

David, is this a change that you need landed for code that is shipping in v1? Or are you happy to work off of m-c and let it ride the trains?

Also let me know when the gaia change lands and I'll file a bug to get the gaia snapshot that releng uses updated.

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0a8ba2e0ed05