[webkit-dev] Moving code from Element to NamedNodeMap -- I think that is the wrong direction

[Ryosuke Niwa]

On Wed, Dec 21, 2011 at 11:08 PM, Darin Adler <darin at apple.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20111221/cde312a1/attachment.html>