The silent failures I keep getting are starting to make me very sad. Digging through debug console output is not a fun process.
Created attachment 429369 [details] [diff] [review] WIP basic remoting of nsConsoleService::LogMessage This just does a straight proxy of LogMessage, so everything shows up in the parent as a generic console message. This is less than ideal, but gets the job done for the time being.
Note: patch depends on the build tier unification in bug 528250.
Created attachment 429383 [details] [diff] [review] WIP proper remoting of script errors This is better, and actually gets the job done. I think I can one up it, though.
Created attachment 429386 [details] [diff] [review] Delegate message remoting to each message type Unlike previous patches, messages now remote themselves. The system appears to work flawlessly.
Hmm, there's something fishy happening on shutdown where the child process aborts when trying to send a script error. I'm looking into that; I would still appreciate a review of the current patch because I believe the method is sound.
Hmm. So a few questions: 1) Wouldn't it make more sense to do the sending in the console service instead? 2) LogMessage can happen on an arbitrary thread. Can the SendRemoteMessage stuff run on an arbitrary thread?
No, IPDL methods have to be main-thread only.
Another option is to add a console listener in the child process which sends stuff over to the parent process...
Just in terms of where to put this code, that is. The main drawback is it would have to know about the kinds of console messages that can happen...
I've got a new version which dispatches an event to the main thread to trigger the IPDL message. In my mind, forcing message types to be aware of how to remote themselves is cleaner than an all-knowing listener or service. It also reduces boilerplate significantly for things like nsScriptError (ie. rv = message->GetX(&x); NS_ENSURE_SUCCESS(rv, rv)). However, if you'd prefer the omnipotent version, I can whip that up as well.
Created attachment 442909 [details] [diff] [review] Patch This is effectively the same patch as previously, but now doesn't break when messages are logged from arbitrary threads.
+++ b/dom/ipc/Makefile.in -I$(topsrcdir)/chrome/src \ + -I$(srcdir)/../../xpcom/base \ I'd say + -I$(topsrcdir)/xpcom/base \ makes more sense.
So with this patch, console listeners in the content process see no messages. Is that desired? If not, it seems cleaner to allow listeners but have a listener that calls SendRemoteMessage. Listeners _are_ notified on the main thread (the console service handles this), and that should make things Just Work. Might even be less boilerplate...
Comment on attachment 442909 [details] [diff] [review] Patch New patch coming up.
Created attachment 477531 [details] [diff] [review] Remote console messages from content to chrome via a listener.
Comment on attachment 477531 [details] [diff] [review] Remote console messages from content to chrome via a listener. >+++ b/dom/ipc/ContentChild.cpp >+ nsAutoPtr<char> category; >+ char *tmp; No, this is totally broken. This will deallocate with delete memory that was allocated with some other allocator. The right way to do this is: nsXPIDLCString category; ..... rv = scriptError->GetCategory(getter_Copies(category)); r=me with that fixed
Created attachment 478788 [details] [diff] [review] Remote console messages from content to chrome via a listener. Incorrect string usage addressed.