refactor: move CompileAndCall to a helper (#20675)

filenames.gni

@@ -545,6 +545,8 @@ filenames = {
     "shell/common/node_bindings_win.cc",
     "shell/common/node_bindings_win.h",
     "shell/common/node_includes.h",
+    "shell/common/node_util.h",
+    "shell/common/node_util.cc",
     "shell/common/options_switches.cc",
     "shell/common/options_switches.h",
     "shell/common/platform_util.h",

patches/node/.patches

@@ -28,7 +28,6 @@ chore_prevent_warn_non_context-aware_native_modules_being_loaded.patch
 chore_allow_the_node_entrypoint_to_be_a_builtin_module.patch
 inherit_electron_crashpad_pipe_name_in_child_process.patch
 fixme_revert_crypto_add_support_for_rsa-pss_keys.patch
-chore_re-add_compileandcall_this_should_be_added_as_a_helper_in.patch
 fix_extern_the_nativemoduleenv_and_options_parser_for_debug_builds.patch
 chore_read_nobrowserglobals_from_global_not_process.patch
 chore_split_createenvironment_into_createenvironment_and.patch

patches/node/chore_re-add_compileandcall_this_should_be_added_as_a_helper_in.patch

@@ -1,55 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Samuel Attard <sattard@slack-corp.com>
-Date: Fri, 28 Jun 2019 17:13:54 -0700
-Subject: chore: re-add CompileAndCall, this should be added as a helper in
- electron
-
-diff --git a/src/node_native_module_env.cc b/src/node_native_module_env.cc
-index 31536000fc8d2f9ce9589ef8e31cb55439fbd28d..6cb49b3b6def15a38ce1ba51da11af2567cb84ec 100644
---- a/src/node_native_module_env.cc
-+++ b/src/node_native_module_env.cc
-@@ -151,6 +151,22 @@ MaybeLocal<Function> NativeModuleEnv::LookupAndCompile(
-   return maybe;
- }
- 
-+MaybeLocal<Value> NativeModuleEnv::CompileAndCall(
-+    Local<Context> context,
-+    const char* id,
-+    std::vector<Local<String>>* parameters,
-+    std::vector<Local<Value>>* arguments,
-+    Environment* optional_env) {
-+  Isolate* isolate = context->GetIsolate();
-+  MaybeLocal<Function> compiled = LookupAndCompile(context, id, parameters, optional_env);
-+  if (compiled.IsEmpty()) {
-+    return MaybeLocal<Value>();
-+  }
-+  Local<Function> fn = compiled.ToLocalChecked().As<Function>();
-+  return fn->Call(
-+      context, v8::Null(isolate), arguments->size(), arguments->data());
-+}
-+
- // TODO(joyeecheung): It is somewhat confusing that Class::Initialize
- // is used to initialize to the binding, but it is the current convention.
- // Rename this across the code base to something that makes more sense.
-diff --git a/src/node_native_module_env.h b/src/node_native_module_env.h
-index f662c67be50d404ee5b6cf6e2b8dd5991c59e723..b91a5059cd1f19d87e5876c372f3ded60681a5df 100644
---- a/src/node_native_module_env.h
-+++ b/src/node_native_module_env.h
-@@ -24,6 +24,17 @@ class NativeModuleEnv {
-       const char* id,
-       std::vector<v8::Local<v8::String>>* parameters,
-       Environment* optional_env);
-+  // Run a script with JS source bundled inside the binary as if it's wrapped
-+  // in a function called with a null receiver and arguments specified in C++.
-+  // The returned value is empty if an exception is encountered.
-+  // JS code run with this method can assume that their top-level
-+  // declarations won't affect the global scope.
-+  static v8::MaybeLocal<v8::Value> CompileAndCall(
-+      v8::Local<v8::Context> context,
-+      const char* id,
-+      std::vector<v8::Local<v8::String>>* parameters,
-+      std::vector<v8::Local<v8::Value>>* arguments,
-+      Environment* optional_env);
- 
-   static v8::Local<v8::Object> GetSourceObject(v8::Local<v8::Context> context);
-   // Returns config.gypi as a JSON string

shell/common/api/atom_api_asar.cc

@@ -16,8 +16,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/native_mate_converters/file_path_converter.h"
 #include "shell/common/node_includes.h"
-#include "third_party/electron_node/src/node_native_module_env.h"
+#include "shell/common/node_util.h"
-
 namespace {
 
 class Archive : public mate::Wrappable<Archive> {
@@ -124,9 +123,9 @@ void InitAsarSupport(v8::Isolate* isolate, v8::Local<v8::Value> require) {
   std::vector<v8::Local<v8::String>> asar_init_params = {
       node::FIXED_ONE_BYTE_STRING(isolate, "require")};
   std::vector<v8::Local<v8::Value>> asar_init_args = {require};
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  electron::util::CompileAndCall(isolate->GetCurrentContext(),
-      isolate->GetCurrentContext(), "electron/js2c/asar_init",
+                                 "electron/js2c/asar_init", &asar_init_params,
-      &asar_init_params, &asar_init_args, nullptr);
+                                 &asar_init_args, nullptr);
 }
 
 v8::Local<v8::Value> SplitPath(v8::Isolate* isolate,

shell/common/node_util.cc

@@ -0,0 +1,33 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/common/node_util.h"
+#include "shell/common/node_includes.h"
+#include "third_party/electron_node/src/node_native_module_env.h"
+
+namespace electron {
+
+namespace util {
+
+v8::MaybeLocal<v8::Value> CompileAndCall(
+    v8::Local<v8::Context> context,
+    const char* id,
+    std::vector<v8::Local<v8::String>>* parameters,
+    std::vector<v8::Local<v8::Value>>* arguments,
+    node::Environment* optional_env) {
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::MaybeLocal<v8::Function> compiled =
+      node::native_module::NativeModuleEnv::LookupAndCompile(
+          context, id, parameters, optional_env);
+  if (compiled.IsEmpty()) {
+    return v8::MaybeLocal<v8::Value>();
+  }
+  v8::Local<v8::Function> fn = compiled.ToLocalChecked().As<v8::Function>();
+  return fn->Call(context, v8::Null(isolate), arguments->size(),
+                  arguments->data());
+}
+
+}  // namespace util
+
+}  // namespace electron

shell/common/node_util.h

@@ -0,0 +1,36 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_NODE_UTIL_H_
+#define SHELL_COMMON_NODE_UTIL_H_
+
+#include <vector>
+
+#include "v8/include/v8.h"
+
+namespace node {
+class Environment;
+}  // namespace node
+
+namespace electron {
+
+namespace util {
+
+// Run a script with JS source bundled inside the binary as if it's wrapped
+// in a function called with a null receiver and arguments specified in C++.
+// The returned value is empty if an exception is encountered.
+// JS code run with this method can assume that their top-level
+// declarations won't affect the global scope.
+v8::MaybeLocal<v8::Value> CompileAndCall(
+    v8::Local<v8::Context> context,
+    const char* id,
+    std::vector<v8::Local<v8::String>>* parameters,
+    std::vector<v8::Local<v8::Value>>* arguments,
+    node::Environment* optional_env);
+
+}  // namespace util
+
+}  // namespace electron
+
+#endif  // SHELL_COMMON_NODE_UTIL_H_

shell/renderer/atom_renderer_client.cc

@@ -16,12 +16,12 @@
 #include "shell/common/gin_helper/event_emitter_caller.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/node_util.h"
 #include "shell/common/options_switches.h"
 #include "shell/renderer/atom_render_frame_observer.h"
 #include "shell/renderer/web_worker_observer.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_local_frame.h"
-#include "third_party/electron_node/src/node_native_module_env.h"
 
 namespace electron {
 
@@ -227,9 +227,8 @@ void AtomRendererClient::SetupMainWorldOverrides(
       env->process_object(),
       GetContext(render_frame->GetWebFrame(), isolate)->Global()};
 
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  util::CompileAndCall(context, "electron/js2c/isolated_bundle",
-      context, "electron/js2c/isolated_bundle", &isolated_bundle_params,
+                       &isolated_bundle_params, &isolated_bundle_args, nullptr);
-      &isolated_bundle_args, nullptr);
 }
 
 void AtomRendererClient::SetupExtensionWorldOverrides(
@@ -255,9 +254,8 @@ void AtomRendererClient::SetupExtensionWorldOverrides(
       GetContext(render_frame->GetWebFrame(), isolate)->Global(),
       v8::Integer::New(isolate, world_id)};
 
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  util::CompileAndCall(context, "electron/js2c/content_script_bundle",
-      context, "electron/js2c/content_script_bundle", &isolated_bundle_params,
+                       &isolated_bundle_params, &isolated_bundle_args, nullptr);
-      &isolated_bundle_args, nullptr);
 #endif
 }
 

shell/renderer/atom_sandboxed_renderer_client.cc

@@ -20,12 +20,12 @@
 #include "shell/common/native_mate_converters/value_converter.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/node_util.h"
 #include "shell/common/options_switches.h"
 #include "shell/renderer/atom_render_frame_observer.h"
 #include "third_party/blink/public/web/blink.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/electron_node/src/node_binding.h"
-#include "third_party/electron_node/src/node_native_module_env.h"
 
 namespace electron {
 
@@ -231,7 +231,7 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
 
   std::vector<v8::Local<v8::Value>> sandbox_preload_bundle_args = {binding};
 
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  util::CompileAndCall(
       isolate->GetCurrentContext(), "electron/js2c/sandbox_bundle",
       &sandbox_preload_bundle_params, &sandbox_preload_bundle_args, nullptr);
 
@@ -259,9 +259,8 @@ void AtomSandboxedRendererClient::SetupMainWorldOverrides(
       process.GetHandle(),
       GetContext(render_frame->GetWebFrame(), isolate)->Global()};
 
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  util::CompileAndCall(context, "electron/js2c/isolated_bundle",
-      context, "electron/js2c/isolated_bundle", &isolated_bundle_params,
+                       &isolated_bundle_params, &isolated_bundle_args, nullptr);
-      &isolated_bundle_args, nullptr);
 }
 
 void AtomSandboxedRendererClient::SetupExtensionWorldOverrides(
@@ -286,9 +285,8 @@ void AtomSandboxedRendererClient::SetupExtensionWorldOverrides(
       GetContext(render_frame->GetWebFrame(), isolate)->Global(),
       v8::Integer::New(isolate, world_id)};
 
-  node::native_module::NativeModuleEnv::CompileAndCall(
+  util::CompileAndCall(context, "electron/js2c/content_script_bundle",
-      context, "electron/js2c/content_script_bundle", &isolated_bundle_params,
+                       &isolated_bundle_params, &isolated_bundle_args, nullptr);
-      &isolated_bundle_args, nullptr);
 #endif
 }