diff --git a/shell/app/electron_main_delegate.cc b/shell/app/electron_main_delegate.cc index 36bcadcabf9..779bcabc49d 100644 --- a/shell/app/electron_main_delegate.cc +++ b/shell/app/electron_main_delegate.cc @@ -40,6 +40,7 @@ #include "shell/common/logging.h" #include "shell/common/options_switches.h" #include "shell/common/platform_util.h" +#include "shell/common/process_util.h" #include "shell/common/thread_restrictions.h" #include "shell/renderer/electron_renderer_client.h" #include "shell/renderer/electron_sandboxed_renderer_client.h" @@ -83,11 +84,6 @@ constexpr base::StringPiece kElectronDisableSandbox("ELECTRON_DISABLE_SANDBOX"); constexpr base::StringPiece kElectronEnableStackDumping( "ELECTRON_ENABLE_STACK_DUMPING"); -bool IsBrowserProcess(base::CommandLine* cmd) { - std::string process_type = cmd->GetSwitchValueASCII(::switches::kProcessType); - return process_type.empty(); -} - // Returns true if this subprocess type needs the ResourceBundle initialized // and resources loaded. bool SubprocessNeedsResourceBundle(const std::string& process_type) { @@ -250,14 +246,12 @@ absl::optional ElectronMainDelegate::BasicStartupComplete() { // On Windows the terminal returns immediately, so we add a new line to // prevent output in the same line as the prompt. - if (IsBrowserProcess(command_line)) + if (IsBrowserProcess()) std::wcout << std::endl; #endif // !BUILDFLAG(IS_WIN) auto env = base::Environment::Create(); - gin_helper::Locker::SetIsBrowserProcess(IsBrowserProcess(command_line)); - // Enable convenient stack printing. This is enabled by default in // non-official builds. if (env->HasVar(kElectronEnableStackDumping)) @@ -290,7 +284,7 @@ absl::optional ElectronMainDelegate::BasicStartupComplete() { // bugs, but no use in Electron. base::win::DisableHandleVerifier(); - if (IsBrowserProcess(command_line)) + if (IsBrowserProcess()) base::win::PinUser32(); #endif @@ -386,7 +380,7 @@ void ElectronMainDelegate::PreSandboxStartup() { crash_keys::SetPlatformCrashKey(); #endif - if (IsBrowserProcess(command_line)) { + if (IsBrowserProcess()) { // Only append arguments for browser process. // Allow file:// URIs to read other file:// URIs by default. diff --git a/shell/common/api/electron_bindings.cc b/shell/common/api/electron_bindings.cc index ef84624a4d4..7a245aa6c59 100644 --- a/shell/common/api/electron_bindings.cc +++ b/shell/common/api/electron_bindings.cc @@ -22,11 +22,11 @@ #include "shell/common/application_info.h" #include "shell/common/gin_converters/file_path_converter.h" #include "shell/common/gin_helper/dictionary.h" -#include "shell/common/gin_helper/locker.h" #include "shell/common/gin_helper/microtasks_scope.h" #include "shell/common/gin_helper/promise.h" #include "shell/common/heap_snapshot.h" #include "shell/common/node_includes.h" +#include "shell/common/process_util.h" #include "shell/common/thread_restrictions.h" #include "third_party/blink/renderer/platform/heap/process_heap.h" // nogncheck @@ -50,7 +50,7 @@ void ElectronBindings::BindProcess(v8::Isolate* isolate, process->SetMethod("getCreationTime", &GetCreationTime); process->SetMethod("getHeapStatistics", &GetHeapStatistics); process->SetMethod("getBlinkMemoryInfo", &GetBlinkMemoryInfo); - if (gin_helper::Locker::IsBrowserProcess()) { + if (electron::IsBrowserProcess()) { process->SetMethod("getProcessMemoryInfo", &GetProcessMemoryInfo); } process->SetMethod("getSystemMemoryInfo", &GetSystemMemoryInfo); @@ -209,7 +209,7 @@ v8::Local ElectronBindings::GetSystemMemoryInfo( // static v8::Local ElectronBindings::GetProcessMemoryInfo( v8::Isolate* isolate) { - CHECK(gin_helper::Locker::IsBrowserProcess()); + CHECK(electron::IsBrowserProcess()); gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); diff --git a/shell/common/gin_helper/callback.cc b/shell/common/gin_helper/callback.cc index 4c68b5e14f4..0a82b901dc0 100644 --- a/shell/common/gin_helper/callback.cc +++ b/shell/common/gin_helper/callback.cc @@ -7,6 +7,7 @@ #include "base/cxx17_backports.h" #include "content/public/browser/browser_thread.h" #include "gin/dictionary.h" +#include "shell/common/process_util.h" namespace gin_helper { @@ -70,7 +71,7 @@ void CallTranslater(v8::Local external, struct DeleteOnUIThread { template static void Destruct(const T* x) { - if (gin_helper::Locker::IsBrowserProcess() && + if (electron::IsBrowserProcess() && !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE, x); diff --git a/shell/common/gin_helper/locker.cc b/shell/common/gin_helper/locker.cc index dbbaf7879fc..84740204ebb 100644 --- a/shell/common/gin_helper/locker.cc +++ b/shell/common/gin_helper/locker.cc @@ -4,19 +4,15 @@ #include "shell/common/gin_helper/locker.h" +#include "shell/common/process_util.h" + namespace gin_helper { Locker::Locker(v8::Isolate* isolate) { - if (IsBrowserProcess()) + if (electron::IsBrowserProcess()) locker_ = std::make_unique(isolate); } Locker::~Locker() = default; -void Locker::SetIsBrowserProcess(bool is_browser_process) { - g_is_browser_process = is_browser_process; -} - -bool Locker::g_is_browser_process = false; - } // namespace gin_helper diff --git a/shell/common/gin_helper/locker.h b/shell/common/gin_helper/locker.h index 9fb11ad4b35..b734d2f31cb 100644 --- a/shell/common/gin_helper/locker.h +++ b/shell/common/gin_helper/locker.h @@ -21,12 +21,6 @@ class Locker { Locker(const Locker&) = delete; Locker& operator=(const Locker&) = delete; - // Returns whether current process is browser process, currently we detect it - // by checking whether current has used V8 Lock, but it might be a bad idea. - static inline bool IsBrowserProcess() { return g_is_browser_process; } - - static void SetIsBrowserProcess(bool is_browser_process); - private: void* operator new(size_t size); void operator delete(void*, size_t); diff --git a/shell/common/gin_helper/microtasks_scope.cc b/shell/common/gin_helper/microtasks_scope.cc index 3b768a326f4..d1bb0674e6f 100644 --- a/shell/common/gin_helper/microtasks_scope.cc +++ b/shell/common/gin_helper/microtasks_scope.cc @@ -4,7 +4,7 @@ #include "shell/common/gin_helper/microtasks_scope.h" -#include "shell/common/gin_helper/locker.h" +#include "shell/common/process_util.h" namespace gin_helper { @@ -12,7 +12,7 @@ MicrotasksScope::MicrotasksScope(v8::Isolate* isolate, v8::MicrotaskQueue* microtask_queue, bool ignore_browser_checkpoint, v8::MicrotasksScope::Type scope_type) { - if (Locker::IsBrowserProcess()) { + if (electron::IsBrowserProcess()) { if (!ignore_browser_checkpoint) v8::MicrotasksScope::PerformCheckpoint(isolate); } else { diff --git a/shell/common/gin_helper/promise.cc b/shell/common/gin_helper/promise.cc index 933c74afc98..7a4720a255a 100644 --- a/shell/common/gin_helper/promise.cc +++ b/shell/common/gin_helper/promise.cc @@ -68,7 +68,7 @@ v8::Local PromiseBase::GetInner() const { // static void Promise::ResolvePromise(Promise promise) { - if (gin_helper::Locker::IsBrowserProcess() && + if (electron::IsBrowserProcess() && !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { content::GetUIThreadTaskRunner({})->PostTask( FROM_HERE, diff --git a/shell/common/gin_helper/promise.h b/shell/common/gin_helper/promise.h index 9660343cc07..efdc1709305 100644 --- a/shell/common/gin_helper/promise.h +++ b/shell/common/gin_helper/promise.h @@ -16,6 +16,7 @@ #include "shell/common/gin_converters/std_converter.h" #include "shell/common/gin_helper/locker.h" #include "shell/common/gin_helper/microtasks_scope.h" +#include "shell/common/process_util.h" namespace gin_helper { @@ -46,7 +47,7 @@ class PromiseBase { // Note: The parameter type is PromiseBase&& so it can take the instances of // Promise type. static void RejectPromise(PromiseBase&& promise, base::StringPiece errmsg) { - if (gin_helper::Locker::IsBrowserProcess() && + if (electron::IsBrowserProcess() && !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { content::GetUIThreadTaskRunner({})->PostTask( FROM_HERE, @@ -89,7 +90,7 @@ class Promise : public PromiseBase { // Helper for resolving the promise with |result|. static void ResolvePromise(Promise promise, RT result) { - if (gin_helper::Locker::IsBrowserProcess() && + if (electron::IsBrowserProcess() && !content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) { content::GetUIThreadTaskRunner({})->PostTask( FROM_HERE, base::BindOnce([](Promise promise, diff --git a/shell/common/gin_helper/trackable_object.cc b/shell/common/gin_helper/trackable_object.cc index 05919133179..517e7df410f 100644 --- a/shell/common/gin_helper/trackable_object.cc +++ b/shell/common/gin_helper/trackable_object.cc @@ -9,7 +9,7 @@ #include "base/functional/bind.h" #include "base/supports_user_data.h" #include "shell/browser/electron_browser_main_parts.h" -#include "shell/common/gin_helper/locker.h" +#include "shell/common/process_util.h" namespace gin_helper { @@ -31,7 +31,7 @@ class IDUserData : public base::SupportsUserData::Data { TrackableObjectBase::TrackableObjectBase() { // TODO(zcbenz): Make TrackedObject work in renderer process. - DCHECK(gin_helper::Locker::IsBrowserProcess()) + DCHECK(electron::IsBrowserProcess()) << "This class only works for browser process"; } diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 83596790b73..b22f0328cd6 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -171,7 +171,7 @@ bool AllowWasmCodeGenerationCallback(v8::Local context, // If we're running with contextIsolation enabled in the renderer process, // fall back to Blink's logic. if (node::Environment::GetCurrent(context) == nullptr) { - if (gin_helper::Locker::IsBrowserProcess()) + if (!electron::IsRendererProcess()) return false; return blink::V8Initializer::WasmCodeGenerationCheckCallbackInMainThread( context, source); @@ -188,7 +188,7 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( // No node environment means we're in the renderer process, either in a // sandboxed renderer or in an unsandboxed renderer with context isolation // enabled. - if (gin_helper::Locker::IsBrowserProcess()) { + if (!electron::IsRendererProcess()) { NOTREACHED(); return {false, {}}; } @@ -197,21 +197,20 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( } // If we get here then we have a node environment, so either a) we're in the - // main process, or b) we're in the renderer process in a context that has - // both node and blink, i.e. contextIsolation disabled. - - // If we're in the main process, delegate to node. - if (gin_helper::Locker::IsBrowserProcess()) { - return node::ModifyCodeGenerationFromStrings(context, source, is_code_like); - } + // non-rendrer process, or b) we're in the renderer process in a context that + // has both node and blink, i.e. contextIsolation disabled. // If we're in the renderer with contextIsolation disabled, ask blink first // (for CSP), and iff that allows codegen, delegate to node. - v8::ModifyCodeGenerationFromStringsResult result = - blink::V8Initializer::CodeGenerationCheckCallbackInMainThread( - context, source, is_code_like); - if (!result.codegen_allowed) - return result; + if (electron::IsRendererProcess()) { + v8::ModifyCodeGenerationFromStringsResult result = + blink::V8Initializer::CodeGenerationCheckCallbackInMainThread( + context, source, is_code_like); + if (!result.codegen_allowed) + return result; + } + + // If we're in the main process or utility process, delegate to node. return node::ModifyCodeGenerationFromStrings(context, source, is_code_like); } diff --git a/shell/common/process_util.cc b/shell/common/process_util.cc index c253af58fc9..f54bdf6c3d3 100644 --- a/shell/common/process_util.cc +++ b/shell/common/process_util.cc @@ -4,6 +4,8 @@ #include "shell/common/process_util.h" +#include "base/command_line.h" +#include "content/public/common/content_switches.h" #include "gin/dictionary.h" #include "shell/common/gin_converters/callback_converter.h" #include "shell/common/node_includes.h" @@ -24,4 +26,18 @@ void EmitWarning(node::Environment* env, emit_warning.Run(warning_msg, warning_type, ""); } +bool IsBrowserProcess() { + auto* command_line = base::CommandLine::ForCurrentProcess(); + std::string process_type = + command_line->GetSwitchValueASCII(switches::kProcessType); + return process_type.empty(); +} + +bool IsRendererProcess() { + auto* command_line = base::CommandLine::ForCurrentProcess(); + std::string process_type = + command_line->GetSwitchValueASCII(switches::kProcessType); + return process_type == switches::kRendererProcess; +} + } // namespace electron diff --git a/shell/common/process_util.h b/shell/common/process_util.h index bd08b102662..8a93eb9488d 100644 --- a/shell/common/process_util.h +++ b/shell/common/process_util.h @@ -17,6 +17,9 @@ void EmitWarning(node::Environment* env, const std::string& warning_msg, const std::string& warning_type); +bool IsBrowserProcess(); +bool IsRendererProcess(); + } // namespace electron #endif // ELECTRON_SHELL_COMMON_PROCESS_UTIL_H_ diff --git a/spec/api-utility-process-spec.ts b/spec/api-utility-process-spec.ts index 5c511877e8b..9d7f81eeefe 100644 --- a/spec/api-utility-process-spec.ts +++ b/spec/api-utility-process-spec.ts @@ -360,5 +360,19 @@ describe('utilityProcess module', () => { await once(child, 'exit'); expect(log).to.equal('hello\n'); }); + + it('does not crash when running eval', async () => { + const child = utilityProcess.fork('./eval.js', [], { + cwd: fixturesPath, + stdio: 'ignore' + }); + await once(child, 'spawn'); + const [data] = await once(child, 'message'); + expect(data).to.equal(42); + // Cleanup. + const exit = once(child, 'exit'); + expect(child.kill()).to.be.true(); + await exit; + }); }); }); diff --git a/spec/fixtures/api/utility-process/eval.js b/spec/fixtures/api/utility-process/eval.js new file mode 100644 index 00000000000..c521d90f1cc --- /dev/null +++ b/spec/fixtures/api/utility-process/eval.js @@ -0,0 +1,6 @@ +const vm = require('node:vm'); + +const contextObject = { result: 0 }; +vm.createContext(contextObject); +vm.runInContext('eval(\'result = 42\')', contextObject); +process.parentPort.postMessage(contextObject.result);