Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2c2892b

Browse filesBrowse files
joyeecheungrichardlau
authored andcommitted
src: set ModuleWrap internal fields only once
There is no need to initialize the internal fields to undefined and then initialize them to something else in the caller. Simply pass the internal fields into the constructor to initialize them just once. PR-URL: #49391 Backport-PR-URL: #51004 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 425e011 commit 2c2892b
Copy full SHA for 2c2892b

File tree

Expand file treeCollapse file tree

2 files changed

+26
-18
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+26
-18
lines changed
Open diff view settings
Collapse file

‎src/module_wrap.cc‎

Copy file name to clipboardExpand all lines: src/module_wrap.cc
+23-17Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,22 @@ using v8::Value;
5252
ModuleWrap::ModuleWrap(Environment* env,
5353
Local<Object> object,
5454
Local<Module> module,
55-
Local<String> url)
56-
: BaseObject(env, object),
57-
module_(env->isolate(), module),
58-
id_(env->get_next_module_id()) {
55+
Local<String> url,
56+
Local<Object> context_object,
57+
Local<Value> synthetic_evaluation_step)
58+
: BaseObject(env, object),
59+
module_(env->isolate(), module),
60+
id_(env->get_next_module_id()) {
5961
env->id_to_module_map.emplace(id_, this);
6062

61-
Local<Value> undefined = Undefined(env->isolate());
6263
object->SetInternalField(kURLSlot, url);
63-
object->SetInternalField(kSyntheticEvaluationStepsSlot, undefined);
64-
object->SetInternalField(kContextObjectSlot, undefined);
64+
object->SetInternalField(kSyntheticEvaluationStepsSlot,
65+
synthetic_evaluation_step);
66+
object->SetInternalField(kContextObjectSlot, context_object);
67+
68+
if (!synthetic_evaluation_step->IsUndefined()) {
69+
synthetic_ = true;
70+
}
6571
}
6672

6773
ModuleWrap::~ModuleWrap() {
@@ -79,7 +85,9 @@ ModuleWrap::~ModuleWrap() {
7985

8086
Local<Context> ModuleWrap::context() const {
8187
Local<Value> obj = object()->GetInternalField(kContextObjectSlot).As<Value>();
82-
if (obj.IsEmpty()) return {};
88+
// If this fails, there is likely a bug e.g. ModuleWrap::context() is accessed
89+
// before the ModuleWrap constructor completes.
90+
CHECK(obj->IsObject());
8391
return obj.As<Object>()->GetCreationContext().ToLocalChecked();
8492
}
8593

@@ -227,18 +235,16 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
227235
return;
228236
}
229237

230-
ModuleWrap* obj = new ModuleWrap(env, that, module, url);
231-
232-
if (synthetic) {
233-
obj->synthetic_ = true;
234-
obj->object()->SetInternalField(kSyntheticEvaluationStepsSlot, args[3]);
235-
}
236-
237238
// Use the extras object as an object whose GetCreationContext() will be the
238239
// original `context`, since the `Context` itself strictly speaking cannot
239240
// be stored in an internal field.
240-
obj->object()->SetInternalField(kContextObjectSlot,
241-
context->GetExtrasBindingObject());
241+
Local<Object> context_object = context->GetExtrasBindingObject();
242+
Local<Value> synthetic_evaluation_step =
243+
synthetic ? args[3] : Undefined(env->isolate()).As<v8::Value>();
244+
245+
ModuleWrap* obj = new ModuleWrap(
246+
env, that, module, url, context_object, synthetic_evaluation_step);
247+
242248
obj->contextify_context_ = contextify_context;
243249

244250
env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);
Collapse file

‎src/module_wrap.h‎

Copy file name to clipboardExpand all lines: src/module_wrap.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ class ModuleWrap : public BaseObject {
7272
ModuleWrap(Environment* env,
7373
v8::Local<v8::Object> object,
7474
v8::Local<v8::Module> module,
75-
v8::Local<v8::String> url);
75+
v8::Local<v8::String> url,
76+
v8::Local<v8::Object> context_object,
77+
v8::Local<v8::Value> synthetic_evaluation_step);
7678
~ModuleWrap() override;
7779

7880
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.