871956 - calling enumerate() in DeviceStorage API causes device to crash/reboot

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Zac C (:zac)]

Since bug 858416 was merged into v1-train calling enumerate on the DeviceStorage object is causing the device to crash and reboot.

This is blocking all of our automated tests as they rely on DeviceStorage and enumerate to gather a list of media files on the device.

Replicated on:
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/606c4fa198c2
Gaia   23cec618ba1ad9f39da1fc5dfa2a4df2b2d5f933
BuildID 20130513230207
Version 18.0



Here is a test case:

------------test case
self.marionette.execute_script('''
    var mediaTypes = ['pictures'];
    mediaTypes.forEach(function (aType) {
        console.log('getting', aType);
        var storage = navigator.getDeviceStorage(aType);
        console.log(storage);
        
        storage.enumerate();
        });
''')

Comment 1

[Jason Smith [:jsmith]]

We need a stack here.

Zac - Can you get a crash report URL from a phone that is causing the crash?

Comment 2

[Zac C (:zac)]

It's your lucky day!

https://crash-stats.mozilla.com/report/index/06eed765-3eb9-47de-bd42-5ed732130514

Comment 3

[Dave Hylands [:dhylands]]

I'll investigate this.

Comment 4

[Dave Hylands [:dhylands]]

Which device are you seeing the crashes on?

Comment 5

[Zac C (:zac)]

Unagi using b2g18/v1-train. I haven't tested any other devices with v1-train today.

Comment 6

[Dave Hylands [:dhylands]]

Attachment: Add an if check around mFile->mFile

Adds a missing if check. I wasn't able to reproduce the crash, so Marionette must be setting up the environment a little differently than non-marionette.

It's perfectly legal for mFile->mFile to be NULL when using a composite device (because composite devices don't have a real root directory).

Comment 7

[Dave Hylands [:dhylands]]

Comment on attachment 749364 [details] [diff] [review]
Add an if check around mFile->mFile

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

r+'d by kanru on IRC

Comment 8

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/projects/birch/rev/1ff1f234ed00

Comment 9

[Dave Hylands [:dhylands]]

I'm going to change this from leo? to leo+ since this is a regression introduced by bug 858416 (which was leo+) and I really want this to land on b2g18 today, so tomorrows sanity tests won't continue to be tripped up.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1ff1f234ed00

Based on comment 0, it seems that we could actually test this?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/ad04d81b3eeb

Comment 12

[Zac C (:zac)]

Attachment: python script to run storage.enumerate

Dave, it might be too late but here is a python script using marionette to run the script I gave in the first comment.

You will need an engineering build that has marionette on it (all of the PVT builds do by default).

Comment 13

[Dave Hylands [:dhylands]]

Hey zac,

I'm still interested in reproducing the problem, so what do I need to run? I can undo my change locally and run the python script.

I'm trying to understand why the marionette test triggers the problem but why none of the xpcshell tests or the test app trigger the problem.

Comment 14

[Zac C (:zac)]

I had to obsolete that python script as it didn't seem to replicate outside the context of our gaiatest/gaia-ui-tests.

I have a more detailed replication script but you have to do a bit of checking out and what not to run it. Best step now is probably to to CC Jgriffin in here as he knows the guts of Marionette and he may be able to shed some light on the execution difference. If he can't then we can fall back to the other replication script.

Also setting Verified-fixed because our automation suite is back up and running. Thanks!

Comment 15

[Jonathan Griffin (:jgriffin)]

It's hard to say, but one issue may be that the Python script is executing this in the top-level process, and in gaia, this API will likely be exercised in the context of child processes.  You might need to launch an app, switch to its frame, and then try enumerating the storage object.