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+

Michael Goddard
Reported 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.
Attachments
Proposed patch (adds caching for Qt bindings) (11.67 KB, patch)
2008-01-07 16:50 PST, Michael Goddard
no flags
Updated patch - better Qt RuntimeObjectImp lifetime management (12.84 KB, patch)
2008-01-08 17:42 PST, Michael Goddard
andersca: review+
Michael Goddard
Comment 1 2008-01-07 16:50:03 PST
Created attachment 18320 [details] Proposed patch (adds caching for Qt bindings)
Michael Goddard
Comment 2 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.
Michael Goddard
Comment 3 2008-01-08 17:42:35 PST
Created attachment 18337 [details] Updated patch - better Qt RuntimeObjectImp lifetime management
Anders Carlsson
Comment 4 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.
Mark Rowe (bdash)
Comment 5 2008-01-13 03:09:19 PST
Landed in r29447 with tweaks to not break the Mac build.
Note You need to log in before you can comment on or make changes to this bug.