Bug 29351

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:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

All

Attachments:

Description	Flags
patch	none
Patch v1	none
proposed patch	none

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.