846090 - XBL mochitests timing out/failing on b2g

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Martijn Wargers (dead)]

I noticed that a lot (all?) of the mochitests involving xbl are failing.
So I tested one xbl mochitest, added this line:
SpecialPowers.addPermission("allowXULXBL", true, document);
And then the test would not time out anymore.

So for some reason, the allowXULXBL permission isn't set.
The locations that are being set is done here:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#434
Those locations are being read from here:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#216

server-locations.txt can be found here:
http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt

It looks like runtestsb2g.py doesn't call this code at all:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#431
While it does so on B2GDesktop:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#546

Comment 1

[Martijn Wargers (dead)]

Attachment: tried this

I tried something like this, but it didn't seem to solve the problem.

Comment 2

[cmtalbert]

Martijn, could you possibly be hitting the issue in bug https://bugzilla.mozilla.org/show_bug.cgi?id=685652?

Comment 3

[Jonathan Griffin (:jgriffin)]

Mochitest is run inside a Gaia app with these permissions:  https://github.com/mozilla-b2g/gaia/blob/master/test_apps/test-container/manifest.webapp

If the container app lacks a permission, there is no way to add it from within mochitest.  We could try adding allowXULXBL to the app's manifest and see if that helps things.

Comment 4

[Jonathan Griffin (:jgriffin)]

We do call initalizeProfile for B2G mochitests, here:

http://mxr.mozilla.org/mozilla-central/source/build/mobile/b2gautomation.py#116

called from 

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#547

Comment 5

[Martijn Wargers (dead)]

Isn't that part of class B2GDesktopMochitest?
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#508

Isn't this code called instead?
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#431

(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> We do call initalizeProfile for B2G mochitests, here:
> 
> http://mxr.mozilla.org/mozilla-central/source/build/mobile/b2gautomation.
> py#116
> 
> called from 
> 
> http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.
> py#547

Comment 6

[Jonathan Griffin (:jgriffin)]

Yes, you're right.  For emulator tests we actually call buildProfile that's in runtests.py here:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#434

and that calls initializeProfile.

Comment 7

[Jonathan Griffin (:jgriffin)]

I should also add that for B2G we don't care about XUL tests, since there are no XUL widgets in B2G.  I think we can safely exclude these tests.

Comment 8

[Martijn Wargers (dead)]

(In reply to Jonathan Griffin (:jgriffin) from comment #3)
> If the container app lacks a permission, there is no way to add it from
> within mochitest.  We could try adding allowXULXBL to the app's manifest and
> see if that helps things.

I tried that, that didn't seem to work.

Comment 9

[Martijn Wargers (dead)]

Attachment: Something like this works as a workaround

Comment 10

[Martijn Wargers (dead)]

From irc:
[22:10]	<gwagner>	mw22: the appId has to match. otherwise we don't allow any permission
[22:11]	<gwagner>	mw22: I see that you always use appId 0

my question, how can you know the appId?
[23:00]	<fabrice>	mw22: it can ba anything, but it has to match what's in the webapps.json

Comment 11

[Martijn Wargers (dead)]

The appId is here inserted in permissions.sqlite database:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#292
It's always 0, which isn't correct, but I have no idea how to get the appId of the mochitest app.

Comment 12

[Martijn Wargers (dead)]

I did this code while running a mochitest:
let appsSvc = SpecialPowers.Cc["@mozilla.org/AppsService;1"].getService(SpecialPowers.Ci.nsIAppsService);
let appId = appsSvc.getAppLocalIdByManifestURL('http://mochi.test:8888');

This returns '0', but that can't be the appId of the mochitest app, since '0' doesn't work.

Comment 13

[Martijn Wargers (dead)]

Ah! Never mind, I should have used:
let appId = appsSvc.getAppLocalIdByManifestURL('http://mochi.test:8888/manifest.webapp');

This gives me '39'.

So I guess I could use that magic number, but that is probably not the best thing to do.

Comment 14

[Jonathan Griffin (:jgriffin)]

B2G mochitests are being updated to use mozbase instead of automation.py.in, so the code that handles profile creation for them is currently at https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/profile.py#L77.

If you moved the webapp manifest updating so that it occurred before the permissions writing, you could then use self.webapps to find the id for the mochitest app.

Unfortunately, we are not using in-tree mozprofile for running B2G mochitests yet, so you wouldn't be able to validate this with a try run.  However, if it worked locally, you could submit a patch to mozbase and we could push that to our internal pypi server that is used to retrieve mozprofile during B2G mochitest runs.

Or, we could wait until we use in-tree mozprofile for B2G mochitests.

Comment 15

[Martijn Wargers (dead)]

I guess that's bug 688667?
Is there a (easy) way to use mozbase instead of automation.py.in, locally?

Comment 16

[Andrew Halberstadt [:ahal]]

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #15)
> I guess that's bug 688667?
> Is there a (easy) way to use mozbase instead of automation.py.in, locally?

If you are using mozilla-central, you are using mozbase instead of automation.py. In that case the allowXULXBL permission should be getting set here: https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L249

To make sure you have the required packages you can just cd testing/marionette/client && python setup.py install

Comment 17

[Jonathan Griffin (:jgriffin)]

If you're wanting to play with mozprofile and see the effects in B2G mochitest, you should instead set up a virtualenv, and then run 'python setup.py develop' from testing/mozbase/mozprofile, before you run setup.py develop from testing/marionette.

Then, you can modify the contents of testing/mozbase/mozprofile, and they will automatically get used when you run mochitests.

Comment 18

[Martijn Wargers (dead)]

This would allow removing the following entries in the exclude list of b2g.json:
content/base/test/test_base.xhtml
content/base/test/test_bug330925.xhtml
content/base/test/test_bug372086.html
content/base/test/test_bug419527.xhtml
content/base/test/test_bug444030.xhtml
content/events/test/test_bug391568.xhtml
layout/inspector/tests/test_bug522601.xhtml
layout/style/test/test_selectors_on_anonymous_content.html

Comment 19

[Martijn Wargers (dead)]

Attachment: workaround patch

Sorry, I don't have the knowledge currently to carry out what is suggested in comment 16 and comment 17.
Is this acceptable for now as a workaround?

Please ignore the file permission changes in this patch, there is something weird going on on my Ubuntu vmware environment with random files getting readonly.

Comment 20

[Jonathan Griffin (:jgriffin)]

Comment on attachment 780625 [details] [diff] [review]
workaround patch

I'd rather not put "random" settings in harness files, as it defeats the purpose of what we're trying to do by consolidating the settings.

Ahal, where would be the right place to put this?  In https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L249, what is 'locations' for mochitests?  Is it some server-locations.txt file, or something else?

Comment 21

[Martijn Wargers (dead)]

Ok, fair enough, it would be a workaround anyway.
Actually, I think these tests could be adjusted to check if the getAnonymousNodes method exists and then skip that xbl related part of the test if xbl is turned off.
I think the mochitest suite in itself should not expect that a browser supports (mozilla's version of) xbl (unless when testing the content/xbl subdiretory).

Comment 22

[Jonathan Griffin (:jgriffin)]

Yes, that would be reasonable as well.  I don't think XBL tests are particularly relevant for B2G anyway.

Comment 23

[Martijn Wargers (dead)]

Just filed bug 898140 for moving xbl/xul mochitests to content/xul content/xbl directories.

Comment 24

[Andrew Halberstadt [:ahal]]

Comment on attachment 780625 [details] [diff] [review]
workaround patch

For reftests we do it here: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/b2g_start_script.js#33

Still kind of hacky I guess. The mochitest version of that script is inside runtestsb2g.py, but if you wait a day or two, it'll be moved to a separate file.

Comment 25

[Andrew Halberstadt [:ahal]]

Also yes, a location is a single line in a server-locations.txt file.

Comment 26

[Martijn Wargers (dead)]

Attachment: Failed attempt

This is a failed attempt at doing as what was suggested.
Obviously, it isn't working, because I don't really know what I'm doing, but perhaps there is something in there that is in the right direction.

Comment 27

[Martijn Wargers (dead)]

Attachment: Failed attempt nr. 2

With some logging here, When I run this, I don't see any mentioning of 'mochi.test' in the webapps list. The 'mochi.test' app is what I need the appId for, to write to the permissions database.

Comment 28

[Jonathan Griffin (:jgriffin)]

I was under the impression that XBL tests are not relevant for B2G; is that not the case?  If they aren't, we shouldn't spend any time trying to fix them.

Comment 29

[Martijn Wargers (dead)]

That might be, I don't know.
But there are some test files that just depend for a small part on xbl.

Comment 30

[Jonathan Griffin (:jgriffin)]

I'll try to find time to run this patch and see what's happening, but I'm not sure it will be this week; the remainder of the week is very busy!

Comment 31

[Martijn Wargers (dead)]

Attachment: b2g_start_script.diff

Another failed attempt, I tried this in b2g_start_script.js, which seems to work for reftest. But for some reason, this hasn't any effect for mochitest.

Comment 32

[Martijn Wargers (dead)]

Ok, reftest doesn't run oop, so perhaps that's the reason it's working there, but not for mochitest.

Comment 33

[Martijn Wargers (dead)]

Attachment: b2g_start_script.diff

Ok, this is something that seems to be working. This could be an acceptable workaround, right? The code inside the loadFrameScript call is:
addEventListener("DOMWindowCreated", function(e) {
  removeEventListener("DOMWindowCreated", arguments.callee, false);
  var window = e.target.defaultView;
  var msg = {
      'op': 'add',
      'type': 'allowXULXBL',
      'permission': 1,
      'url': 'http://mochi.test:8888',
      'appId': window.document.nodePrincipal.appId,
      'isInBrowserElement': false
  };
  sendSyncMessage('SPPermissionManager', msg);
}, false);

Comment 34

[Martijn Wargers (dead)]

Attachment: b2g_start_script.diff

Ok, this is working too, thanks for Olli Pettay that explained to me to use wrappedJSObject for this (in fact, iirc, I have to use this in one of my other patches, perhaps).
This is the source of the loadFrameScript workaround:
addEventListener("DOMWindowCreated", function(e) {
  removeEventListener("DOMWindowCreated", arguments.callee, false);
  var window = e.target.defaultView;
  window.wrappedJSObject.SpecialPowers.addPermission("allowXULXBL", true, window.document);
}

Comment 35

[Jonathan Griffin (:jgriffin)]

Comment on attachment 792810 [details] [diff] [review]
b2g_start_script.diff

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

This looks fine if a try run (for all B2G tests) is green

Comment 36

[Martijn Wargers (dead)]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=cb9dd0f3e3aa

Comment 37

[Martijn Wargers (dead)]

Attachment: 846090.diff (for check-in)

I talked to jgriffin on irc and he agreed on adding the comment that I added in the patch.
It basically explains this is a workaround for bug 848411.

Tryserver was all green, so this can be checked in.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/2c5c0e94bf7c

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2c5c0e94bf7c