[webkit-dev] Tightening up our include discipline

[Adam Barth]

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/20111018/b672931f/attachment.html>