[webkit-dev] Tightening up our include discipline

[Sam Weinig]

I'd be interested in performance cost to that, especially on a test that creates a lot of JS wrapped Events.

-Sam

On Oct 18, 2011, at 6:42 PM, Adam Barth wrote:

> My thought on that is to use a single virtual function and return an atomic string, like we do for elements.
> 
> Adam
> On Oct 18, 2011 6:10 PM, "Sam Weinig" <weinig at apple.com> wrote:
> For the specific case of Event, we do need some sort of poor man's RTTI here for the sake of the bindings, but we could consider alternatives.  Since you need to identify the subclasses, you need to either use many virtual functions, as we do currently for Event, or have some tagging mechanism, which we do for classes like Element and EventListener.
> 
> If we use a tagging technique, we need someway to ensure that the tags are unique. In EventListener, we list the types up front in an enum, though this has same downside as the many virtual functions in that the tendrils of WebAudio reach into the base class.  For Element, we rely on qualified names, which doesn't require the icky reach, but are slightly more costly.  (You still end up with the tendrils in the JS base class).
> 
> Of course, it might make sense to solve the issue of #ifdefs in Event by simply removing the #ifdefs in Event, since they are just base class implementations. The only downside of that is if we later remove the feature, it is harder to find all the places it touched.
> 
> -Sam
> 
> On Oct 18, 2011, at 4:37 PM, Adam Barth wrote:
> 
> > The other day, Sam chided me on a bug for including a header from
> > WebCore proper in a file in WebCore/platform.  Even though I know I'm
> > not supposed to do that, it's an easy mistake to make.  There's some
> > work going on in https://bugs.webkit.org/show_bug.cgi?id=49192 to
> > improve the style checker to flag these sorts of bad dependencies.
> > This check isn't live yet, but don't be surprised if some robotic user
> > complains about a bad include in one of your patches.  (As always, if
> > the bot complains incorrectly, please let me know so we can eliminate
> > false positives.)
> >
> > Relatedly, as part of the recent ENABLE cleanup effort, I noticed that
> > some files (e.g.,
> > http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Event.cpp)
> > attract lots of ifdefs because many unrelated features need to
> > register themselves.  I was thinking it might be an improvement to
> > restructure things a bit so that, for example, WebAudio wouldn't need
> > to reach its fingers into Event.cpp and instead could be better
> > contained in the WebCore/webaudio directory.
> >
> > I welcome any thoughts you have on either of these topics.
> >
> > Thanks,
> > Adam
> > _______________________________________________
> > webkit-dev mailing list
> > webkit-dev at lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20111019/a5949bbb/attachment.html>