Bug 16778

Summary: Language specific bindings cannot cache/reuse previously created Instances and RuntimeObjects
Product: WebKit Reporter: Michael Goddard <michael.goddard>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch (adds caching for Qt bindings)
none
Updated patch - better Qt RuntimeObjectImp lifetime management andersca: review+

Description Michael Goddard 2008-01-07 16:13:00 PST
The static Instance::createRuntimeBinding* functions currently return a new Instance/RuntimeObjectImp for every request.  This means that JSObject equality will not work, and also creates unnecessary objects.  For the Qt port at least, we rely on object equality for connecting/disconnecting signals and slots.
Comment 1 Michael Goddard 2008-01-07 16:50:03 PST
Created attachment 18320 [details]
Proposed patch (adds caching for Qt bindings)
Comment 2 Michael Goddard 2008-01-08 16:53:42 PST
Hmm.. patch (id 18320) not quite complete - doesn't handle the case when an Instance has a ref from someone apart from the RuntimeObjectImp (so the RuntimeObjectImp can be deleted without the cache noticing, and reuses an old pointer).  New patch soon.
Comment 3 Michael Goddard 2008-01-08 17:42:35 PST
Created attachment 18337 [details]
Updated patch - better Qt RuntimeObjectImp lifetime management
Comment 4 Anders Carlsson 2008-01-10 16:24:03 PST
Comment on attachment 18337 [details]
Updated patch - better Qt RuntimeObjectImp lifetime management

>+    virtual BindingLanguage getBindingLanguage() const {return CLanguage;}

Need spaces after the open brace and before the close brace.

>+    virtual BindingLanguage getBindingLanguage() const {return JavaLanguage;}

Here too.

>+    virtual BindingLanguage getBindingLanguage() const {return ObjectiveCLanguage;}

Here too.

>+    virtual BindingLanguage getBindingLanguage() const {return QtLanguage;}

And here :)

This looks great otherwise, r=me.
Comment 5 Mark Rowe (bdash) 2008-01-13 03:09:19 PST
Landed in r29447 with tweaks to not break the Mac build.