review comments

src/node_builtins.cc

@@ -223,12 +223,7 @@ void BuiltinLoader::AddExternalizedBuiltin(const char* id,
   {
     Mutex::ScopedLock lock(externalized_builtins_mutex);
     auto it = externalized_builtin_sources.find(id);
-    if (it != externalized_builtin_sources.end()) {
-      // OK to get the raw pointer, since externalized_builtin_sources owns
-      // the resource, resources are never removed from the map, and
-      // externalized_builtin_sources has static lifetime.
-      resource = it->second.get();
-    } else {
+    if (it == externalized_builtin_sources.end()) {
       std::string source;
       int r = ReadFileSync(&source, filename);
       if (r != 0) {
@@ -247,14 +242,17 @@ void BuiltinLoader::AddExternalizedBuiltin(const char* id,
           reinterpret_cast<char16_t*>(out->data()));
       out->resize(u16_length);
 
-      auto resource_ptr = std::make_unique<StaticExternalTwoByteResource>(
-          out->data(), out->size(), out);
-      // OK to get the raw pointer, since externalized_builtin_sources owns
-      // the resource, resources are never removed from the map, and
-      // externalized_builtin_sources has static lifetime.
-      resource = resource_ptr.get();
-      externalized_builtin_sources[id] = std::move(resource_ptr);
+      auto result = externalized_builtin_sources.emplace(
+          id,
+          std::make_unique<StaticExternalTwoByteResource>(
+              out->data(), out->size(), out));
+      CHECK(result.second);
+      it = result.first;
     }
+    // OK to get the raw pointer, since externalized_builtin_sources owns
+    // the resource, resources are never removed from the map, and
+    // externalized_builtin_sources has static lifetime.
+    resource = it->second.get();
   }
 
   Add(id, UnionBytes(resource));

src/node_union_bytes.h

@@ -65,7 +65,7 @@ class UnionBytes {
   UnionBytes(UnionBytes&&) = default;
   UnionBytes& operator=(UnionBytes&&) = default;
 
-  bool IsOneByte() const { return one_byte_resource_ != nullptr; }
+  bool is_one_byte() const { return one_byte_resource_ != nullptr; }
 
   v8::Local<v8::String> ToStringChecked(v8::Isolate* isolate) const;
 

src/util.cc

@@ -552,7 +552,7 @@ void SetConstructorFunction(Isolate* isolate,
 }
 
 Local<String> UnionBytes::ToStringChecked(Isolate* isolate) const {
-  if (IsOneByte()) {
+  if (is_one_byte()) {
     return String::NewExternalOneByte(isolate, one_byte_resource_)
         .ToLocalChecked();
   } else {

test/cctest/test_per_process.cc

@@ -21,11 +21,11 @@ namespace {
 TEST_F(PerProcessTest, EmbeddedSources) {
   const auto& sources = PerProcessTest::get_sources_for_test();
   ASSERT_TRUE(std::any_of(sources.cbegin(), sources.cend(), [](auto p) {
-    return p.second.IsOneByte();
+    return p.second.is_one_byte();
   })) << "BuiltinLoader::source_ should have some 8bit items";
 
   ASSERT_TRUE(std::any_of(sources.cbegin(), sources.cend(), [](auto p) {
-    return !p.second.IsOneByte();
+    return !p.second.is_one_byte();
   })) << "BuiltinLoader::source_ should have some 16bit items";
 }