Closed Bug 1063569 Opened 9 years ago Closed 8 years ago

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


(Core :: DOM: Device Interfaces, defect)

Not set



Tracking Status
e10s + ---
firefox46 --- fixed
firefox47 --- fixed


(Reporter: drno, Assigned: mccr8)


(Blocks 1 open bug)


(Whiteboard: [DeviceStorageTest])


(1 file, 2 obsolete files)

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.
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?
Flags: needinfo?(nchen)
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.
Flags: needinfo?(nchen)
Jonathan do you know someone who could help fixing this?
Flags: needinfo?(jgriffin)
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
Attached patch 1063569.diff (obsolete) — Splinter Review
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.
Dave, could there be something wrong with navigator.getDeviceStorage() that it doesn't work in e10s?
Flags: needinfo?(dhylands)
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.
Flags: needinfo?(jgriffin)
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),


    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.
Flags: needinfo?(dhylands)
Component: Mochitest → DOM: Device Interfaces
Product: Testing → Core
Version: unspecified → Trunk
Whiteboard: [DeviceStorageTest]
Blocks: 1218958
tracking-e10s: --- → ?
Summary: dom/devicestorage/test/test_dirs.html are disabled → [e10s] dom/devicestorage/test/test_dirs.html are disabled
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 on attachment 8686344 [details] [diff] [review]
Make most dom/devicestorage/ tests work with e10s.

Sorry, wrong bug.
Attachment #8686344 - Attachment is obsolete: true
This seems like it just works now.
Assignee: nobody → continuation
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:
Attachment #8713811 - Flags: review?(dhylands)
Comment on attachment 8713811 [details] [diff] [review]
Enable devicestorage test_dirs.html.

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

Seems fine to me.
Attachment #8713811 - Flags: review?(dhylands) → review+
Attachment #8589692 - Attachment is obsolete: true
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.