0

[Extensions] Rename ErrorWithArguments to ErrorWithArgumentsDoNotUse

This is to discourage adding additional callers to this function.
No behavior change is expected.

Additionally a followup change will add a presubmit warning if a new
caller is added.

Bug: 40944897
Change-Id: Ib92249a624c0de372c5278af7b9c386af5bcafb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6444141
Commit-Queue: Tim <tjudkins@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1445367}
This commit is contained in:
Tim Judkins
2025-04-10 10:26:17 -07:00
committed by Chromium LUCI CQ
parent a10210e9ba
commit 251a2a4a75
13 changed files with 47 additions and 44 deletions
chrome/browser
apps
platform_apps
api
media_galleries
extensions
extensions/browser

@ -809,8 +809,8 @@ void MediaGalleriesAddGalleryWatchFunction::OnPreferencesInit(
api::media_galleries::AddGalleryWatchResult result;
result.gallery_id = kInvalidGalleryId;
result.success = false;
Respond(ErrorWithArguments(AddGalleryWatch::Results::Create(result),
kInvalidGalleryIdMsg));
Respond(ErrorWithArgumentsDoNotUse(AddGalleryWatch::Results::Create(result),
kInvalidGalleryIdMsg));
return;
}
@ -834,14 +834,14 @@ void MediaGalleriesAddGalleryWatchFunction::HandleResponse(
if (!api->ExtensionHasGalleryChangeListener(extension()->id())) {
result.success = false;
Respond(ErrorWithArguments(AddGalleryWatch::Results::Create(result),
kMissingEventListener));
Respond(ErrorWithArgumentsDoNotUse(AddGalleryWatch::Results::Create(result),
kMissingEventListener));
return;
}
result.success = error.empty();
Respond(error.empty() ? WithArguments(result.ToValue())
: ErrorWithArguments(
: ErrorWithArgumentsDoNotUse(
AddGalleryWatch::Results::Create(result), error));
}

@ -128,8 +128,8 @@ void GcmRegisterFunction::CompleteFunctionWithResult(
Respond(succeeded
? ArgumentList(std::move(result))
// TODO(lazyboy): We shouldn't be using |result| in case of error.
: ErrorWithArguments(std::move(result),
GcmResultToError(gcm_result)));
: ErrorWithArgumentsDoNotUse(std::move(result),
GcmResultToError(gcm_result)));
}
GcmUnregisterFunction::GcmUnregisterFunction() = default;
@ -186,8 +186,8 @@ void GcmSendFunction::CompleteFunctionWithResult(
Respond(succeeded
? ArgumentList(std::move(result))
// TODO(lazyboy): We shouldn't be using |result| in case of error.
: ErrorWithArguments(std::move(result),
GcmResultToError(gcm_result)));
: ErrorWithArgumentsDoNotUse(std::move(result),
GcmResultToError(gcm_result)));
}
bool GcmSendFunction::ValidateMessageData(const gcm::MessageData& data) const {

@ -165,7 +165,7 @@ ExtensionFunction::ResponseAction InputImeSetCompositionFunction::Run() {
segments, &error)) {
base::Value::List results;
results.Append(false);
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
std::move(results), InformativeError(error, static_function_name())));
}
return RespondNow(WithArguments(true));
@ -185,7 +185,7 @@ ExtensionFunction::ResponseAction InputImeCommitTextFunction::Run() {
&error)) {
base::Value::List results;
results.Append(false);
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
std::move(results), InformativeError(error, static_function_name())));
}
return RespondNow(WithArguments(true));

@ -989,7 +989,7 @@ ExtensionFunction::ResponseAction InputImeClearCompositionFunction::Run() {
results.Append(success);
return RespondNow(success
? ArgumentList(std::move(results))
: ErrorWithArguments(
: ErrorWithArgumentsDoNotUse(
std::move(results),
InformativeError(error, static_function_name())));
}
@ -1080,7 +1080,7 @@ InputImeSetCandidateWindowPropertiesFunction::Run() {
!engine->SetCandidateWindowVisible(*properties.visible, &error)) {
base::Value::List results;
results.Append(false);
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
std::move(results), InformativeError(error, static_function_name())));
}
@ -1173,7 +1173,7 @@ ExtensionFunction::ResponseAction InputImeSetCandidatesFunction::Run() {
results.Append(success);
return RespondNow(success
? ArgumentList(std::move(results))
: ErrorWithArguments(
: ErrorWithArgumentsDoNotUse(
std::move(results),
InformativeError(error, static_function_name())));
}
@ -1197,7 +1197,7 @@ ExtensionFunction::ResponseAction InputImeSetCursorPositionFunction::Run() {
results.Append(success);
return RespondNow(success
? ArgumentList(std::move(results))
: ErrorWithArguments(
: ErrorWithArgumentsDoNotUse(
std::move(results),
InformativeError(error, static_function_name())));
}

@ -598,7 +598,7 @@ NotificationsCreateFunction::RunNotificationsApi() {
// TODO(dewittj): Add more human-readable error strings if this fails.
std::string error;
if (!CreateNotification(notification_id, &params_->options, &error)) {
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::notifications::Create::Results::Create(notification_id), error));
}
@ -635,7 +635,7 @@ NotificationsUpdateFunction::RunNotificationsApi() {
bool could_update_notification = UpdateNotification(
params_->notification_id, &params_->options, &notification, &error);
if (!could_update_notification) {
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::notifications::Update::Results::Create(false), error));
}

@ -849,7 +849,7 @@ WebstorePrivateBeginInstallWithManifest3Function::BuildResponse(
// In the cases where they do not correspond we should add a new enum value.
// We will need to ensure that the Webstore is entirely basing its logic on
// the result alone before removing the error.
return ErrorWithArguments(
return ErrorWithArgumentsDoNotUse(
BeginInstallWithManifest3::Results::Create(result), error);
}
@ -1220,7 +1220,7 @@ WebstorePrivateGetReferrerChainFunction::Run() {
: nullptr;
if (!outermost_render_frame_host) {
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::webstore_private::GetReferrerChain::Results::Create(""),
kWebstoreUserCancelledError));
}

@ -166,7 +166,7 @@ ExtensionFunction::ResponseValue SocketApiFunction::ErrorWithCode(
const std::string& error) {
base::Value::List args;
args.Append(error_code);
return ErrorWithArguments(std::move(args), error);
return ErrorWithArgumentsDoNotUse(std::move(args), error);
}
std::string SocketApiFunction::GetOriginId() const {
@ -415,7 +415,7 @@ ExtensionFunction::ResponseAction SocketDisconnectFunction::Work() {
base::Value::List args;
args.Append(base::Value());
return RespondNow(
ErrorWithArguments(std::move(args), kSocketNotFoundError));
ErrorWithArgumentsDoNotUse(std::move(args), kSocketNotFoundError));
}
}
@ -537,7 +537,7 @@ ExtensionFunction::ResponseAction SocketAcceptFunction::Work() {
} else {
api::socket::AcceptInfo info;
info.result_code = net::ERR_FAILED;
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::Accept::Results::Create(info), kSocketNotFoundError));
}
}
@ -572,7 +572,7 @@ ExtensionFunction::ResponseAction SocketReadFunction::Work() {
if (!socket) {
api::socket::ReadInfo info;
info.result_code = -1;
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::Read::Results::Create(info), kSocketNotFoundError));
}
@ -619,7 +619,7 @@ ExtensionFunction::ResponseAction SocketWriteFunction::Work() {
if (!socket) {
api::socket::WriteInfo info;
info.bytes_written = -1;
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::Write::Results::Create(info), kSocketNotFoundError));
}
@ -650,7 +650,7 @@ ExtensionFunction::ResponseAction SocketRecvFromFunction::Work() {
api::socket::RecvFromInfo info;
info.result_code = -1;
info.port = 0;
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::RecvFrom::Results::Create(info), kSocketNotFoundError));
}
@ -765,9 +765,9 @@ ExtensionFunction::ResponseAction SocketSetKeepAliveFunction::Work() {
Socket* socket = GetSocket(params->socket_id);
if (!socket) {
return RespondNow(
ErrorWithArguments(api::socket::SetKeepAlive::Results::Create(false),
kSocketNotFoundError));
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::SetKeepAlive::Results::Create(false),
kSocketNotFoundError));
}
int delay = 0;
if (params->delay) {
@ -794,7 +794,7 @@ ExtensionFunction::ResponseAction SocketSetNoDelayFunction::Work() {
Socket* socket = GetSocket(params->socket_id);
if (!socket) {
return RespondNow(ErrorWithArguments(
return RespondNow(ErrorWithArgumentsDoNotUse(
api::socket::SetNoDelay::Results::Create(false), kSocketNotFoundError));
}
socket->SetNoDelay(

@ -138,8 +138,8 @@ class SocketApiFunction : public ExtensionFunction {
// ExtensionFunction:
ResponseAction Run() final;
// Convenience wrapper for ErrorWithArguments(), where the arguments are just
// one integer value.
// Convenience wrapper for ErrorWithArgumentsDoNotUse(), where the arguments
// are just one integer value.
ResponseValue ErrorWithCode(int error_code, const std::string& error);
// Either extension_id() or url origin for CrOS Terminal.

@ -399,8 +399,8 @@ void SocketsTcpSendFunction::SetSendResult(int net_result, int bytes_sent) {
if (net_result == net::OK) {
Respond(ArgumentList(std::move(args)));
} else {
Respond(
ErrorWithArguments(std::move(args), net::ErrorToString(net_result)));
Respond(ErrorWithArgumentsDoNotUse(std::move(args),
net::ErrorToString(net_result)));
}
}

@ -314,8 +314,8 @@ void SocketsUdpSendFunction::SetSendResult(int net_result, int bytes_sent) {
if (net_result == net::OK) {
Respond(ArgumentList(std::move(args)));
} else {
Respond(
ErrorWithArguments(std::move(args), net::ErrorToString(net_result)));
Respond(ErrorWithArgumentsDoNotUse(std::move(args),
net::ErrorToString(net_result)));
}
}

@ -485,8 +485,8 @@ void UsbTransferFunction::OnCompleted(UsbTransferStatus status,
error_args.Append(std::move(transfer_info));
// Using ErrorWithArguments is discouraged but required to provide the
// detailed transfer info as the transfer may have partially succeeded.
Respond(ErrorWithArguments(std::move(error_args),
ConvertTransferStatusToApi(status)));
Respond(ErrorWithArgumentsDoNotUse(std::move(error_args),
ConvertTransferStatusToApi(status)));
}
}
@ -1345,7 +1345,8 @@ void UsbResetDeviceFunction::OnComplete(bool success) {
error_args.Append(false);
// Using ErrorWithArguments is discouraged but required to maintain
// compatibility with existing applications.
Respond(ErrorWithArguments(std::move(error_args), kErrorResetDevice));
Respond(
ErrorWithArgumentsDoNotUse(std::move(error_args), kErrorResetDevice));
}
}

@ -627,7 +627,7 @@ ExtensionFunction::ResponseValue ExtensionFunction::Error(std::string error) {
return CreateErrorResponseValue(std::move(error));
}
ExtensionFunction::ResponseValue ExtensionFunction::ErrorWithArguments(
ExtensionFunction::ResponseValue ExtensionFunction::ErrorWithArgumentsDoNotUse(
base::Value::List args,
const std::string& error) {
return CreateErrorWithArgumentsResponse(std::move(args), error);

@ -500,11 +500,13 @@ class ExtensionFunction : public base::RefCountedThreadSafe<
extensions::ErrorUtils::FormatErrorMessage(format, args...));
}
// Error with a list of arguments |args| to pass to caller.
// Using this ResponseValue indicates something is wrong with the API.
// It shouldn't be possible to have both an error *and* some arguments.
// Some legacy APIs do rely on it though, like webstorePrivate.
ResponseValue ErrorWithArguments(base::Value::List args,
const std::string& error);
// Using this ResponseValue is incompatible with promise based returns and
// indicates something is wrong with the API. If you are trying to use this,
// you likely instead want to be returning a value indicating if the API call
// was a "success" and/or an enum indicating what may have gone wrong.
// Some legacy APIs do still rely on this though.
ResponseValue ErrorWithArgumentsDoNotUse(base::Value::List args,
const std::string& error);
// Bad message. A ResponseValue equivalent to EXTENSION_FUNCTION_VALIDATE(),
// so this will actually kill the renderer and not respond at all.
ResponseValue BadMessage();