RESOLVED FIXED 29351
Each wrapped Objective-C object should use a single RuntimeObjectImp 
https://bugs.webkit.org/show_bug.cgi?id=29351
Summary Each wrapped Objective-C object should use a single RuntimeObjectImp
Darin Adler
Reported 2009-09-17 16:34:22 PDT
When exposing Objective-C objects to JavaScript, we end up making multiple ObjcInstance objects and multiple RuntimeObjectImp objects for a single Objective-C object. This isn't good because it makes them receive multiple finalizeForWebScript calls.
Attachments
patch (17.91 KB, patch)
2009-09-17 16:35 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch v1 (6.60 KB, patch)
2009-09-18 12:29 PDT, Anders Carlsson 		no flags
DetailsFormatted DiffDiff
proposed patch (1.54 KB, patch)
2009-09-18 13:37 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2009-09-17 16:34:38 PDT
rdar://problem/7142294
Darin Adler
Comment 2 2009-09-17 16:35:43 PDT
Created attachment 39735 [details] patch
Sam Weinig
Comment 3 2009-09-17 19:23:40 PDT
Comment on attachment 39735 [details] patch r=me
Darin Adler
Comment 4 2009-09-18 10:49:23 PDT
http://trac.webkit.org/changeset/48513
Anders Carlsson
Comment 5 2009-09-18 12:29:31 PDT
Created attachment 39777 [details] Patch v1
Anders Carlsson
Comment 6 2009-09-18 12:30:17 PDT
Comment on attachment 39777 [details] Patch v1 Wrong bug, sorry!
David Kilzer (:ddkilzer)
Comment 7 2009-09-18 12:43:25 PDT
Comment on attachment 39735 [details] patch This one was landed.
Csaba Osztrogonác
Comment 8 2009-09-18 13:36:52 PDT
(In reply to comment #4) > http://trac.webkit.org/changeset/48513 Unfortunately this patch brake Qt build: http://build.webkit.org/builders/Qt%20Linux%20Release/builds/1693/steps/compile-webkit/logs/stdio The error caused the new virtual function added to runtime.h, but not defined in inherited class QtInstance : public Instance. 125 private: 126 virtual RuntimeObjectImp* newRuntimeObject(ExecState*); newRuntimeObject should have protected visibility, and createRuntimeObject should be renamed to newRuntimeObject in inherited class QtInstance.
Csaba Osztrogonác
Comment 9 2009-09-18 13:37:48 PDT
Created attachment 39785 [details] proposed patch
Eric Seidel (no email)
Comment 10 2009-09-18 13:55:20 PDT
Comment on attachment 39785 [details] proposed patch LGTM.
Eric Seidel (no email)
Comment 11 2009-09-18 13:55:38 PDT
I have to re-open the bug for the commit-queue to see it.
Darin Adler
Comment 12 2009-09-18 14:44:54 PDT
(In reply to comment #8) > 125 private: > 126 virtual RuntimeObjectImp* newRuntimeObject(ExecState*); > > newRuntimeObject should have protected visibility Why? I don't agree. But it's not important. > createRuntimeObject > should be renamed to newRuntimeObject in inherited class QtInstance. Yes, sorry! Thanks for fixing it.
WebKit Commit Bot
Comment 13 2009-09-18 15:12:25 PDT
Comment on attachment 39785 [details] proposed patch Clearing flags on attachment: 39785 Committed r48538: <http://trac.webkit.org/changeset/48538>
WebKit Commit Bot
Comment 14 2009-09-18 15:12:31 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 15 2013-12-19 10:05:13 PST
This change introduced incorrect lifetime management and cross-origin data leaks in WebKit1, see bug 123029 comment 8.
Note You need to log in before you can comment on or make changes to this bug.