refactor: prefer upstream gin::Arguments::ThrowTypeError() over gin_helper (#48368)

* refactor: use gin::Arguments::ThrowTypeError() in AutoUpdater::SetFeedURL()

* refactor: use gin::Arguments::ThrowTypeError() in Browser::Focus()

* refactor: use gin::Arguments::ThrowTypeError() in SystemPreferences::SetUserDefault()

* refactor: use gin::Arguments::ThrowTypeError() in UtilityProcessWrapper::Create()

* refactor: use gin::Arguments::ThrowTypeError() in UtilityProcessWrapper::PostMessage()

* refactor: use gin::Arguments::ThrowTypeError() in ElectronBundleMover::ShouldContinueMove()

* refactor: use gin::Arguments::ThrowTypeError() in OnClientCertificateSelected()

* refactor: use gin::Arguments::ThrowTypeError() in Session::ClearData()

* refactor: use gin::Arguments::ThrowTypeError() in ElectronBrowserContext::DisplayMediaDeviceChosen()

* refactor: use gin::Arguments::ThrowTypeError() in WebContents::ReplaceMisspelling()

* refactor: use gin::Arguments::ThrowTypeError() in WebContents::Print()

* chore: iwyu shell/common/gin_helper/error_thrower.h
This commit is contained in:
Charles Kerr
2025-09-24 19:10:05 -05:00
committed by GitHub
parent b51e82c5fb
commit 6661457cdf
14 changed files with 61 additions and 84 deletions

View File

@@ -455,10 +455,10 @@ void GotPrivateKey(std::shared_ptr<content::ClientCertificateDelegate> delegate,
}
void OnClientCertificateSelected(
v8::Isolate* isolate,
v8::Isolate* const isolate,
std::shared_ptr<content::ClientCertificateDelegate> delegate,
std::shared_ptr<net::ClientCertIdentityList> identities,
gin::Arguments* args) {
gin::Arguments* const args) {
if (args->Length() == 2) {
delegate->ContinueWithCertificate(nullptr, nullptr);
return;
@@ -473,8 +473,7 @@ void OnClientCertificateSelected(
gin_helper::Dictionary cert_data;
if (!gin::ConvertFromV8(isolate, val, &cert_data)) {
gin_helper::ErrorThrower(isolate).ThrowError(
"Must pass valid certificate object.");
args->ThrowTypeError("Must pass valid certificate object.");
return;
}

View File

@@ -21,7 +21,6 @@
#include "shell/common/gin_converters/gurl_converter.h"
#include "shell/common/gin_converters/value_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/gin_helper/handle.h"
#include "shell/common/gin_helper/object_template_builder.h"
#include "shell/common/gin_helper/promise.h"

View File

@@ -1469,9 +1469,8 @@ v8::Local<v8::Promise> Session::ClearCodeCaches(
return handle;
}
v8::Local<v8::Value> Session::ClearData(gin_helper::ErrorThrower thrower,
gin::Arguments* args) {
auto* isolate = JavascriptEnvironment::GetIsolate();
v8::Local<v8::Value> Session::ClearData(gin::Arguments* const args) {
v8::Isolate* const isolate = JavascriptEnvironment::GetIsolate();
BrowsingDataRemover::DataType data_type_mask = kClearDataTypeAll;
std::vector<url::Origin> origins;
@@ -1501,7 +1500,7 @@ v8::Local<v8::Value> Session::ClearData(gin_helper::ErrorThrower thrower,
options.Get("excludeOrigins", &exclude_origin_urls);
if (has_origins_key && has_exclude_origins_key) {
thrower.ThrowError(
args->ThrowTypeError(
"Cannot provide both 'origins' and 'excludeOrigins'");
return v8::Undefined(isolate);
}
@@ -1520,7 +1519,7 @@ v8::Local<v8::Value> Session::ClearData(gin_helper::ErrorThrower thrower,
// Opaque origins cannot be used with this API
if (origin.opaque()) {
thrower.ThrowError(absl::StrFormat(
args->ThrowTypeError(absl::StrFormat(
"Invalid origin: '%s'", origin_url.possibly_invalid_spec()));
return v8::Undefined(isolate);
}

View File

@@ -170,8 +170,7 @@ class Session final : public gin::Wrappable<Session>,
v8::Local<v8::Value> GetPath(v8::Isolate* isolate);
void SetCodeCachePath(gin::Arguments* args);
v8::Local<v8::Promise> ClearCodeCaches(const gin_helper::Dictionary& options);
v8::Local<v8::Value> ClearData(gin_helper::ErrorThrower thrower,
gin::Arguments* args);
v8::Local<v8::Value> ClearData(gin::Arguments* args);
#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
base::Value GetSpellCheckerLanguages();
void SetSpellCheckerLanguages(gin_helper::ErrorThrower thrower,

View File

@@ -7,7 +7,6 @@
#include "shell/common/gin_converters/callback_converter.h"
#include "shell/common/gin_converters/value_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/gin_helper/handle.h"
#include "shell/common/node_includes.h"
#include "ui/gfx/animation/animation.h"

View File

@@ -367,13 +367,11 @@ void SystemPreferences::SetUserDefault(const std::string& name,
}
}
} else {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError("Invalid type: " + type);
args->ThrowTypeError("Invalid type: " + type);
return;
}
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError("Unable to convert value to: " + type);
args->ThrowTypeError("Unable to convert value to: " + type);
}
std::string SystemPreferences::GetAccentColor() {

View File

@@ -26,7 +26,6 @@
#include "shell/common/gin_converters/callback_converter.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/handle.h"
#include "shell/common/gin_helper/object_template_builder.h"
#include "shell/common/node_includes.h"
@@ -329,17 +328,17 @@ void UtilityProcessWrapper::Shutdown(uint64_t exit_code) {
HandleTermination(exit_code);
}
void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
void UtilityProcessWrapper::PostMessage(gin::Arguments* const args) {
if (!node_service_remote_.is_connected())
return;
blink::TransferableMessage transferable_message;
gin_helper::ErrorThrower thrower(args->isolate());
v8::Isolate* const isolate = args->isolate();
// |message| is any value that can be serialized to StructuredClone.
v8::Local<v8::Value> message_value;
if (args->GetNext(&message_value)) {
if (!electron::SerializeV8Value(args->isolate(), message_value,
if (!electron::SerializeV8Value(isolate, message_value,
&transferable_message)) {
// SerializeV8Value sets an exception.
return;
@@ -350,31 +349,30 @@ void UtilityProcessWrapper::PostMessage(gin::Arguments* args) {
std::vector<gin_helper::Handle<MessagePort>> wrapped_ports;
if (args->GetNext(&transferables)) {
std::vector<v8::Local<v8::Value>> wrapped_port_values;
if (!gin::ConvertFromV8(args->isolate(), transferables,
&wrapped_port_values)) {
thrower.ThrowTypeError("transferables must be an array of MessagePorts");
if (!gin::ConvertFromV8(isolate, transferables, &wrapped_port_values)) {
args->ThrowTypeError("transferables must be an array of MessagePorts");
return;
}
for (size_t i = 0; i < wrapped_port_values.size(); ++i) {
if (!gin_helper::IsValidWrappable(wrapped_port_values[i],
&MessagePort::kWrapperInfo)) {
thrower.ThrowTypeError(
args->ThrowTypeError(
base::StrCat({"Port at index ", base::NumberToString(i),
" is not a valid port"}));
return;
}
}
if (!gin::ConvertFromV8(args->isolate(), transferables, &wrapped_ports)) {
thrower.ThrowTypeError("Passed an invalid MessagePort");
if (!gin::ConvertFromV8(isolate, transferables, &wrapped_ports)) {
args->ThrowTypeError("Passed an invalid MessagePort");
return;
}
}
bool threw_exception = false;
transferable_message.ports = MessagePort::DisentanglePorts(
args->isolate(), wrapped_ports, &threw_exception);
transferable_message.ports =
MessagePort::DisentanglePorts(isolate, wrapped_ports, &threw_exception);
if (threw_exception)
return;
@@ -436,11 +434,10 @@ raw_ptr<UtilityProcessWrapper> UtilityProcessWrapper::FromProcessId(
// static
gin_helper::Handle<UtilityProcessWrapper> UtilityProcessWrapper::Create(
gin::Arguments* args) {
gin::Arguments* const args) {
if (!Browser::Get()->is_ready()) {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError(
"utilityProcess cannot be created before app is ready.");
args->ThrowTypeError(
"utilityProcess cannot be created before app is ready.");
return {};
}

View File

@@ -3031,23 +3031,24 @@ void OnPDFCreated(gin_helper::Promise<v8::Local<v8::Value>> promise,
}
} // namespace
void WebContents::Print(gin::Arguments* args) {
auto options = gin_helper::Dictionary::CreateEmpty(args->isolate());
base::Value::Dict settings;
void WebContents::Print(gin::Arguments* const args) {
v8::Isolate* const isolate = args->isolate();
auto options = gin_helper::Dictionary::CreateEmpty(isolate);
if (args->Length() >= 1 && !args->GetNext(&options)) {
gin_helper::ErrorThrower(args->isolate())
.ThrowError("webContents.print(): Invalid print settings specified.");
args->ThrowTypeError(
"webContents.print(): Invalid print settings specified.");
return;
}
printing::CompletionCallback callback;
if (args->Length() == 2 && !args->GetNext(&callback)) {
gin_helper::ErrorThrower(args->isolate())
.ThrowError("webContents.print(): Invalid optional callback provided.");
args->ThrowTypeError(
"webContents.print(): Invalid optional callback provided.");
return;
}
base::Value::Dict settings;
if (options.IsEmptyObject()) {
content::RenderFrameHost* rfh = GetRenderFrameHostToUse(web_contents());
if (!rfh)
@@ -3069,7 +3070,7 @@ void WebContents::Print(gin::Arguments* args) {
options.ValueOrDefault("printBackground", false));
// Set custom margin settings
auto margins = gin_helper::Dictionary::CreateEmpty(args->isolate());
auto margins = gin_helper::Dictionary::CreateEmpty(isolate);
if (options.Get("margins", &margins)) {
printing::mojom::MarginType margin_type =
printing::mojom::MarginType::kDefaultMargins;
@@ -3349,11 +3350,10 @@ void WebContents::ReplaceMisspelling(const std::u16string& word) {
web_contents()->ReplaceMisspelling(word);
}
uint32_t WebContents::FindInPage(gin::Arguments* args) {
uint32_t WebContents::FindInPage(gin::Arguments* const args) {
std::u16string search_text;
if (!args->GetNext(&search_text) || search_text.empty()) {
gin_helper::ErrorThrower(args->isolate())
.ThrowError("Must provide a non-empty search content");
args->ThrowTypeError("Must provide a non-empty search content");
return 0;
}

View File

@@ -18,7 +18,6 @@
#include "shell/browser/browser.h"
#include "shell/common/gin_converters/value_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/error_thrower.h"
namespace auto_updater {
@@ -31,8 +30,7 @@ bool g_update_available = false;
} // namespace
// static
void AutoUpdater::SetFeedURL(gin::Arguments* args) {
gin_helper::ErrorThrower thrower(args->isolate());
void AutoUpdater::SetFeedURL(gin::Arguments* const args) {
gin_helper::Dictionary opts;
std::string feed;
@@ -45,7 +43,7 @@ void AutoUpdater::SetFeedURL(gin::Arguments* args) {
}
} else if (args->GetNext(&opts)) {
if (!opts.Get("url", &feed)) {
thrower.ThrowError(
args->ThrowTypeError(
"Expected options object to contain a 'url' string property in "
"setFeedUrl call");
return;
@@ -53,11 +51,11 @@ void AutoUpdater::SetFeedURL(gin::Arguments* args) {
opts.Get("headers", &requestHeaders);
opts.Get("serverType", &serverType);
if (serverType != "default" && serverType != "json") {
thrower.ThrowError("Expected serverType to be 'default' or 'json'");
args->ThrowTypeError("Expected serverType to be 'default' or 'json'");
return;
}
} else {
thrower.ThrowError(
args->ThrowTypeError(
"Expected an options object with a 'url' property to be provided");
return;
}

View File

@@ -137,13 +137,10 @@ void Browser::Focus(gin::Arguments* args) {
gin_helper::Dictionary opts;
bool steal_focus = false;
if (args->GetNext(&opts)) {
gin_helper::ErrorThrower thrower(args->isolate());
if (!opts.Get("steal", &steal_focus)) {
thrower.ThrowError(
"Expected options object to contain a 'steal' boolean property");
return;
}
if (args->GetNext(&opts) && !opts.Get("steal", &steal_focus)) {
args->ThrowTypeError(
"Expected options object to contain a 'steal' boolean property");
return;
}
[[AtomApplication sharedApplication] activateIgnoringOtherApps:steal_focus];

View File

@@ -66,7 +66,6 @@
#include "shell/common/electron_paths.h"
#include "shell/common/gin_converters/frame_converter.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/options_switches.h"
#include "shell/common/thread_restrictions.h"
#include "third_party/blink/public/common/page/page_zoom.h"
@@ -681,7 +680,7 @@ void ElectronBrowserContext::SetDisplayMediaRequestHandler(
void ElectronBrowserContext::DisplayMediaDeviceChosen(
const content::MediaStreamRequest& request,
content::MediaResponseCallback callback,
gin::Arguments* args) {
gin::Arguments* const args) {
blink::mojom::StreamDevicesSetPtr stream_devices_set =
blink::mojom::StreamDevicesSet::New();
v8::Local<v8::Value> result;
@@ -693,10 +692,9 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
}
gin_helper::Dictionary result_dict;
if (!gin::ConvertFromV8(args->isolate(), result, &result_dict)) {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError(
"Display Media Request streams callback must be called with null "
"or a valid object");
args->ThrowTypeError(
"Display Media Request streams callback must be called with null "
"or a valid object");
std::move(callback).Run(
blink::mojom::StreamDevicesSet(),
blink::mojom::MediaStreamRequestResult::CAPTURE_FAILURE, nullptr);
@@ -735,9 +733,8 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
content::DesktopMediaID::Parse(video_device.id));
devices.video_device = video_device;
} else {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError(
"video must be a WebFrameMain or DesktopCapturerSource");
args->ThrowTypeError(
"video must be a WebFrameMain or DesktopCapturerSource");
std::move(callback).Run(
blink::mojom::StreamDevicesSet(),
blink::mojom::MediaStreamRequestResult::CAPTURE_FAILURE, nullptr);
@@ -784,10 +781,9 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
GetAudioDesktopMediaId(request.requested_audio_device_ids));
devices.audio_device = audio_device;
} else {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError(
"audio must be a WebFrameMain, \"loopback\" or "
"\"loopbackWithMute\"");
args->ThrowTypeError(
"audio must be a WebFrameMain, \"loopback\" or "
"\"loopbackWithMute\"");
std::move(callback).Run(
blink::mojom::StreamDevicesSet(),
blink::mojom::MediaStreamRequestResult::CAPTURE_FAILURE, nullptr);
@@ -796,9 +792,8 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
}
if ((video_requested && !has_video)) {
gin_helper::ErrorThrower(args->isolate())
.ThrowTypeError(
"Video was requested, but no video stream was provided");
args->ThrowTypeError(
"Video was requested, but no video stream was provided");
std::move(callback).Run(
blink::mojom::StreamDevicesSet(),
blink::mojom::MediaStreamRequestResult::CAPTURE_FAILURE, nullptr);

View File

@@ -26,8 +26,7 @@ class ElectronBundleMover {
static bool IsCurrentAppInApplicationsFolder();
private:
static bool ShouldContinueMove(gin_helper::ErrorThrower thrower,
BundlerMoverConflictType type,
static bool ShouldContinueMove(BundlerMoverConflictType type,
gin::Arguments* args);
};

View File

@@ -328,9 +328,9 @@ bool IsApplicationAtPathRunning(NSString* bundlePath) {
namespace electron {
bool ElectronBundleMover::ShouldContinueMove(gin_helper::ErrorThrower thrower,
BundlerMoverConflictType type,
gin::Arguments* args) {
bool ElectronBundleMover::ShouldContinueMove(
const BundlerMoverConflictType type,
gin::Arguments* const args) {
gin::Dictionary options(args->isolate());
bool hasOptions = args->GetNext(&options);
base::OnceCallback<v8::Local<v8::Value>(BundlerMoverConflictType)>
@@ -345,7 +345,7 @@ bool ElectronBundleMover::ShouldContinueMove(gin_helper::ErrorThrower thrower,
// we only want to throw an error if a user has returned a non-boolean
// value; this allows for client-side error handling should something in
// the handler throw
thrower.ThrowError("Invalid conflict handler return type.");
args->ThrowTypeError("Invalid conflict handler return type.");
}
}
return true;
@@ -406,8 +406,8 @@ bool ElectronBundleMover::Move(gin_helper::ErrorThrower thrower,
// But first, make sure that it's not running
if (IsApplicationAtPathRunning(destinationPath)) {
// Check for callback handler and get user choice for open/quit
if (!ShouldContinueMove(
thrower, BundlerMoverConflictType::kExistsAndRunning, args))
if (!ShouldContinueMove(BundlerMoverConflictType::kExistsAndRunning,
args))
return false;
// Unless explicitly denied, give running app focus and terminate self
@@ -420,8 +420,7 @@ bool ElectronBundleMover::Move(gin_helper::ErrorThrower thrower,
return true;
} else {
// Check callback handler and get user choice for app trashing
if (!ShouldContinueMove(thrower, BundlerMoverConflictType::kExists,
args))
if (!ShouldContinueMove(BundlerMoverConflictType::kExists, args))
return false;
// Unless explicitly denied, attempt to trash old app

View File

@@ -16,7 +16,6 @@
#endif
#include "shell/common/gin_converters/gfx_converter.h"
#include "shell/common/gin_converters/optional_converter.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/node_includes.h"
#include "shell/common/node_util.h"