src: simplify Javascript code embedding

* use `string_view` instead of `UnionBytes`

Author: refack

common.gypi

@@ -231,6 +231,8 @@
         'SuppressStartupBanner': 'true',
         'WarnAsError': 'false',
         'WarningLevel': 3,       # /W3
+        'AdditionalOptions': [ '/std:c++17' ],
+
       },
       'VCLinkerTool': {
         'conditions': [
@@ -334,7 +336,7 @@
       }],
       [ 'OS in "linux freebsd openbsd solaris android aix cloudabi"', {
         'cflags': [ '-Wall', '-Wextra', '-Wno-unused-parameter', ],
-        'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++1y' ],
+        'cflags_cc': [ '-fno-rtti', '-fno-exceptions', '-std=gnu++1z' ],
         'ldflags': [ '-rdynamic' ],
         'target_conditions': [
           # The 1990s toolchain on SmartOS can't handle thin archives.
@@ -461,7 +463,7 @@
           ['clang==1', {
             'xcode_settings': {
               'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',
-              'CLANG_CXX_LANGUAGE_STANDARD': 'gnu++1y',  # -std=gnu++1y
+              'CLANG_CXX_LANGUAGE_STANDARD': 'gnu++1z',  # -std=gnu++1z
               'CLANG_CXX_LIBRARY': 'libc++',
             },
           }],

deps/gtest/gtest.gyp

@@ -7,6 +7,7 @@
       'direct_dependent_settings': {
         'include_dirs': ['include'],
       },
+      'defines': [ 'GTEST_LANG_CXX11' ],
       'include_dirs': ['.', 'include'],
       'sources': [
         'src/gtest-death-test.cc',

node.gyp

@@ -523,6 +523,7 @@
         'src/node_http2_state.h',
         'src/node_i18n.h',
         'src/node_internals.h',
+        'src/node_javascript.h',
         'src/node_messaging.h',
         'src/node_metadata.h',
         'src/node_mutex.h',
@@ -538,7 +539,6 @@
         'src/node_revert.h',
         'src/node_root_certs.h',
         'src/node_stat_watcher.h',
-        'src/node_union_bytes.h',
         'src/node_url.h',
         'src/node_version.h',
         'src/node_v8_platform-inl.h',
@@ -1031,7 +1031,10 @@
         '<(SHARED_INTERMEDIATE_DIR)', # for node_natives.h
       ],
 
-      'defines': [ 'NODE_WANT_INTERNALS=1' ],
+      'defines': [
+        'NODE_WANT_INTERNALS=1',
+        'GTEST_LANG_CXX11'
+      ],
 
       'sources': [
         'test/cctest/node_test_fixture.cc',

node.gypi

@@ -78,6 +78,9 @@
         '<(_msvs_precompiled_header)',
         '<(_msvs_precompiled_source)',
       ],
+      'include_dirs': [
+        'tools/msvs/pch',
+      ],
     }, { # POSIX
       'defines': [ '__POSIX__' ],
     }],

src/node_javascript.h

@@ -0,0 +1,32 @@
+#pragma once
+
+#include <map>
+#include <string>
+
+#if __has_include(<string_view>)
+#include <string_view>
+using std::string_view;
+#else
+#include <experimental/string_view>
+using std::experimental::string_view;
+#endif
+
+
+
+namespace node {
+
+namespace native_module {
+
+using NativeModuleRecordMap = std::map<std::string, string_view>;
+
+class JavascriptEmbeddedCode {
+ public:
+  JavascriptEmbeddedCode();
+ protected:
+  string_view config_;
+  NativeModuleRecordMap source_;
+};
+
+}  // namespace native_module
+
+}  // namespace node

src/node_native_module.cc

@@ -1,5 +1,8 @@
 #include "node_native_module.h"
 #include "node_errors.h"
+#include "env.h"
+
+#include <set>
 
 namespace node {
 
@@ -16,18 +19,15 @@ using v8::DEFAULT;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
-using v8::HandleScope;
 using v8::Integer;
 using v8::IntegrityLevel;
 using v8::Isolate;
 using v8::Local;
-using v8::Maybe;
 using v8::MaybeLocal;
 using v8::Name;
 using v8::None;
 using v8::Object;
 using v8::PropertyCallbackInfo;
-using v8::Script;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
 using v8::Set;
@@ -36,6 +36,32 @@ using v8::String;
 using v8::Uint8Array;
 using v8::Value;
 
+class UInt8SpanResource
+    : public String::ExternalOneByteStringResource {
+ public:
+  explicit UInt8SpanResource(string_view span) : span_(span) {}
+  UInt8SpanResource(const UInt8SpanResource&) = delete;
+  UInt8SpanResource(const UInt8SpanResource&&) = delete;
+  UInt8SpanResource& operator=(const UInt8SpanResource&) = delete;
+  UInt8SpanResource& operator=(const UInt8SpanResource&&) = delete;
+
+  ~UInt8SpanResource() override = default;
+  void Dispose() override { delete this; }
+
+  const char* data() const override { return span_.data(); }
+  size_t length() const override { return span_.size(); }
+
+  static v8::Local<v8::String> ToStringChecked(v8::Isolate* isolate,
+                                               string_view span) {
+    return
+        v8::String::NewExternalOneByte(isolate, new UInt8SpanResource{span})
+        .ToLocalChecked();
+  }
+
+ private:
+  const string_view span_;
+};
+
 void NativeModuleLoader::InitializeModuleCategories() {
   if (module_categories_.is_initialized) {
     DCHECK(!module_categories_.can_be_required.empty());
@@ -83,22 +109,18 @@ void NativeModuleLoader::InitializeModuleCategories() {
       "internal/v8_prof_processor",
   };
 
-  for (auto const& x : source_) {
+  for (const auto& x : source_) {
     const std::string& id = x.first;
     for (auto const& prefix : prefixes) {
       if (prefix.length() > id.length()) {
         continue;
       }
       if (id.find(prefix) == 0) {
-        module_categories_.cannot_be_required.emplace(id);
+        module_categories_.cannot_be_required.insert(id);
       }
     }
-  }
-
-  for (auto const& x : source_) {
-    const std::string& id = x.first;
     if (0 == module_categories_.cannot_be_required.count(id)) {
-      module_categories_.can_be_required.emplace(id);
+      module_categories_.can_be_required.insert(id);
     }
   }
 
@@ -111,8 +133,11 @@ Local<Object> MapToObject(Local<Context> context,
   Isolate* isolate = context->GetIsolate();
   Local<Object> out = Object::New(isolate);
   for (auto const& x : in) {
-    Local<String> key = OneByteString(isolate, x.first.c_str(), x.first.size());
-    out->Set(context, key, x.second.ToStringChecked(isolate)).FromJust();
+    const Local<String> key = OneByteString(
+      isolate, x.first.c_str(), x.first.size());
+    const Local<String> val = UInt8SpanResource::ToStringChecked(
+      isolate, x.second);
+    out->Set(context, key, val).Check();
   }
   return out;
 }
@@ -206,18 +231,14 @@ void NativeModuleLoader::ConfigStringGetter(
       per_process::native_module_loader.GetConfigString(info.GetIsolate()));
 }
 
-Local<Object> NativeModuleLoader::GetSourceObject(
-    Local<Context> context) const {
+Local<Object> NativeModuleLoader::GetSourceObject(Local<Context> context)
+  const {
   return MapToObject(context, source_);
 }
 
-Local<String> NativeModuleLoader::GetConfigString(Isolate* isolate) const {
-  return config_.ToStringChecked(isolate);
-}
-
-NativeModuleLoader::NativeModuleLoader() : config_(GetConfig()) {
-  LoadJavaScriptSource();
-  LoadCodeCache();
+Local<String> NativeModuleLoader::GetConfigString(Isolate* isolate)
+  const {
+  return UInt8SpanResource::ToStringChecked(isolate, config_);;
 }
 
 // This is supposed to be run only by the main thread in
@@ -294,9 +315,8 @@ MaybeLocal<Function> NativeModuleLoader::LookupAndCompile(
   Isolate* isolate = context->GetIsolate();
   EscapableHandleScope scope(isolate);
 
-  const auto source_it = source_.find(id);
-  CHECK_NE(source_it, source_.end());
-  Local<String> source = source_it->second.ToStringChecked(isolate);
+  const Local<String> source = UInt8SpanResource::ToStringChecked(
+    isolate, source_.at(id));
 
   std::string filename_s = id + std::string(".js");
   Local<String> filename =

src/node_native_module.h

@@ -3,18 +3,19 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include <map>
-#include <set>
-#include <string>
 #include "env.h"
+#include "node_javascript.h"
 #include "node_mutex.h"
-#include "node_union_bytes.h"
 #include "v8.h"
 
+#include <memory>
+#include <set>
+#include <string>
+
 namespace node {
+
 namespace native_module {
 
-using NativeModuleRecordMap = std::map<std::string, UnionBytes>;
 using NativeModuleCacheMap =
     std::unordered_map<std::string,
                        std::unique_ptr<v8::ScriptCompiler::CachedData>>;
@@ -26,13 +27,16 @@ using NativeModuleCacheMap =
 // This class should not depend on a particular isolate, context, or
 // environment. Rather it should take them as arguments when necessary.
 // The instances of this class are per-process.
-class NativeModuleLoader {
+class NativeModuleLoader : public JavascriptEmbeddedCode {
  public:
-  NativeModuleLoader();
+  NativeModuleLoader() = default;
+  ~NativeModuleLoader() = default;
   // TODO(joyeecheung): maybe we should make this a singleton, instead of
   // putting it in per_process.
   NativeModuleLoader(const NativeModuleLoader&) = delete;
+  NativeModuleLoader(const NativeModuleLoader&&) = delete;
   NativeModuleLoader& operator=(const NativeModuleLoader&) = delete;
+  NativeModuleLoader& operator=(const NativeModuleLoader&&) = delete;
 
   static void Initialize(v8::Local<v8::Object> target,
                          v8::Local<v8::Value> unused,
@@ -75,12 +79,11 @@ class NativeModuleLoader {
 
   // Generated by tools/js2c.py as node_javascript.cc
   void LoadJavaScriptSource();  // Loads data into source_
-  UnionBytes GetConfig();       // Return data for config.gypi
 
   // Generated by tools/generate_code_cache.js as node_code_cache.cc when
   // the build is configured with --code-cache-path=.... They are noops
   // in node_code_cache_stub.cc
-  void LoadCodeCache();      // Loads data into code_cache_
+  void LoadCodeCache();  // Loads data into code_cache_
 
   // Compile a script as a NativeModule that can be loaded via
   // NativeModule.p.require in JS land.
@@ -96,9 +99,7 @@ class NativeModuleLoader {
 
   ModuleCategories module_categories_;
 
-  NativeModuleRecordMap source_;
   NativeModuleCacheMap code_cache_;
-  UnionBytes config_;
 
   // Used to synchronize access to the code cache map
   Mutex code_cache_mutex_;