See url testcase, when loading that testcase, you should get to see 'succeed!' after 500ms. On the b2g OS, this seems to cause a crash, because I get to see a 'Well, this is embarassing' screen to see after a short while in the b2g browser. This is the cause of the failure in: http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_bug431701.html?force=1 And I think also this one: http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_bug422537.html?force=1

Please provide a stack trace (see https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Getting_crashes_off_the_Device).

mwargers@ubuntu:~/B2G$ adb shell ls -l /data/b2g/mozilla/Crash\ Reports/submitted/ gives me: /data/b2g/mozilla/Crash Reports/submitted/: No such file or directory mwargers@ubuntu:~/B2G$ adb shell b2g-ps gives me: /system/bin/b2g-ps: 36: Syntax error: Bad substitution From adb logcat: E/GeckoConsole( 45): [JavaScript Error: "Error: Permission denied to access property 'top'" {file: "app://browser.gaiamobile.org/js/browser.js" line: 554}] F/libc ( 441): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1) I/Gecko ( 45): I/Gecko ( 45): ###!!! [Parent][AsyncChannel] Error: Channel error: cannot send/recv I/Gecko ( 45): I/DEBUG ( 406): debuggerd committing suicide to free the zombie! I/DEBUG ( 457): debuggerd: Nov 16 2012 00:46:34 That's the only I can get out of the b2g emulator. Are there other ways to get stack traces?

I could get a crash report for this, on my Peak with a Nightly build of Firefox OS: bp-7c36897e-57bc-4d53-b0f3-63a562130905

Suffice to say it's easy to reproduce by reloading.

Somewhat different stack on a 1.1 build on my otoro: bp-49ebe465-4cd4-4891-b8ce-48f892130905

It's likely that this is coming from sending a Document, which creates an nsStorageInputStream which doesn't inherit from nsIIPCSerializableInputStream.

Are stack traces from crashes on the emulator not possible?

If comment 6 is right (which seems plausible), who should own this change to the IPC code?

This builds, but it's totally untested. If someone else feels inspired they can try it out and even try to fix it up. Recommendations for testing it?

I will.

I've got an xpcshell test that reproduces this, which is lovely.

Test passes. Now hopefully somebody can tell me whether this is a sensible way to serialize and deserialize the input stream.

Comment on attachment 801241 [details] [diff] [review] Make StorageInputStream serializable cross-process. I'm absolutely sure that I am not the correct reviewer for this. Who reviewed the original serializable networking streams?

I reworked all of this a while ago so it's probably me.

Comment on attachment 801241 [details] [diff] [review] Make StorageInputStream serializable cross-process. I suppose this is Ben's baby.

Comment on attachment 801241 [details] [diff] [review] Make StorageInputStream serializable cross-process. Review of attachment 801241 [details] [diff] [review]: ----------------------------------------------------------------- As discussed on IRC we may be able to ignore the complexity here and simply serialize as a normal StringInputStreamParams. If you hit problems just re-r? me.

Comment on attachment 827684 [details] [diff] [review] Make StorageInputStream serializable cross-process. Review of attachment 827684 [details] [diff] [review]: ----------------------------------------------------------------- This looks great! Thanks. ::: xpcom/io/nsStorageStream.cpp @@ +55,5 @@ > nsStorageStream::nsStorageStream() > : mSegmentedBuffer(0), mSegmentSize(0), mWriteInProgress(false), > mLastSegmentNum(-1), mWriteCursor(0), mSegmentEnd(0), mLogicalLength(0) > { > + LOG_(("Creating nsStorageStream [%p].\n", this)); Are all these LOG changes just line endings or something? @@ +532,5 @@ > + nsCString combined; > + int32_t capacity = mStorageStream->mLastSegmentNum; > + uint32_t offset = mReadCursor; > + for (int32_t i = mSegmentNum; i <= capacity; i++) { > + combined.Append(mStorageStream->mSegmentedBuffer->GetSegment(i) + offset); Hm, you've got some sign fun going on here... I realize we will most likely never have more than INT32_MAX segments here, but we can just make it safe now and never have to worry about it. How about you just do this: nsCString combined; if (mStorageStream->mLastSegmentNum >= 0) { uint32_t capacity = uint32_t(mStorageStream->mLastSegmentNum); uint32_t offset = mReadCursor; for (uint32_t i = mSegmentNum; i <= capacity; i++) { // ... } }

(In reply to ben turner [:bent] (needinfo? encouraged) from comment #18) > Comment on attachment 827684 [details] [diff] [review] > Make StorageInputStream serializable cross-process. > > Review of attachment 827684 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks great! Thanks. > > ::: xpcom/io/nsStorageStream.cpp > @@ +55,5 @@ > > nsStorageStream::nsStorageStream() > > : mSegmentedBuffer(0), mSegmentSize(0), mWriteInProgress(false), > > mLastSegmentNum(-1), mWriteCursor(0), mSegmentEnd(0), mLogicalLength(0) > > { > > + LOG_(("Creating nsStorageStream [%p].\n", this)); > > Are all these LOG changes just line endings or something? Renaming LOG to LOG_ because of the Chromium headers. Want me to just do #undef LOG instead?

(In reply to comment #19) > (In reply to ben turner [:bent] (needinfo? encouraged) from comment #18) > > Comment on attachment 827684 [details] [diff] [review] > > Make StorageInputStream serializable cross-process. > > > > Review of attachment 827684 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > This looks great! Thanks. > > > > ::: xpcom/io/nsStorageStream.cpp > > @@ +55,5 @@ > > > nsStorageStream::nsStorageStream() > > > : mSegmentedBuffer(0), mSegmentSize(0), mWriteInProgress(false), > > > mLastSegmentNum(-1), mWriteCursor(0), mSegmentEnd(0), mLogicalLength(0) > > > { > > > + LOG_(("Creating nsStorageStream [%p].\n", this)); > > > > Are all these LOG changes just line endings or something? > > Renaming LOG to LOG_ because of the Chromium headers. Want me to just do #undef > LOG instead? This should no longer be an issue, I renamed those macros.

Backed out for ASAN errors on the new test: https://hg.mozilla.org/integration/mozilla-inbound/rev/08e0f1a9d9fa https://tbpl.mozilla.org/php/getParsedLog.php?id=32540159&tree=Mozilla-Inbound

When fixing this, could you also remove the lines in the b2g.json file that are about this? http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g.json#93

What is the status of this?

It bounced with ASAN errors and I haven't looked at it since.

https://tbpl.mozilla.org/?tree=Try&rev=f8c62171f162 Trying a new approach.

ASAN did not complain about this one, and it involves understanding 100% less of the implementation of nsStorageInputStream and nsSegementedBuffer.

Comment on attachment 8428297 [details] [diff] [review] Make StorageInputStream serializable cross-process Review of attachment 8428297 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: xpcom/io/nsStorageStream.cpp @@ +594,5 @@ > + > + combined.SetCapacity(remaining); > + uint32_t numRead = 0; > + > + rv = Read(combined.BeginWriting(), remaining, &numRead); This won't close on EOF will it?

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #29) > ::: xpcom/io/nsStorageStream.cpp > @@ +594,5 @@ > > + > > + combined.SetCapacity(remaining); > > + uint32_t numRead = 0; > > + > > + rv = Read(combined.BeginWriting(), remaining, &numRead); > > This won't close on EOF will it? It doesn't appear so from reading the code.

Just waiting for the tree to open before this can land.