Bug 20574 – bindings need better internal support for binding-object-to-webkit-c++-object mapping

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Luke Kenneth Casson Leighton]

the addition of the glib bindings to the DOM highlight clearly that
better support is needed for bindings.

you now have three bindings systems, each of which are duplicating
thousands of lines of code to provide, in their own special and
absolutely identical way:

* object type mapping on a per-object per-binding basis, to
  mirror Node::nodeType() onto e.g. GDOM_TYPE_XXX for glib

* object hash-mapping on a per-binding basis, to match unique
  creation of binding-objects onto webkit objects.

* a "wrapper" factory on a per-object-type per-binding basis,
  to actually _create_ the binding-objects of the right type,
  when the binding system doesn't know what type of webkit
  object it's being asked to create.

for the glib bindings, i've cut and paste and global-search-replaced
about _two thousand_ lines of JS wrapper factory and other code,
in order to duplicate the exact same required functionality
line-for-line, concept-for-concept, as can be seen in GDOMBinding.cpp
and GDOMHTMLElementWrapperfactory.cpp

to add support for SVG Document objects will require duplication of and global-search-replace modification of  another thousand or so lines of code.

it would be very simple to add:

* a templated version of the wrapper factory system

* a per-webkit-object pointer to a "private" wrapper class, which
  can contain a list of binding objects, covered and accessed by
  an enum.

  one single extra pointer is not that much space to save vs the
  _three_ (at the moment) comparatively inefficient and duplicated
  sets of hash-map code, each of which require "safety checks"
  to help deal with bugs (e.g. see addWrapper in JSDOMBindings.cpp)

  those bugs don't _exist_ if you have a pointer to a list of
  binding objects _in_ the webkit class: when the webkit class
  is delete()d the binding objects can be double-checked to be
  non-existent.

[Mark Rowe (bdash)]

Increasing the size of every DOM object by four/eight bytes is not the right way to address this.

[Luke Kenneth Casson Leighton]

have you considered doing a test evaluation - especially on AJAX web sites -
to get some statistics on how many entries there are in the various HashMaps?

bearing in mind that the HashMaps are... at least double the amount of memory of a single per-object pointer, then if over 50% of the c++ DOM model objects have javascript binding objects attached to them, you're not saving any memory at all, and adding complexity andd overhead to boot.

if the overhead of the HashMaps are _three_ times that of a single per-object pointer, then ...

so, i believe it may be worthwhile adding in some printfs into addWrapper in JSDOMBindings.cpp to print out the count of the number of HashMap entries, and also have a count printout of the number of c++ DOM objects.

even if it turns out that, even on heavy-javascript-based web sites such as gmail, yahoo mail and other sites, that the memory overhead of the HashMaps is significantly less than having a single pointer in the c++ DOM objects which goes unused (perhaps a lot of the time), the issue of my having to cut-and-paste and global-search-replace a good 1,500 line of code to get a corresponding object map for the glib bindinds remains.

[Anders Carlsson]

Regressing memory is a big no-no, so let's not do this.