[Webkit-unassigned] [Bug 19917] [XBL] We need the ability to manage bindings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=19917


eric at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22119|review?                     |review-
               Flag|                            |




------- Comment #5 from eric at webkit.org  2008-07-23 13:31 PDT -------
(From update of attachment 22119)
static XBLBindingManager* manager;
 37 
 38 XBLBindingManager* XBLBindingManager::sharedInstance()
 39 {
 40     if (!manager)
 41         manager = new XBLBindingManager();
 42 
 43     return manager;
 44 }


should probably just chagne to:

sharedInstance()
{
   static XBLBindingManager manager;
   return &manager;
}

Um... just fix it:
 57     // FIXME: we can add the same binding several times.
Seems like it would be easy to walk the bindings.

Why do these XBLBindings need to be new'd?  Can'd you just use:
Vector<XBLBinding> ?

Also, as much as I hate DeprecatedValueList, I think we might want to do this
with a list structure.  I'm not sure how fast we need removes to be... but with
a vector they'll be quite slow.
I see these as quite similar to Event listeners.

Overall, I think this is fine, but I think that this should at least be:
Vector<XBLBinding>
instead of:
Vector<XBLBinding*>
for easier memory management.

You could also just use:
HashMap<String, Element*>

Why wouldn't this just be:
HashMap<Element*, Vector<std:pair<String, Element*> > >

Yeah, this just seems a bit over-designed, when all you need is a simple hash
to map from elements to a list of uris.  XBLBinding could/should probably just
be a private class on XBLbinding manager.

we'll talk about this over irc.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.