868625 - DeviceStorage: addNamed() crashes when invoked from an inline activity

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[David Flanagan [:djf]]

I'm trying to save a file from the Open activity of the Gallery app. My call to addNamed creates a 0 length file, and then the Gallery crashes. Neither onsuccess or onerror is ever being triggered.

I don't know if we've ever used DeviceStorage from an inline activity before, so possibly this is a novel combination and this bug has gone undetected. Or maybe it is some more recent regression.

Comment 1

[David Flanagan [:djf]]

Attachment: logcat excerpt

I'm attaching a logcat excerpt from right before the crash. It starts with console.log output from right before and right after the call to addNamed().

Comment 2

[David Flanagan [:djf]]

To reproduce, try these steps:

1) Grab this PR from github: https://github.com/mozilla-b2g/gaia/pull/9549

2) Send yourself an email  with an image attached

3) Check your email in the Email app, tap the download button, and then tap the view button

4) You should see the image in the Gallery app (via the Open activity).

5) Tap save.  It is supposed to display a "file saved" message and remain on the preview. But the preview disappears and you see the email again.  If you now look in /sdcard/downloads/email/ you should see a zero length file.

Comment 3

[David Flanagan [:djf]]

Nominating this for leo+ because it is a crash and because it blocks two bugs that are need for MMS

Comment 4

[Dave Hylands [:dhylands]]

So far, I've been able to get as far as it seems to hang inside nsDeviceStorage::Write when it calls aInputStream->Available().

I still need to figure out all of the processes.

Comment 5

[Dave Hylands [:dhylands]]

If I run my test app and do a AddNamed, then I see the following when I do

MOZ_IPC_MESSAGE_LOG=1 /system/bin/b2g.sh

[462][stderr]AddParams: p.type() = sdcard p.relpath = test-composite.txt
[462][stderr]About to GetBlob
[462][stderr]After GetBlob
[462][stderr]About to GetInternalStream
[462][stderr]After GetInternalStream
[462][stderr]About to WriteFileEvent
[495][stderr]About to write data to file
[495][stderr]DeviceStorageFile::Write
[time:1367633810126856][462][PBlobParent] Sending Msg___delete__([TODO])
[time:1367633810127650][628][PBlobChild] Received Msg___delete__([TODO])
[495][stderr]DeviceStorageFile::Write about to create IOEvent 'created'
[495][stderr]DeviceStorageFile::Write about to dispatch IOEvent to main thread
[495][stderr]DeviceStorageFile::Write about to check Available
[495][stderr]DeviceStorageFile::Write about to create outputStream
[495][stderr]DeviceStorageFile::Write about to create buffered outputStream
[495][stderr]DeviceStorageFile::Write about to Call WriteFrom bufSize = 40
[495][stderr]DeviceStorageFile::Write about to create IOEvent 'modified'
[495][stderr]DeviceStorageFile::Write about to dispatch IOEvent to main thread
[time:1367633810229823][462][PContentParent] Sending Msg_FilePathUpdate([TODO])

(so pid 462 is the parent and 495 is my test app)

When I do the Save from the inline activity (trying to save an attached image) then I see:

[681][stderr]AddNamed path = 'downloads/email/red-sports_12.jpg'
[681][stderr]Posting StorageRequest
[time:1367634006731898][681][PBrowserChild] Sending Msg_PContentPermissionRequestConstructor([TODO])
[time:1367634006732905][462][PBrowserParent] Received Msg_PContentPermissionRequestConstructor([TODO])
[time:1367634006734095][681][PContentPermissionRequestChild] Sending Msg_prompt([TODO])
[time:1367634006734736][462][PContentPermissionRequestParent] Received Msg_prompt([TODO])
[time:1367634006746638][462][PContentPermissionRequestParent] Sending Msg___delete__([TODO])
[time:1367634006752131][681][PContentChild] Sending Msg_ConsoleMessage([TODO])
[time:1367634006752924][462][PContentParent] Received Msg_ConsoleMessage([TODO])
[time:1367634006754664][681][PContentChild] Sending Msg_ConsoleMessage([TODO])
[time:1367634006755457][462][PContentParent] Received Msg_ConsoleMessage([TODO])
[time:1367634006756068][681][PContentPermissionRequestChild] Received Msg___delete__([TODO])
[time:1367634006756342][681][PContentChild] Sending Msg_PBlobConstructor([TODO])
[time:1367634006757288][462][PContentParent] Received Msg_PBlobConstructor([TODO])
[time:1367634006757960][681][PBlobChild] Sending Msg_PBlobConstructor([TODO])
[time:1367634006758784][462][PBlobParent] Received Msg_PBlobConstructor([TODO])
[681][stderr]Allow: Sending to parent, relpath = 'downloads/email/red-sports_12.jpg'
[time:1367634006759608][681][PContentChild] Sending Msg_PDeviceStorageRequestConstructor([TODO])
[time:1367634006760462][462][PContentParent] Received Msg_PDeviceStorageRequestConstructor([TODO])
[462][stderr]AddParams: p.type() = pictures p.relpath = downloads/email/red-sports_12.jpg
[462][stderr]About to GetBlob
[462][stderr]After GetBlob
[462][stderr]About to GetInternalStream
[time:1367634006768549][462][PBlobParent] Sending Msg_PBlobStreamConstructor([TODO])
[time:1367634006769282][681][PBlobChild] Received Msg_PBlobStreamConstructor([TODO])
[time:1367634006769495][681][PBlobChild] Sending Msg_PBlobStreamConstructor([TODO])
[Child 681] WARNING: Don't know if this stream is seekable yet!: file /home/work/B2G-unagi/mozilla-b2g18/dom/ipc/Blob.cpp, line 304
[time:1367634006769801][681][PBlobStreamChild] Sending Msg___delete__([TODO])
[Parent 462] WARNING: Don't know if this stream is seekable yet!: file /home/work/B2G-unagi/mozilla-b2g18/dom/ipc/Blob.cpp, line 304
[462][stderr]After GetInternalStream
[462][stderr]About to WriteFileEvent
[676][stderr]About to write data to file
[676][stderr]DeviceStorageFile::Write
[time:1367634006779963][462][PBlobParent] Sending Msg___delete__([TODO])
[time:1367634006781947][681][PBlobChild] Received Msg___delete__([TODO])
[676][stderr]DeviceStorageFile::Write about to create IOEvent 'created'
[676][stderr]DeviceStorageFile::Write about to dispatch IOEvent to main thread
[676][stderr]DeviceStorageFile::Write about to check Available
[time:1367634006785395][462][PBlobParent] Received Msg_PBlobStreamConstructor([TODO])

###!!! [Parent][AsyncChannel] Error: Route error: message sent to unknown actor ID

[462][stderr]
###!!! [Parent][AsyncChannel] Error: Route error: message sent to unknown actor ID

KillProcess(pid = 681)
[462][stderr]KillProcess(pid = 681)
[time:1367634006786951][462][PContentParent] Sending Msg_FilePathUpdate([TODO])

this time 462 is still the parent, 681 is the gallery app, and I'm not sure what 676 is.

Comment 6

[Dylan Oliver [:doliver]]

triage: blocking based on comment #3

Comment 7

[David Flanagan [:djf]]

I've tested to confirm that this is not the same bug as 850941. The blob passed to the activity has a reasonable size.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I'm working on this.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch for mozilla-b2g18

This patch is only for mozilla-b2g18 as the Blob implementation on trunk is significantly different.

Basically all nsIDOMBlob objects hand out nsIInputStream objects for reading their contents. Cross-process nsIDOMBlob objects have an extra constraint in that their nsIInputStream must hold the nsIDOMBlob alive while the stream is being used. This requirement is met just fine for ordinary nsIDOMBlobs, but MozActivity creates a special multipart blob implementation that was failing to honor this requirement. Memory-backed blobs were also broken in the same way so I fixed them too.

We'll have to test trunk separately (I can't get my trunk build to load gaia for some reason...)

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

David, can you test this please?

Comment 11

[Dominic Kuo [:dkuo]]

(In reply to ben turner [:bent] from comment #10)
> David, can you test this please?

Ben,

I am implementing the save feature in open activity of the Music app just the same as David did(invoked addNamed() from an inline activity), so I tried to apply your patch and built a mozilla-b2g18 gecko for unagi, it works for me! After music called DeviceStorage: addNamed() it didn't crash again and callback successfully. The new file is also saved. I haven't test for gallery and video(reviewing) but I guess they should both work.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Dominic Kuo [:dkuo] from comment #11)

Thanks Dominic!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Ben says on IRC that any further work for m-c should take place in a new bug.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1198d6865502

Comment 14

[David Flanagan [:djf]]

Ben,

I can confirm Dominic's observation. With this patch in gecko, I can modify the email app to invoke the activity and can save the file without crashing. That is the exact test case that was crashing previously.