Ben, please make sure to ping Cameron when we're got reverse CPOWs or whatever else might make JS gopher work. (Do we have a bug open for reverse CPOW?)
Since Joe recommended starting with a test suite, it seems most logical to me to do such a thing through the xpcshell test harness and create a small gopher server similar to httpd.js to test against. Would that be the best way? I don't think this would be a good fit for mochitest since it seems HTTP-centric.
> xpcshell test harness and create a small gopher server similar to httpd.js Yes, that sounds like a good plan to me.
Created attachment 449575 [details] [diff] [review] Work in progress, Gopher e10s WIP on Gopher in e10s. The attached test case does work for 1.9.2 but I must be missing something because xpcshell-test just freezes up waiting for a signal I must not be sending. What am I missing here? This is a combination of both the existing code for HttpChannel* and FtpChannel*.
Created attachment 450296 [details] [diff] [review] WIP attempt 2 (in line with 536289) WIP 2. Updated to review of 536289 since this is based on it. Still can't get unit test to work: it hangs when it asks the ioService for a channel. What did I do wrong here?
Problem #1: You're using the RefcountHitZero method (which is pretty self-describing), but then calling Release() in DeallocPGopherChannel. That's wrong; you should be deleting the pointer instead. As it stands right now, you're still not releasing the IPDL actor at any point, so that code will never run. Problem #2: You haven't changed the nsGopherHandler::NewProxiedChannel code to hand back a GopherChannelChild in the content process, so none of the IPDL work ever happens. This is the cause of the ioService hang.
I think I get it now. Respin coming up. Thanks!
Created attachment 450460 [details] [diff] [review] Working patch with simple unit test With Josh's suggested changes. This passes the attached unit test.
Created attachment 450568 [details] [diff] [review] Revised working patch with simple unit test Whoops, repaired reference leak. Also made similar nitfixes requested in 536289.
We don't support gopher anymore, do we?
We hope to support Gopher in JS, but we're never going to implement it in C++, and that code's been removed from non-e10s.
Right you are!
Actually, I would think that bug 590682 (especially Honza's patch, if I understand it correctly) should cover this case for any protocol, including Gopher.
Comment on attachment 450568 [details] [diff] [review] Revised working patch with simple unit test Clearing review request, since this has been dupe'd to another bug. Please reopen or file a new bug if this issue is still somehow no covered by the bug it's been dupe'd to.