[Webkit-unassigned] [Bug 90307] Standalone Devtools Module for Timeline

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90307


Michael Klepikov <klm at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |klm at google.com




--- Comment #9 from Michael Klepikov <klm at google.com>  2012-07-10 13:15:58 PST ---
(In reply to comment #4)
> (From update of attachment 150253 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150253&action=review
> 
> > Source/WebCore/inspector/front-end/TimelineEmbedded.js:6
> > +    this.timelineManager = new WebInspector.TimelineManager();
> 
> It sounds like too much context for the timeline. We should use some dependency injection scheme to workaround it better.

I think this is the crux of the argument, also per our email conversation: this patch should become a refactoring that makes the Timeline more modular, with more explicit dependencies. Gabriel has uploaded the next version of the patch, where he no longer modifies existing files, but replaces some of the dependent classes with his own inherited versions. The main purpose of that new patch version is to *identify* these Timeline dependencies and give an idea of what it would take to reduce the list below.

We don't know how the DevTools team would prefer to deal with injection, and we would like some guidance. Right now WebInspector is kind of a god object -- it knows about all the other objects in the system, and it makes them public, and the other classes access these objects through the WebInspector's public variables, not via their own members, making the dependencies implicit. I would think that ideally we may want to get away from this setup (please correct me if I'm wrong), but it would be too much of a sweeping change to do that in one shot. We need to go in smaller steps, and I'd like to get on the same page how exactly you would prefer us to do that.

We could for example, as Gabriel proposed, introduce constructors with arguments -- explicit dependency injection, and fall back to the current behavior if the args are undefined -- allowing the current system to continue to work while we begin to use injection in some scenarios.

Another possibility is a poor man's GUICE -- basically a map of class -> object, which would either be DevTools-global or belong to the WebInspector (which is more or less the same thing). The other objects, instead of "new", would ask the WebInspector to give them an instance of that class. The startup code would set up the map.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.