I think we should implement IPDL `discard` semantics, and I've attached a patch to bug 532208 which uses them to make PBrowserStream stateful. At least for now, I don't think we should allow anything to race with __delete__, so it requires two-phase destruction states, but that's not a big deal.

(In reply to comment #0) > At least for now, I don't think we should allow anything to race with > __delete__, so it requires two-phase destruction states, but that's not a big > deal. Wholeheartedly agreed. To support this we'd need something like bug 532044.

One obstacle to implementing |discard| for sync and rpc messages is what discarding them should entail. The original proposal on m.d.t.dom was limited to async only, where discard just means not invoking the Recv*() handler. I think if at all possible, we should *not* invoke Recv*() or Answer*() handlers for these discarded messages. But, because the other side expects a return message, we probably need to communicate discarded-ness back across. Initially I see a few ways to do this. (1) Return a |false| error code back to C++ Send/Recv*() handlers. This should work just fine on the parent side, but children are currently allowed to assume a |true| error code and thus could ignore discarded-ness and return garbage return-values to callers. One way to work around this is to make "normal" messages |void Foo()| and |discard| messages |bool Foo() __attribute__((warn_unused_result))|. (2) For possibly-|discard|ed sync/rpc messages, emit the method signature bool Foo(..., bool* discarded); This has the advantage of allowing the error check |if (!Send*(...)| to still work, and makes it easy to catch child code that ignores |discarded|. The downside is that this outparam gives Send/Call methods 4 possible "return statuses", when only three are valid: true+discard=false, false+discard=x, true+discard=true. (3) For possibly-|discard|ed sync/rpc messages, emit the method signature Status Send/Call*(...) __attribute__((warn_unused_result)); for both parents and children, where |Status| is a C++ class wrapping |enum eStatus { OK, Error, Discard };|. (|Status| can't be a bare enum because gcc allows coercing enums to bools, which could lead to nasty bugs.) I kind of prefer (3), but not particularly strongly. A second obstacle to |discard|ed sync/rpc messages is determining "fallback" values to use for outparams returned to foreign C/C++ code that ends up sending the possibly-discarded message. This will definitely have to be taken case by case, but from what I've seen of NPAPI+major plugins it usually seems safe to return NP_ERR_NO_ERROR with NULL or 0 outparams. Some things like NPN_RequestRead() are even easier, as we can just ignore the request. We might be able to crib some of this from nsNPAPI*.

Per workweek discussion, we're going to go with a modified version of 3) where all methods (whether or not they are discardable) return an C++ wrapper around an enum which doesn't auto-convert to bool. Then client code will be changed to if (IPDLOK != SendSomeMessage()) { // failure mode here } cjones, does that match your recollection of our discussion?

Yes. Also, in addition to { OK, ERROR, DISCARD }, I think we'll need EINTR. But more on that next week! ;)