Bug 31873

Summary:

[V8] Avoid using JavaScript objects as context data

Product:

WebKit

Reporter:

Søren Gjesse <sgjesse>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, dglazkov, pfeldman, sgjesse, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Use string instead if JavaScript object for context "data"	pfeldman: review-
Updated patch	none

Søren Gjesse

Reported 2009-11-25 07:35:58 PST

The V8 API provides the ability to associate a context with a "data" object. If a context dependent object is used this ha the side effect of keeping the context alive for some time after the page using the context has been closed. To avoid this the context "data" object should be a string which is not context dependent. In the V8 API the type will be changed from Object to String in the near future. See http://crbug.com/23058.

Attachments
Use string instead if JavaScript object for context "data" (4.14 KB, patch) 2009-11-25 07:55 PST, Søren Gjesse	pfeldman: review-	Details Formatted Diff Diff
Updated patch (4.63 KB, patch) 2009-11-26 00:01 PST, Søren Gjesse	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Søren Gjesse

Comment 1 2009-11-25 07:55:31 PST

Created attachment 43847 [details] Use string instead if JavaScript object for context "data"

Søren Gjesse

Comment 2 2009-11-25 08:28:27 PST

This should not be committed before http://codereview.chromium.org/443002 has been committed in Chromium.

Pavel Feldman

Comment 3 2009-11-25 08:40:13 PST

Comment on attachment 43847 [details] Use string instead if JavaScript object for context "data" > -const char* V8Proxy::kContextDebugDataType = "type"; > -const char* V8Proxy::kContextDebugDataValue = "value"; Remove these from .h as well? Otherwise r+.

Yury Semikhatsky

Comment 4 2009-11-25 08:41:53 PST

Looks good to me. (In reply to comment #1) > Created an attachment (id=43847) [details] > Use string instead if JavaScript object for context "data"

Søren Gjesse

Comment 5 2009-11-26 00:01:31 PST

Created attachment 43900 [details] Updated patch Removed unused members from V8Proxy.h

Søren Gjesse

Comment 6 2009-11-26 00:08:14 PST

(In reply to comment #3) > (From update of attachment 43847 [details]) > > -const char* V8Proxy::kContextDebugDataType = "type"; > > -const char* V8Proxy::kContextDebugDataValue = "value"; > > Remove these from .h as well? Otherwise r+. Done.

WebKit Commit Bot

Comment 7 2009-11-26 00:43:49 PST

Comment on attachment 43900 [details] Updated patch Clearing flags on attachment: 43900 Committed r51407: <http://trac.webkit.org/changeset/51407>

WebKit Commit Bot

Comment 8 2009-11-26 00:43:54 PST

All reviewed patches have been landed. Closing bug.

Dimitri Glazkov (Google)

Comment 9 2009-12-07 09:52:55 PST

This change has a flaw in logic. It removes the ability to set debug context id for a newly created context. The early return: if (debugId == -1) return false; causes V8Proxy::setInjectedScriptContext to always return false for newly created worlds with id > 0. I'll fix.

Note You need to log in before you can comment on or make changes to this bug.