WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch v1
(6.60 KB, patch)
2009-09-18 12:29 PDT
,
Anders Carlsson
no flags
Details
Formatted Diff
Diff
proposed patch
(1.54 KB, patch)
2009-09-18 13:37 PDT
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug