Summary: | Each wrapped Objective-C object should use a single RuntimeObjectImp | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||
Component: | WebKit API | Assignee: | Darin Adler <darin> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, commit-queue, ddkilzer, eric, ossy, sam | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Darin Adler
2009-09-17 16:34:22 PDT
Created attachment 39735 [details]
patch
Comment on attachment 39735 [details]
patch
r=me
Created attachment 39777 [details]
Patch v1
Comment on attachment 39777 [details]
Patch v1
Wrong bug, sorry!
Comment on attachment 39735 [details]
patch
This one was landed.
(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. Created attachment 39785 [details]
proposed patch
Comment on attachment 39785 [details]
proposed patch
LGTM.
I have to re-open the bug for the commit-queue to see it. (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. Comment on attachment 39785 [details] proposed patch Clearing flags on attachment: 39785 Committed r48538: <http://trac.webkit.org/changeset/48538> All reviewed patches have been landed. Closing bug. This change introduced incorrect lifetime management and cross-origin data leaks in WebKit1, see bug 123029 comment 8. |