Adding instrumentation for new events is currently very cumbersome, as it requires a lot of plumbing. Currently, a new timeline event adds at 3-6 new methods. Plumbing events from other modules (e.g. platform or embedder) or from different threads is event more inconvenient. It is suggested to use event-bases system for delivering instrumentation events from call site to consumer (e.g. inspector or other tools, such as event tracer). This would also let us to unify call sites with event tracer in the future.
Created attachment 174652 [details] Proof of concept implementation -- just several timeline events replaced
Comment on attachment 174652 [details] Proof of concept implementation -- just several timeline events replaced Attachment 174652 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14857634
Comment on attachment 174652 [details] Proof of concept implementation -- just several timeline events replaced Attachment 174652 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14878058
Comment on attachment 174652 [details] Proof of concept implementation -- just several timeline events replaced Clearing r? as this requires long offline discussions.
Summary of offline discussion with Andrey: Pavel: I don't like the fact that there is no place to look up the event signature Andrey: I agree, but what are our options? Pavel: I don't see the necessity of the intermediate data format, I'd rather pass raw types to the recipients and let them serialize it all. Andrey: I can live with that Pavel suggests: Frankly speaking, I don't like macro definitions and I think that creating classes for events is overengineering. Can we keep things simple and define an interface per instrumentation module (much like PlatformInstrumentation and InspectorInstrumentation)? I don't think we need to support multiple clients at a time. Yes, it does require putting together stupid lines of code such as inline void PlatformInstrumentation::willDecodeImage(const String& imageType) { FAST_RETURN_IF_NO_CLIENT(); m_client->willDecodeImage(imageType); } But is it that bad after all? A method in the interface, a delegating guard method like this and we are good to go? Or should we define these in the IDL files and generate this delegating thing out of it?