From a468ed7f10301dc1782de0105ae3800d82b1c7e1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 12 Sep 2025 18:19:07 -0500 Subject: [PATCH] refactor: narrow or remove gin arguments (#48300) * refactor: narrow App:SetJumpList() arg from gin::Arguments* to v8::Isolate* * refactor: narrow WebContents::AddWorkSpace() arg from gin::Arguments* to v8::Isolate* * refactor: narrow ShowMessageBox() arg from gin::Arguments* to v8::Isolate* * refactor: narrow ShowOpenDialog() arg from gin::Arguments* to v8::Isolate* * refactor: remove unused gin::Arguments* arg from OverrideGlobalPropertyFromIsolatedWorld() * refactor: narrow WebContents::StartDrag() arg from gin::Arguments* to v8::Isolate* * refactor: narrow NetLog::StopLogging() arg from gin::Arguments* to v8::Isolate* * refactor: narrow Protocol::IsProtocolHandled() arg from gin::Arguments* to v8::Isolate* --- shell/browser/api/electron_api_app.cc | 11 ++++---- shell/browser/api/electron_api_app.h | 2 +- shell/browser/api/electron_api_dialog.cc | 11 ++++---- shell/browser/api/electron_api_net_log.cc | 4 +-- shell/browser/api/electron_api_net_log.h | 2 +- shell/browser/api/electron_api_protocol.cc | 25 +++++++++---------- shell/browser/api/electron_api_protocol.h | 4 +-- .../browser/api/electron_api_web_contents.cc | 24 ++++++++---------- shell/browser/api/electron_api_web_contents.h | 6 ++--- .../api/electron_api_context_bridge.cc | 5 ++-- 10 files changed, 44 insertions(+), 50 deletions(-) diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 4fb59a5157..dd5cfe9330 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -1241,14 +1241,13 @@ v8::Local App::GetJumpListSettings() { return dict.GetHandle(); } -JumpListResult App::SetJumpList(v8::Local val, - gin::Arguments* args) { +JumpListResult App::SetJumpList(v8::Isolate* const isolate, + v8::Local val) { std::vector categories; bool delete_jump_list = val->IsNull(); - if (!delete_jump_list && - !gin::ConvertFromV8(args->isolate(), val, &categories)) { - gin_helper::ErrorThrower(args->isolate()) - .ThrowTypeError("Argument must be null or an array of categories"); + if (!delete_jump_list && !gin::ConvertFromV8(isolate, val, &categories)) { + gin_helper::ErrorThrower{isolate}.ThrowTypeError( + "Argument must be null or an array of categories"); return JumpListResult::kArgumentError; } diff --git a/shell/browser/api/electron_api_app.h b/shell/browser/api/electron_api_app.h index 7e91e44eb7..b1e40cedb7 100644 --- a/shell/browser/api/electron_api_app.h +++ b/shell/browser/api/electron_api_app.h @@ -258,7 +258,7 @@ class App final : public gin::Wrappable, v8::Local GetJumpListSettings(); // Set or remove a custom Jump List for the application. - JumpListResult SetJumpList(v8::Local val, gin::Arguments* args); + JumpListResult SetJumpList(v8::Isolate* isolate, v8::Local val); #endif // BUILDFLAG(IS_WIN) std::unique_ptr process_singleton_; diff --git a/shell/browser/api/electron_api_dialog.cc b/shell/browser/api/electron_api_dialog.cc index 9311c26e4d..89b8ab09b5 100644 --- a/shell/browser/api/electron_api_dialog.cc +++ b/shell/browser/api/electron_api_dialog.cc @@ -42,9 +42,8 @@ void ResolvePromiseObject(gin_helper::Promise promise, } v8::Local ShowMessageBox( - const electron::MessageBoxSettings& settings, - gin::Arguments* args) { - v8::Isolate* isolate = args->isolate(); + v8::Isolate* const isolate, + const electron::MessageBoxSettings& settings) { gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); @@ -62,9 +61,9 @@ void ShowOpenDialogSync(const file_dialog::DialogSettings& settings, } v8::Local ShowOpenDialog( - const file_dialog::DialogSettings& settings, - gin::Arguments* args) { - gin_helper::Promise promise(args->isolate()); + v8::Isolate* const isolate, + const file_dialog::DialogSettings& settings) { + gin_helper::Promise promise{isolate}; v8::Local handle = promise.GetHandle(); file_dialog::ShowOpenDialog(settings, std::move(promise)); return handle; diff --git a/shell/browser/api/electron_api_net_log.cc b/shell/browser/api/electron_api_net_log.cc index 29c04d585e..0a6059e51f 100644 --- a/shell/browser/api/electron_api_net_log.cc +++ b/shell/browser/api/electron_api_net_log.cc @@ -194,8 +194,8 @@ bool NetLog::IsCurrentlyLogging() const { return !!net_log_exporter_; } -v8::Local NetLog::StopLogging(gin::Arguments* args) { - gin_helper::Promise promise(args->isolate()); +v8::Local NetLog::StopLogging(v8::Isolate* const isolate) { + gin_helper::Promise promise{isolate}; v8::Local handle = promise.GetHandle(); if (net_log_exporter_) { diff --git a/shell/browser/api/electron_api_net_log.h b/shell/browser/api/electron_api_net_log.h index d056ad3062..a03de704e2 100644 --- a/shell/browser/api/electron_api_net_log.h +++ b/shell/browser/api/electron_api_net_log.h @@ -45,7 +45,7 @@ class NetLog final : public gin_helper::DeprecatedWrappable { v8::Local StartLogging(base::FilePath log_path, gin::Arguments* args); - v8::Local StopLogging(gin::Arguments* args); + v8::Local StopLogging(v8::Isolate* isolate); bool IsCurrentlyLogging() const; // gin_helper::Wrappable diff --git a/shell/browser/api/electron_api_protocol.cc b/shell/browser/api/electron_api_protocol.cc index a8afe13cfe..1e29412d0c 100644 --- a/shell/browser/api/electron_api_protocol.cc +++ b/shell/browser/api/electron_api_protocol.cc @@ -251,24 +251,23 @@ bool Protocol::IsProtocolIntercepted(const std::string& scheme) { return protocol_registry_->FindIntercepted(scheme) != nullptr; } -v8::Local Protocol::IsProtocolHandled(const std::string& scheme, - gin::Arguments* args) { - util::EmitWarning(args->isolate(), +v8::Local Protocol::IsProtocolHandled(v8::Isolate* const isolate, + const std::string& scheme) { + util::EmitWarning(isolate, "The protocol.isProtocolHandled API is deprecated, " "use protocol.isProtocolRegistered " "or protocol.isProtocolIntercepted instead.", "ProtocolDeprecateIsProtocolHandled"); return gin_helper::Promise::ResolvedPromise( - args->isolate(), - IsProtocolRegistered(scheme) || IsProtocolIntercepted(scheme) || - // The |isProtocolHandled| should return true for builtin - // schemes, however with NetworkService it is impossible to - // know which schemes are registered until a real network - // request is sent. - // So we have to test against a hard-coded builtin schemes - // list make it work with old code. We should deprecate - // this API with the new |isProtocolRegistered| API. - base::Contains(kBuiltinSchemes, scheme)); + isolate, IsProtocolRegistered(scheme) || IsProtocolIntercepted(scheme) || + // The |isProtocolHandled| should return true for builtin + // schemes, however with NetworkService it is impossible to + // know which schemes are registered until a real network + // request is sent. + // So we have to test against a hard-coded builtin schemes + // list make it work with old code. We should deprecate + // this API with the new |isProtocolRegistered| API. + base::Contains(kBuiltinSchemes, scheme)); } void Protocol::HandleOptionalCallback(gin::Arguments* args, Error error) { diff --git a/shell/browser/api/electron_api_protocol.h b/shell/browser/api/electron_api_protocol.h index 63c395b98d..372e646259 100644 --- a/shell/browser/api/electron_api_protocol.h +++ b/shell/browser/api/electron_api_protocol.h @@ -89,8 +89,8 @@ class Protocol final : public gin_helper::DeprecatedWrappable, bool IsProtocolIntercepted(const std::string& scheme); // Old async version of IsProtocolRegistered. - v8::Local IsProtocolHandled(const std::string& scheme, - gin::Arguments* args); + v8::Local IsProtocolHandled(v8::Isolate* isolate, + const std::string& scheme); // Helper for converting old registration APIs to new RegisterProtocol API. template diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 5e5797a3bc..e4839a64a8 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -3259,21 +3259,19 @@ v8::Local WebContents::PrintToPDF(const base::Value& settings) { } #endif -void WebContents::AddWorkSpace(gin::Arguments* args, +void WebContents::AddWorkSpace(v8::Isolate* const isolate, const base::FilePath& path) { if (path.empty()) { - gin_helper::ErrorThrower(args->isolate()) - .ThrowError("path cannot be empty"); + gin_helper::ErrorThrower{isolate}.ThrowError("path cannot be empty"); return; } DevToolsAddFileSystem(std::string(), path); } -void WebContents::RemoveWorkSpace(gin::Arguments* args, +void WebContents::RemoveWorkSpace(v8::Isolate* const isolate, const base::FilePath& path) { if (path.empty()) { - gin_helper::ErrorThrower(args->isolate()) - .ThrowError("path cannot be empty"); + gin_helper::ErrorThrower{isolate}.ThrowError("path cannot be empty"); return; } DevToolsRemoveFileSystem(path); @@ -3508,8 +3506,8 @@ void WebContents::EndFrameSubscription() { frame_subscriber_.reset(); } -void WebContents::StartDrag(const gin_helper::Dictionary& item, - gin::Arguments* args) { +void WebContents::StartDrag(v8::Isolate* const isolate, + const gin_helper::Dictionary& item) { base::FilePath file; std::vector files; if (!item.Get("files", &files) && item.Get("file", &file)) { @@ -3518,13 +3516,13 @@ void WebContents::StartDrag(const gin_helper::Dictionary& item, v8::Local icon_value; if (!item.Get("icon", &icon_value)) { - gin_helper::ErrorThrower(args->isolate()) - .ThrowError("'icon' parameter is required"); + gin_helper::ErrorThrower{isolate}.ThrowError( + "'icon' parameter is required"); return; } NativeImage* icon = nullptr; - if (!NativeImage::TryConvertNativeImage(args->isolate(), icon_value, &icon) || + if (!NativeImage::TryConvertNativeImage(isolate, icon_value, &icon) || icon->image().IsEmpty()) { return; } @@ -3534,8 +3532,8 @@ void WebContents::StartDrag(const gin_helper::Dictionary& item, base::CurrentThread::ScopedAllowApplicationTasksInNativeNestedLoop allow; DragFileItems(files, icon->image(), web_contents()->GetNativeView()); } else { - gin_helper::ErrorThrower(args->isolate()) - .ThrowError("Must specify either 'file' or 'files' option"); + gin_helper::ErrorThrower{isolate}.ThrowError( + "Must specify either 'file' or 'files' option"); } } diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 9e506880f6..084fd3e0f1 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -267,8 +267,8 @@ class WebContents final : public ExclusiveAccessContext, void SetNextChildWebPreferences(const gin_helper::Dictionary); // DevTools workspace api. - void AddWorkSpace(gin::Arguments* args, const base::FilePath& path); - void RemoveWorkSpace(gin::Arguments* args, const base::FilePath& path); + void AddWorkSpace(v8::Isolate* isolate, const base::FilePath& path); + void RemoveWorkSpace(v8::Isolate* isolate, const base::FilePath& path); // Editing commands. void Undo(); @@ -303,7 +303,7 @@ class WebContents final : public ExclusiveAccessContext, void EndFrameSubscription(); // Dragging native items. - void StartDrag(const gin_helper::Dictionary& item, gin::Arguments* args); + void StartDrag(v8::Isolate* isolate, const gin_helper::Dictionary& item); // Captures the page with |rect|, |callback| would be called when capturing is // done. diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc index 23b4d507a6..3efed5f4d5 100644 --- a/shell/renderer/api/electron_api_context_bridge.cc +++ b/shell/renderer/api/electron_api_context_bridge.cc @@ -875,8 +875,7 @@ bool OverrideGlobalPropertyFromIsolatedWorld( v8::Isolate* const isolate, const std::vector& key_path, v8::Local getter, - v8::Local setter, - gin_helper::Arguments* args) { + v8::Local setter) { if (key_path.empty()) return false; @@ -917,7 +916,7 @@ bool OverrideGlobalPropertyFromIsolatedWorld( v8::PropertyDescriptor desc(getter_proxy, setter_proxy); bool success = IsTrue(target_object->DefineProperty( - main_context, gin::StringToV8(args->isolate(), final_key), desc)); + main_context, gin::StringToV8(isolate, final_key), desc)); DCHECK(success); return success; }