Bug 20621 - Add Peerable.h and make Node inherit from Peerable
Summary: Add Peerable.h and make Node inherit from Peerable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 20619
  Show dependency treegraph
 
Reported: 2008-09-03 04:27 PDT by Eric Seidel (no email)
Modified: 2010-04-21 13:12 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-09-03 04:27:58 PDT
Add Peerable.h and make Node inherit from Peerable

V8 bindings require that all bound objects have a peer pointer.  Currently this is done via a hack in RefCounted.h and TreeShared.h in Chromium:
http://src.chromium.org/svn/trunk/src/webkit/pending/wtf/RefCounted.h
http://src.chromium.org/svn/trunk/src/webkit/pending/TreeShared.h

This should be split out into a separate Peerable.h header file, and Node (and anything else which needs to be bound in V8) needs to inherit from it.  RefCounted and TreeShared should not include Peerable, and Peerable need not be virtual, it could be a template.
Comment 1 Sam Weinig 2008-09-07 14:51:34 PDT
I do not think it makes sense to have a Peerable base class for Node or other bound object base classes, at least without conclusive evidence that the memory hit is worth it in performance gain.  Even if that issue can be reconciled, bound objects like Node can be bound to multiple binding languages simultaneously (JS and ObjC for instance) and the Peerable class does not account for that.  If we find that the perf gain large enough, perhaps we can consider using it as a fast path for JS, and relegating the other binding languages to their current HashMap based caching.  If that is the case, we should rename Peerable to something that connotes that it is JavaScript specific.
Comment 2 Dave Hyatt 2008-09-07 14:54:34 PDT
We certainly wouldn't want it for non-V8.  I assume Eric is saying it would still only be defined for V8.

Comment 3 Sam Weinig 2008-10-29 15:05:22 PDT
I don't think this is the correct usage of the GoogleBug, which is really meant to be a bug in a high profile google web product and not a Chromium issue.
Comment 4 Alexey Proskuryakov 2010-04-21 13:12:16 PDT
It's not WONTFIX, it's FIXED. I believe that ScriptWrappable is the new Peerable:

class Node : public EventTarget, public TreeShared<Node>, public ScriptWrappable