From 01cab978f7359374efc5bd609da0448249fbcec3 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 3 Oct 2025 14:10:29 -0500 Subject: [PATCH] refactor: remove `gin_helper::Arguments` (#48374) * refactor: make api::Clipboard::GetClipboardBuffer() private * refactor: move GetClipboadBuffer() into anonymous namespace * refactor: use gin::Arguments in StopRecording() * refactor: use gin::Arguments in ImageView::New() * refactor: use gin::Arguments in AppendSwitch() * refactor: use gin::Arguments WebContentsView::New() * refactor: make gin::Arguments arg const in WrappableBase::InitWithArgs() This makes explicit that we are using it for wrapper + isolate, not the args values * refactor: remove gin_helper::Arguments arg from ExposeAPI() refactor: remove gin_helper::Arguments arg from ExposeAPIInWorld() * refactor: remove gin_helper::Arguments arg from ElectronBindings::GetSystemMemoryInfo() * refactor: remove gin_helper::Arguments arg from preload_utils::GetBinding() * refactor: use gin::Arguments in OpenExternal() * refactor: use gin::Arguments in ExecuteInWorld() * refactor: use gin::Arguments in ExecuteJavaScript() * refactor: use gin::Arguments in InvokeNew() * refactor: use gin::Arguments in ExecuteJavaScriptInIsolatedWorld() * refactor: remove unused GetNextArgument() marshaller for gin_helper::Arguments * refactor: remove unused #include gin_helper/arguments.h * chore: remove unused gin_helper::Arguments * fixup! refactor: use gin::Arguments in ExecuteJavaScriptInIsolatedWorld() Xref: https://github.com/electron/electron/pull/48447 --- filenames.gni | 2 - .../api/electron_api_content_tracing.cc | 4 +- .../api/electron_api_web_contents_view.cc | 23 ++++---- .../api/electron_api_web_contents_view.h | 2 +- .../api/views/electron_api_image_view.cc | 2 +- .../api/views/electron_api_image_view.h | 7 ++- shell/browser/browser.cc | 2 +- shell/browser/native_window_views.cc | 1 - shell/common/api/electron_api_command_line.cc | 2 +- shell/common/api/electron_api_shell.cc | 5 +- shell/common/api/electron_bindings.cc | 7 +-- shell/common/api/electron_bindings.h | 4 +- shell/common/gin_helper/arguments.cc | 26 --------- shell/common/gin_helper/arguments.h | 53 ------------------- shell/common/gin_helper/constructor.h | 8 +-- shell/common/gin_helper/function_template.h | 10 ---- shell/common/gin_helper/wrappable.cc | 2 +- shell/common/gin_helper/wrappable_base.h | 2 +- .../api/electron_api_context_bridge.cc | 24 ++++----- shell/renderer/api/electron_api_web_frame.cc | 38 +++++++------ shell/renderer/preload_utils.cc | 12 ++--- shell/renderer/preload_utils.h | 3 +- 22 files changed, 76 insertions(+), 163 deletions(-) delete mode 100644 shell/common/gin_helper/arguments.cc delete mode 100644 shell/common/gin_helper/arguments.h diff --git a/filenames.gni b/filenames.gni index 488058008c..a41f5cae22 100644 --- a/filenames.gni +++ b/filenames.gni @@ -629,8 +629,6 @@ filenames = { "shell/common/gin_converters/usb_device_info_converter.h", "shell/common/gin_converters/value_converter.cc", "shell/common/gin_converters/value_converter.h", - "shell/common/gin_helper/arguments.cc", - "shell/common/gin_helper/arguments.h", "shell/common/gin_helper/callback.cc", "shell/common/gin_helper/callback.h", "shell/common/gin_helper/cleaned_up_at_exit.cc", diff --git a/shell/browser/api/electron_api_content_tracing.cc b/shell/browser/api/electron_api_content_tracing.cc index 278ec587df..90652c0de5 100644 --- a/shell/browser/api/electron_api_content_tracing.cc +++ b/shell/browser/api/electron_api_content_tracing.cc @@ -98,8 +98,8 @@ void StopTracing(gin_helper::Promise promise, } } -v8::Local StopRecording(gin_helper::Arguments* args) { - gin_helper::Promise promise(args->isolate()); +v8::Local StopRecording(gin::Arguments* const args) { + gin_helper::Promise promise{args->isolate()}; v8::Local handle = promise.GetHandle(); base::FilePath path; diff --git a/shell/browser/api/electron_api_web_contents_view.cc b/shell/browser/api/electron_api_web_contents_view.cc index 0b9be9bd55..360d03d050 100644 --- a/shell/browser/api/electron_api_web_contents_view.cc +++ b/shell/browser/api/electron_api_web_contents_view.cc @@ -153,36 +153,37 @@ v8::Local WebContentsView::GetConstructor(v8::Isolate* isolate) { } // static -gin_helper::WrappableBase* WebContentsView::New(gin_helper::Arguments* args) { +gin_helper::WrappableBase* WebContentsView::New(gin::Arguments* const args) { + v8::Isolate* const isolate = args->isolate(); gin_helper::Dictionary web_preferences; v8::Local existing_web_contents_value; { v8::Local options_value; if (args->GetNext(&options_value)) { gin_helper::Dictionary options; - if (!gin::ConvertFromV8(args->isolate(), options_value, &options)) { - args->ThrowError("options must be an object"); + if (!gin::ConvertFromV8(isolate, options_value, &options)) { + args->ThrowTypeError("options must be an object"); return nullptr; } v8::Local web_preferences_value; if (options.Get("webPreferences", &web_preferences_value)) { - if (!gin::ConvertFromV8(args->isolate(), web_preferences_value, + if (!gin::ConvertFromV8(isolate, web_preferences_value, &web_preferences)) { - args->ThrowError("options.webPreferences must be an object"); + args->ThrowTypeError("options.webPreferences must be an object"); return nullptr; } } if (options.Get("webContents", &existing_web_contents_value)) { gin_helper::Handle existing_web_contents; - if (!gin::ConvertFromV8(args->isolate(), existing_web_contents_value, + if (!gin::ConvertFromV8(isolate, existing_web_contents_value, &existing_web_contents)) { - args->ThrowError("options.webContents must be a WebContents"); + args->ThrowTypeError("options.webContents must be a WebContents"); return nullptr; } if (existing_web_contents->owner_window() != nullptr) { - args->ThrowError( + args->ThrowTypeError( "options.webContents is already attached to a window"); return nullptr; } @@ -191,7 +192,7 @@ gin_helper::WrappableBase* WebContentsView::New(gin_helper::Arguments* args) { } if (web_preferences.IsEmpty()) - web_preferences = gin_helper::Dictionary::CreateEmpty(args->isolate()); + web_preferences = gin_helper::Dictionary::CreateEmpty(isolate); if (!web_preferences.Has(options::kShow)) web_preferences.Set(options::kShow, false); @@ -200,10 +201,10 @@ gin_helper::WrappableBase* WebContentsView::New(gin_helper::Arguments* args) { } auto web_contents = - WebContents::CreateFromWebPreferences(args->isolate(), web_preferences); + WebContents::CreateFromWebPreferences(isolate, web_preferences); // Constructor call. - auto* view = new WebContentsView(args->isolate(), web_contents); + auto* view = new WebContentsView{isolate, web_contents}; view->InitWithArgs(args); return view; } diff --git a/shell/browser/api/electron_api_web_contents_view.h b/shell/browser/api/electron_api_web_contents_view.h index 51c67231e1..f04254d30b 100644 --- a/shell/browser/api/electron_api_web_contents_view.h +++ b/shell/browser/api/electron_api_web_contents_view.h @@ -57,7 +57,7 @@ class WebContentsView : public View, void OnViewRemovedFromWidget(views::View* view) override; private: - static gin_helper::WrappableBase* New(gin_helper::Arguments* args); + static gin_helper::WrappableBase* New(gin::Arguments* args); void ApplyBorderRadius(); diff --git a/shell/browser/api/views/electron_api_image_view.cc b/shell/browser/api/views/electron_api_image_view.cc index c0578c7777..3dbc56f226 100644 --- a/shell/browser/api/views/electron_api_image_view.cc +++ b/shell/browser/api/views/electron_api_image_view.cc @@ -25,7 +25,7 @@ void ImageView::SetImage(const gfx::Image& image) { } // static -gin_helper::WrappableBase* ImageView::New(gin_helper::Arguments* args) { +gin_helper::WrappableBase* ImageView::New(gin::Arguments* const args) { // Constructor call. auto* view = new ImageView(); view->InitWithArgs(args); diff --git a/shell/browser/api/views/electron_api_image_view.h b/shell/browser/api/views/electron_api_image_view.h index 78d913c046..156d256771 100644 --- a/shell/browser/api/views/electron_api_image_view.h +++ b/shell/browser/api/views/electron_api_image_view.h @@ -13,8 +13,11 @@ namespace gfx { class Image; } -namespace gin_helper { +namespace gin { class Arguments; +} // namespace gin + +namespace gin_helper { class WrappableBase; } // namespace gin_helper @@ -22,7 +25,7 @@ namespace electron::api { class ImageView : public View { public: - static gin_helper::WrappableBase* New(gin_helper::Arguments* args); + static gin_helper::WrappableBase* New(gin::Arguments* args); static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); diff --git a/shell/browser/browser.cc b/shell/browser/browser.cc index 0c0e7c8c50..ca23057de9 100644 --- a/shell/browser/browser.cc +++ b/shell/browser/browser.cc @@ -13,13 +13,13 @@ #include "base/task/single_thread_task_runner.h" #include "base/threading/thread_restrictions.h" #include "chrome/common/chrome_paths.h" +#include "gin/arguments.h" #include "shell/browser/browser_observer.h" #include "shell/browser/electron_browser_main_parts.h" #include "shell/browser/native_window.h" #include "shell/browser/window_list.h" #include "shell/common/application_info.h" #include "shell/common/gin_converters/login_item_settings_converter.h" -#include "shell/common/gin_helper/arguments.h" #include "shell/common/thread_restrictions.h" namespace electron { diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index fa447f6af0..6621158a0e 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -35,7 +35,6 @@ #include "shell/browser/window_list.h" #include "shell/common/electron_constants.h" #include "shell/common/gin_converters/image_converter.h" -#include "shell/common/gin_helper/arguments.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/options_switches.h" #include "ui/aura/window_tree_host.h" diff --git a/shell/common/api/electron_api_command_line.cc b/shell/common/api/electron_api_command_line.cc index 5225a43ec6..98bbf60351 100644 --- a/shell/common/api/electron_api_command_line.cc +++ b/shell/common/api/electron_api_command_line.cc @@ -29,7 +29,7 @@ base::CommandLine::StringType GetSwitchValue(gin_helper::ErrorThrower thrower, } void AppendSwitch(const std::string& switch_string, - gin_helper::Arguments* args) { + gin::Arguments* const args) { auto switch_str = base::ToLowerASCII(switch_string); auto* command_line = base::CommandLine::ForCurrentProcess(); if (base::EndsWith(switch_string, "-path", diff --git a/shell/common/api/electron_api_shell.cc b/shell/common/api/electron_api_shell.cc index 944e4a6b36..fc9d014a14 100644 --- a/shell/common/api/electron_api_shell.cc +++ b/shell/common/api/electron_api_shell.cc @@ -55,7 +55,8 @@ void OnOpenFinished(gin_helper::Promise promise, promise.RejectWithErrorMessage(error); } -v8::Local OpenExternal(const GURL& url, gin::Arguments* args) { +v8::Local OpenExternal(const GURL& url, + gin::Arguments* const args) { gin_helper::Promise promise(args->isolate()); v8::Local handle = promise.GetHandle(); @@ -108,7 +109,7 @@ v8::Local TrashItem(v8::Isolate* isolate, #if BUILDFLAG(IS_WIN) bool WriteShortcutLink(const base::FilePath& shortcut_path, - gin_helper::Arguments* args) { + gin::Arguments* const args) { base::win::ShortcutOperation operation = base::win::ShortcutOperation::kCreateAlways; args->GetNext(&operation); diff --git a/shell/common/api/electron_bindings.cc b/shell/common/api/electron_bindings.cc index 417334f331..3bae82a897 100644 --- a/shell/common/api/electron_bindings.cc +++ b/shell/common/api/electron_bindings.cc @@ -21,6 +21,7 @@ #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/error_thrower.h" #include "shell/common/gin_helper/locker.h" #include "shell/common/gin_helper/promise.h" #include "shell/common/heap_snapshot.h" @@ -163,11 +164,11 @@ v8::Local ElectronBindings::GetCreationTime(v8::Isolate* isolate) { // static v8::Local ElectronBindings::GetSystemMemoryInfo( - v8::Isolate* isolate, - gin_helper::Arguments* args) { + v8::Isolate* const isolate) { base::SystemMemoryInfo mem_info; if (!base::GetSystemMemoryInfo(&mem_info)) { - args->ThrowError("Unable to retrieve system memory information"); + gin_helper::ErrorThrower{isolate}.ThrowError( + "Unable to retrieve system memory information"); return v8::Undefined(isolate); } diff --git a/shell/common/api/electron_bindings.h b/shell/common/api/electron_bindings.h index 851ae358f6..76259e0480 100644 --- a/shell/common/api/electron_bindings.h +++ b/shell/common/api/electron_bindings.h @@ -18,7 +18,6 @@ class FilePath; } namespace gin_helper { -class Arguments; class Dictionary; template class Promise; @@ -67,8 +66,7 @@ class ElectronBindings { static void Hang(); static v8::Local GetHeapStatistics(v8::Isolate* isolate); static v8::Local GetCreationTime(v8::Isolate* isolate); - static v8::Local GetSystemMemoryInfo(v8::Isolate* isolate, - gin_helper::Arguments* args); + static v8::Local GetSystemMemoryInfo(v8::Isolate* isolate); static v8::Local GetProcessMemoryInfo(v8::Isolate* isolate); static v8::Local GetBlinkMemoryInfo(v8::Isolate* isolate); static v8::Local GetCPUUsage(base::ProcessMetrics* metrics, diff --git a/shell/common/gin_helper/arguments.cc b/shell/common/gin_helper/arguments.cc deleted file mode 100644 index 9047d26d6d..0000000000 --- a/shell/common/gin_helper/arguments.cc +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE.chromium file. - -#include - -#include "shell/common/gin_helper/arguments.h" - -#include "v8/include/v8-exception.h" - -namespace gin_helper { - -void Arguments::ThrowError() const { - // Gin advances |next_| counter when conversion fails while we do not, so we - // have to manually advance the counter here to make gin report error with the - // correct index. - const_cast(this)->Skip(); - gin::Arguments::ThrowError(); -} - -void Arguments::ThrowError(const std::string_view message) const { - isolate()->ThrowException( - v8::Exception::Error(gin::StringToV8(isolate(), message))); -} - -} // namespace gin_helper diff --git a/shell/common/gin_helper/arguments.h b/shell/common/gin_helper/arguments.h deleted file mode 100644 index eb79200972..0000000000 --- a/shell/common/gin_helper/arguments.h +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE.chromium file. - -#ifndef ELECTRON_SHELL_COMMON_GIN_HELPER_ARGUMENTS_H_ -#define ELECTRON_SHELL_COMMON_GIN_HELPER_ARGUMENTS_H_ - -#include - -#include "gin/arguments.h" - -namespace gin_helper { - -// Provides additional APIs to the gin::Arguments class. -class Arguments : public gin::Arguments { - public: - // Get the next argument, if conversion to T fails then state is unchanged. - // - // This is difference from gin::Arguments::GetNext which always advances the - // |next_| counter no matter whether the conversion succeeds. - template - bool GetNext(T* out) { - v8::Local val = PeekNext(); - if (val.IsEmpty()) - return false; - if (!gin::ConvertFromV8(isolate(), val, out)) - return false; - Skip(); - return true; - } - - // Gin always returns true when converting V8 value to boolean, we do not want - // this behavior when parsing parameters. - bool GetNext(bool* out) { - v8::Local val = PeekNext(); - if (val.IsEmpty() || !val->IsBoolean()) - return false; - *out = val->BooleanValue(isolate()); - Skip(); - return true; - } - - // Throw error with custom error message. - void ThrowError() const; - void ThrowError(std::string_view message) const; - - private: - // MUST NOT ADD ANY DATA MEMBER. -}; - -} // namespace gin_helper - -#endif // ELECTRON_SHELL_COMMON_GIN_HELPER_ARGUMENTS_H_ diff --git a/shell/common/gin_helper/constructor.h b/shell/common/gin_helper/constructor.h index 7ae5e096c1..2e406a0dbb 100644 --- a/shell/common/gin_helper/constructor.h +++ b/shell/common/gin_helper/constructor.h @@ -40,7 +40,7 @@ class GinArgumentsToTuple { // Invoke a callback with arguments extracted from `args`. template WrappableBase* InvokeFactory( - gin::Arguments* args, + gin::Arguments* const args, const base::RepeatingCallback& callback) { auto [ok, tup] = GinArgumentsToTuple::GetArgs(args); if (!ok) @@ -52,10 +52,10 @@ WrappableBase* InvokeFactory( template void InvokeNew(const base::RepeatingCallback& factory, - v8::Isolate* isolate, - gin_helper::Arguments* args) { + v8::Isolate* const isolate, + gin::Arguments* const args) { if (!args->IsConstructCall()) { - args->ThrowError("Requires constructor call"); + args->ThrowTypeError("Requires constructor call"); return; } diff --git a/shell/common/gin_helper/function_template.h b/shell/common/gin_helper/function_template.h index 43bace908b..c9d82747ce 100644 --- a/shell/common/gin_helper/function_template.h +++ b/shell/common/gin_helper/function_template.h @@ -13,7 +13,6 @@ #include "base/memory/raw_ptr.h" #include "gin/arguments.h" #include "gin/per_isolate_data.h" -#include "shell/common/gin_helper/arguments.h" #include "shell/common/gin_helper/destroyable.h" #include "shell/common/gin_helper/error_thrower.h" #include "v8/include/v8-context.h" @@ -154,15 +153,6 @@ inline bool GetNextArgument(gin::Arguments* args, return true; } -// Electron-specific GetNextArgument that supports the gin_helper::Arguments. -inline bool GetNextArgument(gin::Arguments* args, - const InvokerOptions& invoker_options, - bool is_first, - gin_helper::Arguments** result) { - *result = static_cast(args); - return true; -} - // For advanced use cases, we allow callers to request the unparsed Arguments // object and poke around in it directly. inline bool GetNextArgument(gin::Arguments* args, diff --git a/shell/common/gin_helper/wrappable.cc b/shell/common/gin_helper/wrappable.cc index 94dbe2ffbe..2549fe8af6 100644 --- a/shell/common/gin_helper/wrappable.cc +++ b/shell/common/gin_helper/wrappable.cc @@ -48,7 +48,7 @@ v8::Local WrappableBase::GetWrapper() const { return {}; } -void WrappableBase::InitWithArgs(gin::Arguments* args) { +void WrappableBase::InitWithArgs(const gin::Arguments* const args) { v8::Local holder; args->GetHolder(&holder); InitWith(args->isolate(), holder); diff --git a/shell/common/gin_helper/wrappable_base.h b/shell/common/gin_helper/wrappable_base.h index 35a9de3f21..5d63a148ab 100644 --- a/shell/common/gin_helper/wrappable_base.h +++ b/shell/common/gin_helper/wrappable_base.h @@ -51,7 +51,7 @@ class WrappableBase { virtual void InitWith(v8::Isolate* isolate, v8::Local wrapper); // Helper to init with arguments. - void InitWithArgs(gin::Arguments* args); + void InitWithArgs(const gin::Arguments* args); v8::Global wrapper_; // Weak diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc index 3efed5f4d5..343fa0e70b 100644 --- a/shell/renderer/api/electron_api_context_bridge.cc +++ b/shell/renderer/api/electron_api_context_bridge.cc @@ -739,14 +739,13 @@ void ExposeAPI(v8::Isolate* isolate, v8::Isolate* target_isolate, v8::Local target_context, const std::string& key, - v8::Local api, - gin_helper::Arguments* args) { + v8::Local api) { DCHECK(!target_context.IsEmpty()); v8::Context::Scope target_context_scope(target_context); gin_helper::Dictionary global(target_isolate, target_context->Global()); if (global.Has(key)) { - args->ThrowError( + gin_helper::ErrorThrower{isolate}.ThrowError( "Cannot bind an API on top of an existing property on the window " "object"); return; @@ -813,8 +812,7 @@ v8::MaybeLocal GetTargetContext(v8::Isolate* isolate, void ExposeAPIInWorld(v8::Isolate* isolate, const int world_id, const std::string& key, - v8::Local api, - gin_helper::Arguments* args) { + v8::Local api) { TRACE_EVENT2("electron", "ContextBridge::ExposeAPIInWorld", "key", key, "worldId", world_id); v8::Local source_context = isolate->GetCurrentContext(); @@ -825,8 +823,7 @@ void ExposeAPIInWorld(v8::Isolate* isolate, if (maybe_target_context.IsEmpty() || !target_isolate) return; v8::Local target_context = maybe_target_context.ToLocalChecked(); - ExposeAPI(isolate, source_context, target_isolate, target_context, key, api, - args); + ExposeAPI(isolate, source_context, target_isolate, target_context, key, api); } gin_helper::Dictionary TraceKeyPath(const gin_helper::Dictionary& start, @@ -923,24 +920,23 @@ bool OverrideGlobalPropertyFromIsolatedWorld( } // Serialize script to be executed in the given world. -v8::Local ExecuteInWorld(v8::Isolate* isolate, +v8::Local ExecuteInWorld(v8::Isolate* const isolate, const int world_id, - gin_helper::Arguments* args) { + gin::Arguments* const args) { // Get context of caller v8::Local source_context = isolate->GetCurrentContext(); // Get execution script argument gin_helper::Dictionary exec_script; if (args->Length() >= 1 && !args->GetNext(&exec_script)) { - gin_helper::ErrorThrower(args->isolate()).ThrowError("Invalid script"); + args->ThrowTypeError("Invalid script"); return v8::Undefined(isolate); } // Get "func" from execution script v8::Local func; if (!exec_script.Get("func", &func)) { - gin_helper::ErrorThrower(isolate).ThrowError( - "Function 'func' is required in script"); + args->ThrowTypeError("Function 'func' is required in script"); return v8::Undefined(isolate); } @@ -949,7 +945,7 @@ v8::Local ExecuteInWorld(v8::Isolate* isolate, v8::Local args_value; if (exec_script.Get("args", &args_value)) { if (!args_value->IsArray()) { - gin_helper::ErrorThrower(isolate).ThrowError("'args' must be an array"); + args->ThrowTypeError("'args' must be an array"); return v8::Undefined(isolate); } args_array = args_value.As(); @@ -961,7 +957,7 @@ v8::Local ExecuteInWorld(v8::Isolate* isolate, v8::Local serialized_function; if (!func->FunctionProtoToString(isolate->GetCurrentContext()) .ToLocal(&serialized_function)) { - gin_helper::ErrorThrower(isolate).ThrowError( + gin_helper::ErrorThrower{isolate}.ThrowError( "Failed to serialize function"); return v8::Undefined(isolate); } diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index 2575ee0e18..5c700084c3 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -18,6 +18,7 @@ #include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_frame_observer.h" #include "content/public/renderer/render_frame_visitor.h" +#include "gin/arguments.h" #include "gin/object_template_builder.h" #include "services/service_manager/public/cpp/interface_provider.h" #include "shell/common/api/api.mojom.h" @@ -637,12 +638,11 @@ class WebFrameRenderer final return !context->GetContentSecurityPolicy()->ShouldCheckEval(); } - v8::Local ExecuteJavaScript(gin::Arguments* gin_args, + // webFrame.executeJavaScript(code[, userGesture][, callback]) + v8::Local ExecuteJavaScript(gin::Arguments* const args, const std::u16string& code) { - gin_helper::Arguments* args = static_cast(gin_args); - - v8::Isolate* isolate = args->isolate(); - gin_helper::Promise> promise(isolate); + v8::Isolate* const isolate = args->isolate(); + gin_helper::Promise> promise{isolate}; v8::Local handle = promise.GetHandle(); content::RenderFrame* render_frame; @@ -655,10 +655,14 @@ class WebFrameRenderer final const blink::WebScriptSource source{blink::WebString::FromUTF16(code)}; bool has_user_gesture = false; - args->GetNext(&has_user_gesture); + if (auto next = args->PeekNext(); !next.IsEmpty() && next->IsBoolean()) { + args->GetNext(&has_user_gesture); + } ScriptExecutionCallback::CompletionCallback completion_callback; - args->GetNext(&completion_callback); + if (auto next = args->PeekNext(); !next.IsEmpty() && next->IsFunction()) { + args->GetNext(&completion_callback); + } auto* self = new ScriptExecutionCallback(std::move(promise), std::move(completion_callback)); @@ -679,14 +683,14 @@ class WebFrameRenderer final return handle; } + // executeJavaScriptInIsolatedWorld( + // worldId, scripts[, userGesture][, callback]) v8::Local ExecuteJavaScriptInIsolatedWorld( - gin::Arguments* gin_args, - int world_id, + gin::Arguments* const args, + const int world_id, const std::vector& scripts) { - gin_helper::Arguments* args = static_cast(gin_args); - - v8::Isolate* isolate = args->isolate(); - gin_helper::Promise> promise(isolate); + v8::Isolate* const isolate = args->isolate(); + gin_helper::Promise> promise{isolate}; v8::Local handle = promise.GetHandle(); content::RenderFrame* render_frame; @@ -698,10 +702,14 @@ class WebFrameRenderer final } bool has_user_gesture = false; - args->GetNext(&has_user_gesture); + if (auto next = args->PeekNext(); !next.IsEmpty() && next->IsBoolean()) { + args->GetNext(&has_user_gesture); + } ScriptExecutionCallback::CompletionCallback completion_callback; - args->GetNext(&completion_callback); + if (auto next = args->PeekNext(); !next.IsEmpty() && next->IsFunction()) { + args->GetNext(&completion_callback); + } std::vector sources; sources.reserve(scripts.size()); diff --git a/shell/renderer/preload_utils.cc b/shell/renderer/preload_utils.cc index 298b018eab..a7a3d55859 100644 --- a/shell/renderer/preload_utils.cc +++ b/shell/renderer/preload_utils.cc @@ -6,7 +6,6 @@ #include "base/process/process.h" #include "base/strings/strcat.h" -#include "shell/common/gin_helper/arguments.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/node_includes.h" #include "v8/include/v8-context.h" @@ -34,20 +33,19 @@ v8::Local GetBindingCache(v8::Isolate* isolate) { // adapted from node.cc v8::Local GetBinding(v8::Isolate* isolate, - v8::Local key, - gin_helper::Arguments* margs) { + v8::Local key) { v8::Local exports; - std::string binding_key = gin::V8ToString(isolate, key); + const std::string binding_key = gin::V8ToString(isolate, key); gin_helper::Dictionary cache(isolate, GetBindingCache(isolate)); if (cache.Get(binding_key, &exports)) { return exports; } - auto* mod = node::binding::get_linked_module(binding_key.c_str()); - + auto* const mod = node::binding::get_linked_module(binding_key.c_str()); if (!mod) { - margs->ThrowError(base::StrCat({"No such binding: ", binding_key})); + gin_helper::ErrorThrower{isolate}.ThrowError( + base::StrCat({"No such binding: ", binding_key})); return exports; } diff --git a/shell/renderer/preload_utils.h b/shell/renderer/preload_utils.h index 542e76a125..b2931cf54a 100644 --- a/shell/renderer/preload_utils.h +++ b/shell/renderer/preload_utils.h @@ -14,8 +14,7 @@ class Arguments; namespace electron::preload_utils { v8::Local GetBinding(v8::Isolate* isolate, - v8::Local key, - gin_helper::Arguments* margs); + v8::Local key); v8::Local CreatePreloadScript(v8::Isolate* isolate, v8::Local source);