1063569 - [e10s] dom/devicestorage/test/test_dirs.html are disabled

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Nils Ohlmeier [:drno]]

As part of bug 1057558 we disabled the IPC test dom/devicestorage/test/test_dirs.html as it was causing perma-orange.

Someone needs to investigate why this test if failing and enable it once it is fixed.

Comment 1

[Nils Ohlmeier [:drno]]

Jim as you seem to be the author of this test, could you please have a look why this test is failing as part of IPC?

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Sorry, I'm not knowledgeable about how device storage works differently with IPC or how to debug IPC tests. My guess would be SpecialPowers.pushPermissions is not doing what it should. Maybe somebody from Ateam can have a look.

Comment 3

[Nils Ohlmeier [:drno]]

Jonathan do you know someone who could help fixing this?

Comment 4

[Martijn Wargers (dead)]

Running with ./mach mochitest-plain --e10s dom/devicestorage/test/test_dirs.html , I get the same failures as with test_ipc.html:
9 INFO TEST-UNEXPECTED-FAIL | dom/devicestorage/test/test_dirs.html | Should have apps storage with permission - expected PASS
##### GetRootDirectoryForType('sdcard', '') failed #####
10 INFO TEST-UNEXPECTED-FAIL | dom/devicestorage/test/test_dirs.html | Should have sdcard storage - expected PASS
##### GetRootDirectoryForType('crashes', '') failed #####
11 INFO TEST-UNEXPECTED-FAIL | dom/devicestorage/test/test_dirs.html | Should have crashes storage - expected PASS

Comment 5

[Martijn Wargers (dead)]

Attachment: 1063569.diff

I tried something like this, but doesn't work.
This makes it at least clear that pushPermissions seems to work correctly, as testPermission shows that the 'webapps-manage' permission is correctly set.

Comment 6

[Martijn Wargers (dead)]

Dave, could there be something wrong with navigator.getDeviceStorage() that it doesn't work in e10s?

Comment 7

[Jonathan Griffin (:jgriffin)]

I don't think this is a harness issue.  We have other e10s tests that work fine with SpecialPowers.pushPermissions, and Martijn's patch seems to exonerate that as well.  Will wait for Dave's reply before looking further.

Comment 8

[Dave Hylands [:dhylands]]

Hmmm. It looks like the tests are running in a child process, and then it calls:

  dirService->Get(NS_APP_USER_PROFILE_50_DIR, NS_GET_IID(nsIFile),
                  getter_AddRefs(sDirs->apps));

or

    NS_GetSpecialDirectory("UAppData", getter_AddRefs(sDirs->crashes));

sDirs->apps (or sDirs->crashes) comes back NULL.

B2G runs with e10s but takes different code paths, and it doesn't call either of those 2 functions.

In B2G we pass a list of volumes from the parent to the child when the child app starts. This happens through: ContentParent::ForwardKnownInfo

It calls SendVolumes (on B2G). For non-B2G we could perhaps have it send the profile directory and UAppData directory.

Comment 9

[Andrew McCreight [:mccr8]]

Attachment: Make most dom/devicestorage/ tests work with e10s.

These tests use devicestorage_setup() in devicestorage_common() to do
two things:

a) Set up some preferences. This just requires using pushPrefEnv().

b) Delete a special directory for testing device storage things, to
make sure it is empty. This requires running a chrome script and
waiting for it to finish.

Actually converting the individual tests just requires wrapping up the
rest of the test inside a giant function.

Comment 10

[Andrew McCreight [:mccr8]]

Comment on attachment 8686344 [details] [diff] [review]
Make most dom/devicestorage/ tests work with e10s.

Sorry, wrong bug.

Comment 11

[Andrew McCreight [:mccr8]]

This seems like it just works now.

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: Enable devicestorage test_dirs.html.

Dave, does it sound reasonable to enable this? It looks like the concern that led this to be disabled involved test_ipc.html, which is gone now, and various storage testing things have been fixed.

This try run is a horror show but it looks like test_dirs is passing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=85c4362ab798&selectedJob=16059206

Comment 13

[Dave Hylands [:dhylands]]

Comment on attachment 8713811 [details] [diff] [review]
Enable devicestorage test_dirs.html.

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

Seems fine to me.

Comment 14

[Pulsebot]

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

Comment 15

[Carsten Book [:Tomcat]]

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

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1111cbd52899