Created attachment 651761 [details] [diff] [review] Patch, v1 The old IPC::InputStream uses manual WriteParam/ReadParam and also calls do_CreateInstance on a CID string supplied by the child. Attached patch adds serialize hooks to the remaining input stream classes and switches PNecko stuff to use them. r? to khuey for the input stream class changes. r? to cjones for the ipc/ipdl changes (which we discussed over irc).
Comment on attachment 651761 [details] [diff] [review] Patch, v1 r=me on ipc/ipdl changes
Comment on attachment 651761 [details] [diff] [review] Patch, v1 Review of attachment 651761 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/glue/InputStreamUtils.cpp @@ +103,1 @@ > return nullptr; MOZ_NOT_REACHED crashes opt build right? If so, you don't need the return nullptr anymore. ::: netwerk/base/src/nsBufferedStreams.cpp @@ +493,5 @@ > + > + if (mStream) { > + nsCOMPtr<nsIIPCSerializableInputStream> stream = > + do_QueryInterface(mStream); > + NS_ASSERTION(stream, "Wrapped stream is not serializable!"); Should this be a fatal assertion, since we'll crash at the dereference a few lines below? ::: netwerk/base/src/nsMIMEInputStream.cpp @@ +302,5 @@ > + > + if (mData) { > + nsCOMPtr<nsIIPCSerializableInputStream> stream = > + do_QueryInterface(mData); > + NS_ASSERTION(stream, "Wrapped stream is not serializable!"); This too.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2) > Should this be a fatal assertion, since we'll crash at the dereference a few > lines below? I don't want to get into that really. Right now this file only uses NS_ASSERTION, and whether those should be fatal or not is up to others. https://hg.mozilla.org/integration/mozilla-inbound/rev/41a3cdf92063
Backed out for Linux xpcshell permaorange. https://hg.mozilla.org/integration/mozilla-inbound/rev/9209d9af04d4 https://tbpl.mozilla.org/php/getParsedLog.php?id=14607998&tree=Mozilla-Inbound TEST-INFO | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit_ipc/test_post_wrap.js | running test ... TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit_ipc/test_post_wrap.js | test failed (with xpcshell return code: 0), see following log: >>>>>>> TEST-INFO | (xpcshell/head.js) | test 1 pending parent: TEST-INFO | (xpcshell/head.js) | test 2 pending parent: TEST-INFO | (xpcshell/head.js) | test 2 finished parent: TEST-INFO | (xpcshell/head.js) | running event loop child: CHILD-TEST-STARTED child: TEST-INFO | (xpcshell/head.js) | test 1 pending child: TEST-INFO | (xpcshell/head.js) | test 2 pending child: TEST-INFO | (xpcshell/head.js) | test 2 finished child: TEST-INFO | (xpcshell/head.js) | running event loop child: TEST-PASS | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js | [serverHandler : 79] POST == POST child: TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js | --AaB03x Content-Disposition: form-data; name="body" 0123456789 --AaB03x Content-Disposition: form-data; name="files"; filename="test_readline6.txt" Content-Type: application/octet-stream Content-Length: 4097 xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxE --AaB03x-- == - See following stack: child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_throw :: line 451 child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: _do_check_eq :: line 545 child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_check_eq :: line 566 child: JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/tests/netwerk/test/unit/test_post.js :: serverHandler :: line 91 child: native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0 child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 2312 child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1170 child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1618 child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1466 child: JS frame :: resource://testing-common/httpd.js :: <TOP_LEVEL> :: line 1335 child: native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0 child: TEST-INFO | (xpcshell/head.js) | exiting test child: CHILD-TEST-COMPLETED parent: TEST-INFO | (xpcshell/head.js) | test 1 finished parent: TEST-INFO | (xpcshell/head.js) | exiting test parent: TEST-INFO | (xpcshell/head.js) | No (+ 0) checks actually run child: TEST-PASS | ../unit/head_channels.js | [null : 147] 321 == 321 child: TEST-INFO | (xpcshell/head.js) | test 1 finished child: TEST-INFO | (xpcshell/head.js) | exiting test NOTE: child process received `Goodbye', closing down <<<<<<<
FYI, looks like Windows had issues too. https://tbpl.mozilla.org/php/getParsedLog.php?id=14612600&tree=Mozilla-Inbound TEST-INFO | c:\talos-slave\test\build\xpcshell\tests\netwerk\test\unit_ipc\test_post_wrap.js | running test ... command timed out: 1200 seconds without output, attempting to kill SIGKILL failed to kill process using fake rc=-1 program finished with exit code -1
Fixed it, https://hg.mozilla.org/integration/mozilla-inbound/rev/759b5d914905
(I forgot to add a case statement for MIMEInputStream to DeserializeInputStream())
https://hg.mozilla.org/mozilla-central/rev/759b5d914905
or
