On Wed, Dec 21, 2011 at 11:08 PM, Darin Adler <[email protected]> wrote:
> One of the main things the Element class does is implement an attribute > store for elements. Currently, this attribute store is in a separate > reference-counted object. That object is exposed to the DOM as NamedNodeMap. > > But I believe this is not a good pattern for the long run. The reason that > separate object exists with a reference count on it is that it is exposed > as part of the DOM API, to provide something we can return from the > attributes function. We don’t want our in-memory implementation of elements > to be tied to the fact that occasionally a client might want access to the > reference counted map. > A much better factoring is to turn NamedNodeMap into a wrapper for the > Element. The actual attribute mapping should be elsewhere. Either in > Element itself, or some other helper class. > > I’ve seen patches moving more of the attribute logic to NamedNodeMap, and > I am concerned that this is not a step in the right direction. > The patches you have in your mind are probably ones authored by Adam (Klein) and reviewed by me. I agree with your long-term goal here but the immediate problem we have today is that code that updates and notifies attributes are scattered all over the place, and making refactoring harder. For example, prior to http://trac.webkit.org/changeset/103452/, calls to willModifyAttribute and attributeChanged/dispatchSubtreeModifiedEvent were made in different functions. All these patches are attempts to make the interfaces to obtain and update attributes more consistent and small so that we can do refactorings like turning NamedNodeMap into an Element wrapper easy in the future. For example, once we can land the patch for https://bugs.webkit.org/show_bug.cgi?id=75054 all calls to above functions will be done in 1-2 places, we'll be able to start moving code from NamedNodeMap to Element if we wish. Hopefully that addresses your concern. One of the first things we should do is to remove the separate reference > count stored in each NamedNodeMap. That can be done immediately by using > OwnPtr instead of RefPtr to hold it on each Element, and changing the ref > and deref functions to simply ref and deref the owning element instead of > the map itself. > > But longer term we want to get rid of other unneeded overhead. For > example, the back pointer from the heap-allocated vector of attributes to > the element is a waste of space. To do that, I think we want to implement > NamedNodeMap.idl on a class that does nothing but pass the calls through. > These are good ideas! Could you file bugs for these goals so that Adam and I can tackle those after the break? - Ryosuke
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
