Bug 63925 - [V8][Chromium] Remove use of OwnHandle from V8LocalContext
Summary: [V8][Chromium] Remove use of OwnHandle from V8LocalContext
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-05 01:17 PDT by Hans Wennborg
Modified: 2011-07-05 15:18 PDT (History)
3 users (show)

See Also:

Attachments
Patch (2.47 KB, patch)
2011-07-05 01:22 PDT, Hans Wennborg 		no flags 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 Hans Wennborg 2011-07-05 01:17:32 PDT 
[V8][Chromium] Remove use of OwnHandle from V8LocalContext
Comment 1 Hans Wennborg 2011-07-05 01:22:44 PDT 
Created attachment 99677 [details]
Patch
Comment 2 Hans Wennborg 2011-07-05 01:24:51 PDT 
Uploaded as per the discussion in Bug 62345.
Comment 3 Dmitry Lomov 2011-07-05 11:36:35 PDT 
(In reply to comment #2)
> Uploaded as per the discussion in Bug 62345.

LGTM.

Just to note, it is still unclear why OwnHandle does not do the job - I will investigate this more.

> Source/WebCore/bindings/v8/V8Utilities.cpp:52
> +    : m_context(v8::Context::New())

It does not matter one bit currently, but logically we want to do any V8 operations (including newing up the context) after we ensure that V8BindingPerIsolateData is initialized.
Comment 4 Adam Barth 2011-07-05 14:47:37 PDT 
> It does not matter one bit currently, but logically we want to do any V8 operations (including newing up the context) after we ensure that V8BindingPerIsolateData is initialized.

So you're saying we shouldn't do this in the initializer list?
Comment 5 WebKit Review Bot 2011-07-05 15:13:55 PDT 
Comment on attachment 99677 [details]
Patch

Clearing flags on attachment: 99677

Committed r90407: <http://trac.webkit.org/changeset/90407>
Comment 6 WebKit Review Bot 2011-07-05 15:13:59 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Dmitry Lomov 2011-07-05 15:18:54 PDT 
(In reply to comment #4)
> > It does not matter one bit currently, but logically we want to do any V8 operations (including newing up the context) after we ensure that V8BindingPerIsolateData is initialized.
> 
> So you're saying we shouldn't do this in the initializer list?

In the Grand Scheme Of Things (TM), yes, but it does not matter as of today.