gin/data_object_builder.cc - Issue 2845463002: gin: Create a DataObjectBuilder class to help create simple data objects.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: gin/data_object_builder.cc

Issue 2845463002: gin: Create a DataObjectBuilder class to help create simple data objects. (Closed)

Patch Set: data_object_builder.cc Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #include "gin/data_object_builder.h"

	6

	7 namespace gin {

	8

	9 DataObjectBuilder::DataObjectBuilder(v8::Isolate* isolate)

	10 : isolate_(isolate),

	11 context_(isolate->GetCurrentContext()),
	Devlin 2017/04/26 15:50:31 Would it be better to pass in the context and get Would it be better to pass in the context and get the isolate from that, rather than the other way around? That way we have an explicit context for the SetDataProperty() calls. jbroman 2017/04/26 17:47:31 It's a bit of a lie because v8::Object::New alread Show quoted text On 2017/04/26 at 15:50:31, Devlin wrote: > Would it be better to pass in the context and get the isolate from that, rather than the other way around? That way we have an explicit context for the SetDataProperty() calls. It's a bit of a lie because v8::Object::New already uses the current context. To make that properly true we'd need a v8::Context::Scope, but the caller is in a good position to ensure that one is in place (and probably already has). I don't feel strongly, so I could budge on this if you feel strongly. Devlin 2017/04/26 18:30:23 Ah, good point. This is fine. On that note, naiv Show quoted text On 2017/04/26 17:47:31, jbroman wrote: > On 2017/04/26 at 15:50:31, Devlin wrote: > > Would it be better to pass in the context and get the isolate from that, > rather than the other way around? That way we have an explicit context for the > SetDataProperty() calls. > > It's a bit of a lie because v8::Object::New already uses the current context. To > make that properly true we'd need a v8::Context::Scope, but the caller is in a > good position to ensure that one is in place (and probably already has). > > I don't feel strongly, so I could budge on this if you feel strongly. Ah, good point. This is fine. On that note, naively, it seems to me we should have a version of v8::Object::New that takes a context, since most things that result in context-related happenings - getting/setting/executing - now seem to. jochen@, can you shed some light on why we don't do this?
	12 object_(v8::Object::New(isolate)) {}

	13

	14 } // namespace gin

OLD	NEW

« gin/data_object_builder.h ('K') | « gin/data_object_builder.h ('k') | gin/data_object_builder_unittest.cc » ('j') | gin/data_object_builder_unittest.cc » ('J')