This should allow releasing the UI thread from some load. I have a work in progress patch for this (though not ready for review yet).

Since this greatly improves visual performance, I think we should block 1.0 on it.

This patch implements asynchronous ical parsing up to the point where calEvent/calTodo objects are created. I haven't found a clean way to create the latter without running into deadlocks.

Comment on attachment 378070 [details] [diff] [review] patch - v1 >- calICSServiceConstructor, >- NULL, >- NULL, >- NULL, >- NS_CI_INTERFACE_GETTER_NAME(calICSService), >- NULL, >- &NS_CLASSINFO_NAME(calICSService) Why no class info? >+ icsParser.parseString(data, null, null); >+ parser.parseString(data, null, null); >+ parser.parseString(str, null, null); I must have missed this file on review, but I assume you changed calIIcsParser.idl somewhere. Instead of adding another null argument, I'd suggest to use the idl [optional] tag for the latter two arguments. This way you are not forced to provide the argument each time. >+ let worker = { // nsIRunnable: >+ run: function worker_run() { >+ let res = null; >+ try { >+ actionFunc(callingThread); >+ } catch (exc) { >+ res = exc; >+ } >+ >+ if (!respFunc) { >+ return; >+ } >+ >+ let response = { // nsIRunnable: >+ run: function response_run() { >+ respFunc(res); >+ } This can be shortende a small bit using the new function syntax of js 1.7 or 1.8: let response = { run: function() respFunc(res) }; >+ void onParsingDone(in nsresult rc, in calIIcsParser parser); Only a small nit, but lets align this with our other listeners: onParsingComplete. > void parseString(in AString aICSString, >- in calITimezoneProvider aTzProvider); >+ in calITimezoneProvider aTzProvider, >+ in calIIcsParsingListener aAsyncParsing); Is there any way to interupt this? i.e what happens if the current xul window is closed and the aAsyncParsing listener goes out of context? >+var gIcsParserClassInfo = { > }; Maybe you can move the class info into the component itself instead of providing an extra object. >+ // Do the actual string reading and ical parsing on a athread, but process the parsed ical s/athread/thread/ r=philipp, looking forward to this patch!

(In reply to comment #3) > (From update of attachment 378070 [details] [diff] [review]) > >- calICSServiceConstructor, > >- NULL, > >- NULL, > >- NULL, > >- NS_CI_INTERFACE_GETTER_NAME(calICSService), > >- NULL, > >- &NS_CLASSINFO_NAME(calICSService) > Why no class info? covered by NS_IMPL_CI_INTERFACE_GETTERn > >+ icsParser.parseString(data, null, null); > >+ parser.parseString(data, null, null); > >+ parser.parseString(str, null, null); > I must have missed this file on review, but I assume you changed > calIIcsParser.idl somewhere. Instead of adding another null argument, I'd > suggest to use the idl [optional] tag for the latter two arguments. This way > you are not forced to provide the argument each time. good idea, changed latter two args to be optional > >+ void onParsingDone(in nsresult rc, in calIIcsParser parser); > Only a small nit, but lets align this with our other listeners: > onParsingComplete. done > > void parseString(in AString aICSString, > >- in calITimezoneProvider aTzProvider); > >+ in calITimezoneProvider aTzProvider, > >+ in calIIcsParsingListener aAsyncParsing); > Is there any way to interupt this? i.e what happens if the current xul window > is closed and the aAsyncParsing listener goes out of context? I cannot imagine a current situation where this would happen, because the async API is by now only used from within the ics provider. However, such calls would bounce which ~might~ be OK since the program will not break. We might suppress errors logs or even opt for additional API. Should we cover further optimizations (caldav, wcap) in follow-up bugs? Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/aaa44b859559> -> FIXED

> Should we cover further optimizations (caldav, wcap) in follow-up bugs? Yes, please file some as needed.

I think this checkin regressed Bug 494783.