Bug 16778 - Language specific bindings cannot cache/reuse previously created Instances and RuntimeObjects
Summary: Language specific bindings cannot cache/reuse previously created Instances an...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-07 16:13 PST by Michael Goddard
Modified: 2008-01-13 03:09 PST (History)
1 user (show)

See Also:


Attachments
Proposed patch (adds caching for Qt bindings) (11.67 KB, patch)
2008-01-07 16:50 PST, Michael Goddard
no flags Details | Formatted Diff | Diff
Updated patch - better Qt RuntimeObjectImp lifetime management (12.84 KB, patch)
2008-01-08 17:42 PST, Michael Goddard
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.