Bug 94450 - [V8] Remove V8Proxy from V8IsolatedContext
Summary: [V8] Remove V8Proxy from V8IsolatedContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 85330
  Show dependency tree / graph
 
Reported: 2012-08-20 01:06 PDT by Kentaro Hara
Modified: 2012-08-20 17:07 PDT (History)
3 users (show)

See Also:

Attachments
Patch (5.46 KB, patch)
2012-08-20 01:07 PDT, Kentaro Hara 		webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-08-20 01:06:14 PDT 
We can remove dependency on V8Proxy from V8IsolatedContext.
Comment 1 Kentaro Hara 2012-08-20 01:07:32 PDT 
Created attachment 159352 [details]
Patch
Comment 2 Kentaro Hara 2012-08-20 01:10:30 PDT 
Comment on attachment 159352 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159352&action=review

> Source/WebCore/bindings/v8/ScriptController.cpp:198
> +                isolatedContext = new V8IsolatedContext(m_frame, extensionGroup, worldID);

Should V8IsolatedContext be a RefPtr? If so, I'll write a follow-up fix to make it reference-countable.
Comment 3 Adam Barth 2012-08-20 11:12:54 PDT 
Comment on attachment 159352 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159352&action=review

>> Source/WebCore/bindings/v8/ScriptController.cpp:198
>> -                isolatedContext = new V8IsolatedContext(proxy(), extensionGroup, worldID);
>> +                isolatedContext = new V8IsolatedContext(m_frame, extensionGroup, worldID);
> 
> Should V8IsolatedContext be a RefPtr? If so, I'll write a follow-up fix to make it reference-countable.

Its lifetime is managed by the V8 garbage collector.  If you see a clean way to do that with RefPtr, go for it.
Comment 4 WebKit Review Bot 2012-08-20 14:59:10 PDT 
Comment on attachment 159352 [details]
Patch

Rejecting attachment 159352 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
eeded at 1 with fuzz 3.
patching file Source/WebCore/bindings/v8/ScriptController.cpp
patching file Source/WebCore/bindings/v8/V8IsolatedContext.cpp
Hunk #2 FAILED at 64.
1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/bindings/v8/V8IsolatedContext.cpp.rej
patching file Source/WebCore/bindings/v8/V8IsolatedContext.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13543490
Comment 5 Kentaro Hara 2012-08-20 16:57:55 PDT 
Committed r126096: <http://trac.webkit.org/changeset/126096>