Skip to content

Commit

Permalink
Convert SafeJsonParser to base::OnceCallback
Browse files Browse the repository at this point in the history
The callback passed to SafeJsonParser will only be invoked once, so
change the definition of the callbacks to be base::OnceCallback<>
to make this explicit.

Convert client code to use base::BindOnce() when interacting with
SafeJsonParser instead of base::Bind() or base::BindRepeating().

TBR=sky@chromium.org

Bug: 842655, 964232, 714018
Change-Id: Id948c848f5c193959cc0da2c8eadb21cc55a841c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1624810
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664288}
  • Loading branch information
sdefresne authored and Commit Bot committed May 29, 2019
1 parent a2004a8 commit ada1d84
Show file tree
Hide file tree
Showing 47 changed files with 259 additions and 248 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ void DigitalAssetLinksHandler::OnURLLoadComplete(
data_decoder::SafeJsonParser::Parse(
/* connector=*/nullptr, // Connector is unused on Android.
*response_body,
base::Bind(&DigitalAssetLinksHandler::OnJSONParseSucceeded,
weak_ptr_factory_.GetWeakPtr(), package, fingerprint,
relationship),
base::Bind(&DigitalAssetLinksHandler::OnJSONParseFailed,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&DigitalAssetLinksHandler::OnJSONParseSucceeded,
weak_ptr_factory_.GetWeakPtr(), package, fingerprint,
relationship),
base::BindOnce(&DigitalAssetLinksHandler::OnJSONParseFailed,
weak_ptr_factory_.GetWeakPtr()));

url_loader_.reset(nullptr);
}
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/android/explore_sites/ntp_json_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ void NTPJsonFetcher::OnSimpleLoaderComplete(
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
*response_body,
base::BindRepeating(&NTPJsonFetcher::OnJsonParseSuccess,
weak_factory_.GetWeakPtr()),
base::BindRepeating(&NTPJsonFetcher::OnJsonParseError,
weak_factory_.GetWeakPtr()));
base::BindOnce(&NTPJsonFetcher::OnJsonParseSuccess,
weak_factory_.GetWeakPtr()),
base::BindOnce(&NTPJsonFetcher::OnJsonParseError,
weak_factory_.GetWeakPtr()));
}

void NTPJsonFetcher::OnJsonParseSuccess(base::Value parsed_json) {
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/chromeos/arc/policy/arc_policy_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,9 @@ void ArcPolicyBridge::ReportCompliance(const std::string& request,
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
request,
base::Bind(&ArcPolicyBridge::OnReportComplianceParseSuccess,
weak_ptr_factory_.GetWeakPtr(), repeating_callback),
base::Bind(&OnReportComplianceParseFailure, repeating_callback));
base::BindOnce(&ArcPolicyBridge::OnReportComplianceParseSuccess,
weak_ptr_factory_.GetWeakPtr(), repeating_callback),
base::BindOnce(&OnReportComplianceParseFailure, repeating_callback));
}

void ArcPolicyBridge::ReportCloudDpsRequested(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ void IntentConfigHelper::ParseConfig(const std::string& json_config) {
// Parse JSON in utility process via Data Decoder Service.
data_decoder::SafeJsonParser::Parse(
connection->GetConnector(), json_config,
base::BindRepeating(&IntentConfigHelper::ParseConfigDone,
weak_factory_.GetWeakPtr()),
base::BindRepeating(&IntentConfigHelper::ParseConfigFailed,
weak_factory_.GetWeakPtr()));
base::BindOnce(&IntentConfigHelper::ParseConfigDone,
weak_factory_.GetWeakPtr()),
base::BindOnce(&IntentConfigHelper::ParseConfigFailed,
weak_factory_.GetWeakPtr()));
}

void IntentConfigHelper::ParseConfigDone(base::Value config_value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,9 @@ void CheckForSanitizedWhitelistOnTaskRunner(
data_decoder::JsonSanitizer::Sanitize(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
unsafe_json,
base::Bind(&OnWhitelistSanitizationResult, crx_id, task_runner, callback),
base::Bind(&OnWhitelistSanitizationError, whitelist_path));
base::BindOnce(&OnWhitelistSanitizationResult, crx_id, task_runner,
callback),
base::BindOnce(&OnWhitelistSanitizationError, whitelist_path));
}

void RemoveUnregisteredWhitelistsOnTaskRunner(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,11 @@ void ChromeManagementAPIDelegate::
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
manifest_str,
base::Bind(
base::BindOnce(
&extensions::ManagementGetPermissionWarningsByManifestFunction::
OnParseSuccess,
function),
base::Bind(
base::BindOnce(
&extensions::ManagementGetPermissionWarningsByManifestFunction::
OnParseFailure,
function));
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/extensions/webstore_data_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ void WebstoreDataFetcher::OnSimpleLoaderComplete(
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
*response_body,
base::Bind(&WebstoreDataFetcher::OnJsonParseSuccess, AsWeakPtr()),
base::Bind(&WebstoreDataFetcher::OnJsonParseFailure, AsWeakPtr()));
base::BindOnce(&WebstoreDataFetcher::OnJsonParseSuccess, AsWeakPtr()),
base::BindOnce(&WebstoreDataFetcher::OnJsonParseFailure, AsWeakPtr()));
}

} // namespace extensions
5 changes: 3 additions & 2 deletions chrome/browser/extensions/webstore_install_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ void WebstoreInstallHelper::Start(

data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
manifest_, base::Bind(&WebstoreInstallHelper::OnJSONParseSucceeded, this),
base::Bind(&WebstoreInstallHelper::OnJSONParseFailed, this));
manifest_,
base::BindOnce(&WebstoreInstallHelper::OnJSONParseSucceeded, this),
base::BindOnce(&WebstoreInstallHelper::OnJSONParseFailed, this));

if (icon_url_.is_empty()) {
icon_decode_complete_ = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ std::unique_ptr<InvalidationService> CreateInvalidationServiceForSenderId(
->driver(),
profile->GetPrefs(),
base::BindRepeating(
data_decoder::SafeJsonParser::Parse,
&data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()->GetConnector()),
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess()
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/media/router/data_decoder_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ void DataDecoder::ParseXml(const std::string& unsafe_xml,

void DataDecoder::ParseJson(
const std::string& unsafe_json,
const data_decoder::SafeJsonParser::SuccessCallback& success_callback,
const data_decoder::SafeJsonParser::ErrorCallback& error_callback) {
data_decoder::SafeJsonParser::ParseBatch(connector_.get(), unsafe_json,
success_callback, error_callback,
kDataDecoderServiceBatchId);
data_decoder::SafeJsonParser::SuccessCallback success_callback,
data_decoder::SafeJsonParser::ErrorCallback error_callback) {
data_decoder::SafeJsonParser::ParseBatch(
connector_.get(), unsafe_json, std::move(success_callback),
std::move(error_callback), kDataDecoderServiceBatchId);
}

} // namespace media_router
7 changes: 3 additions & 4 deletions chrome/browser/media/router/data_decoder_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ class DataDecoder {
void ParseXml(const std::string& unsafe_xml,
data_decoder::XmlParserCallback callback);

void ParseJson(
const std::string& unsafe_json,
const data_decoder::SafeJsonParser::SuccessCallback& success_callback,
const data_decoder::SafeJsonParser::ErrorCallback& error_callback);
void ParseJson(const std::string& unsafe_json,
data_decoder::SafeJsonParser::SuccessCallback success_callback,
data_decoder::SafeJsonParser::ErrorCallback error_callback);

private:
std::unique_ptr<service_manager::Connector> connector_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void RegisterArticleProviderIfEnabled(ContentSuggestionsService* service,
#endif // BUILDFLAG(ENABLE_OFFLINE_PAGES)
auto suggestions_fetcher = std::make_unique<RemoteSuggestionsFetcherImpl>(
identity_manager, url_loader_factory, pref_service, language_histogram,
base::Bind(
base::BindRepeating(
&data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()->GetConnector()),
GetFetchEndpoint(), api_key, user_classifier);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ntp_tiles/chrome_popular_sites_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ChromePopularSitesFactory::NewForProfile(Profile* profile) {
g_browser_process->variations_service(),
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess(),
base::Bind(
data_decoder::SafeJsonParser::Parse,
base::BindRepeating(
&data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()->GetConnector()));
}
6 changes: 3 additions & 3 deletions chrome/browser/plugins/plugins_resource_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ PluginsResourceService::PluginsResourceService(PrefService* local_state)
g_browser_process->system_network_context_manager()
->GetSharedURLLoaderFactory(),
switches::kDisableBackgroundNetworking,
base::Bind(data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()
->GetConnector()),
base::BindRepeating(&data_decoder::SafeJsonParser::Parse,
content::ServiceManagerConnection::GetForProcess()
->GetConnector()),
kPluginResourceServiceTrafficAnnotation,
base::BindOnce(&content::GetNetworkConnectionTracker)) {}

Expand Down
25 changes: 13 additions & 12 deletions chrome/browser/safe_json_parser_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,28 @@ class SafeJsonParserTest : public InProcessBrowserTest {
SafeJsonParser::SuccessCallback success_callback;
SafeJsonParser::ErrorCallback error_callback;
if (value_with_error.value) {
success_callback =
base::Bind(&SafeJsonParserTest::ExpectValue, base::Unretained(this),
base::Passed(std::move(*value_with_error.value)));
error_callback = base::Bind(&SafeJsonParserTest::FailWithError,
base::Unretained(this));
success_callback = base::BindOnce(&SafeJsonParserTest::ExpectValue,
base::Unretained(this),
std::move(*value_with_error.value));
error_callback = base::BindOnce(&SafeJsonParserTest::FailWithError,
base::Unretained(this));
} else {
success_callback = base::Bind(&SafeJsonParserTest::FailWithValue,
base::Unretained(this));
error_callback =
base::Bind(&SafeJsonParserTest::ExpectError, base::Unretained(this),
value_with_error.error_message);
success_callback = base::BindOnce(&SafeJsonParserTest::FailWithValue,
base::Unretained(this));
error_callback = base::BindOnce(&SafeJsonParserTest::ExpectError,
base::Unretained(this),
value_with_error.error_message);
}

if (batch_id) {
SafeJsonParser::ParseBatch(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
json, success_callback, error_callback, *batch_id);
json, std::move(success_callback), std::move(error_callback),
*batch_id);
} else {
SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
json, success_callback, error_callback);
json, std::move(success_callback), std::move(error_callback));
}

message_loop_runner_->Run();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ void OneGoogleBarLoaderImpl::LoadDone(
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
response,
base::BindRepeating(&OneGoogleBarLoaderImpl::JsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindRepeating(&OneGoogleBarLoaderImpl::JsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&OneGoogleBarLoaderImpl::JsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OneGoogleBarLoaderImpl::JsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
}

void OneGoogleBarLoaderImpl::JsonParsed(base::Value value) {
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/search/promos/promo_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ void PromoService::OnLoadDone(std::unique_ptr<std::string> response_body) {
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
response,
base::BindRepeating(&PromoService::OnJsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindRepeating(&PromoService::OnJsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&PromoService::OnJsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&PromoService::OnJsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
}

void PromoService::OnJsonParsed(base::Value value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ void SearchSuggestLoaderImpl::LoadDone(
data_decoder::SafeJsonParser::Parse(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
response,
base::BindRepeating(&SearchSuggestLoaderImpl::JsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindRepeating(&SearchSuggestLoaderImpl::JsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&SearchSuggestLoaderImpl::JsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&SearchSuggestLoaderImpl::JsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
}

void SearchSuggestLoaderImpl::JsonParsed(base::Value value) {
Expand Down
8 changes: 4 additions & 4 deletions components/cast_channel/cast_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,10 @@ void CastMessageHandler::OnMessage(const CastSocket& socket,
cast_channel::CastMessage_PayloadType_STRING) {
data_decoder::SafeJsonParser::ParseBatch(
connector_.get(), message.payload_utf8(),
base::BindRepeating(&CastMessageHandler::HandleCastInternalMessage,
weak_ptr_factory_.GetWeakPtr(), socket.id(),
message.source_id(), message.destination_id()),
base::BindRepeating(&ReportParseError), data_decoder_batch_id_);
base::BindOnce(&CastMessageHandler::HandleCastInternalMessage,
weak_ptr_factory_.GetWeakPtr(), socket.id(),
message.source_id(), message.destination_id()),
base::BindOnce(&ReportParseError), data_decoder_batch_id_);
} else {
DLOG(ERROR) << "Dropping internal message with binary payload: "
<< message.namespace_();
Expand Down
5 changes: 2 additions & 3 deletions components/invalidation/public/invalidation_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,10 @@ enum class HandlerOwnerType {
class Invalidation;
class InvalidationHandler;

// TODO(https://crbug.com/842655): Convert Repeating to Once.
using ParseJSONCallback = base::RepeatingCallback<void(
const std::string& unsafe_json,
const base::RepeatingCallback<void(base::Value)>& success_callback,
const base::RepeatingCallback<void(const std::string&)>& error_callback)>;
base::OnceCallback<void(base::Value)> success_callback,
base::OnceCallback<void(const std::string&)> error_callback)>;

struct INVALIDATION_EXPORT ObjectIdLessThan {
bool operator()(const invalidation::ObjectId& lhs,
Expand Down
8 changes: 4 additions & 4 deletions components/journey/journey_info_json_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ namespace journey {
// serializing the request body protos.
class JourneyInfoJsonRequest {
// Callbacks for JSON parsing to allow injecting platform-dependent code.
using SuccessCallback = base::RepeatingCallback<void(base::Value result)>;
using ErrorCallback = base::RepeatingCallback<void(const std::string& error)>;
using SuccessCallback = base::OnceCallback<void(base::Value result)>;
using ErrorCallback = base::OnceCallback<void(const std::string& error)>;
using ParseJSONCallback =
base::RepeatingCallback<void(const std::string& raw_json_string,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback)>;
SuccessCallback success_callback,
ErrorCallback error_callback)>;
using CompletedCallback =
base::OnceCallback<void(base::Optional<base::Value> result,
const std::string& error_detail)>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,20 @@ class MockSnippetsAvailableCallback {
};

void ParseJson(const std::string& json,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback) {
SuccessCallback success_callback,
ErrorCallback error_callback) {
base::JSONReader json_reader;
base::Optional<base::Value> value = json_reader.ReadToValue(json);
if (value) {
success_callback.Run(std::move(*value));
std::move(success_callback).Run(std::move(*value));
} else {
error_callback.Run(json_reader.GetErrorMessage());
std::move(error_callback).Run(json_reader.GetErrorMessage());
}
}

void ParseJsonDelayed(const std::string& json,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback) {
SuccessCallback success_callback,
ErrorCallback error_callback) {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&ParseJson, json, std::move(success_callback),
Expand Down
8 changes: 4 additions & 4 deletions components/ntp_snippets/remote/request_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ struct RequestParams {
};

// Callbacks for JSON parsing to allow injecting platform-dependent code.
using SuccessCallback = base::Callback<void(base::Value result)>;
using ErrorCallback = base::Callback<void(const std::string& error)>;
using SuccessCallback = base::OnceCallback<void(base::Value result)>;
using ErrorCallback = base::OnceCallback<void(const std::string& error)>;
using ParseJSONCallback =
base::Callback<void(const std::string& raw_json_string,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback)>;
SuccessCallback success_callback,
ErrorCallback error_callback)>;

} // namespace ntp_snippets

Expand Down
12 changes: 6 additions & 6 deletions components/ntp_tiles/popular_sites_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,12 @@ PopularSitesImpl::PopularSitesImpl(
const TemplateURLService* template_url_service,
VariationsService* variations_service,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
ParseJSONCallback parse_json)
const ParseJSONCallback& parse_json)
: prefs_(prefs),
template_url_service_(template_url_service),
variations_(variations_service),
url_loader_factory_(std::move(url_loader_factory)),
parse_json_(std::move(parse_json)),
parse_json_(parse_json),
is_fallback_(false),
sections_(
ParseSites(*prefs->GetList(prefs::kPopularSitesJsonPref),
Expand Down Expand Up @@ -465,10 +465,10 @@ void PopularSitesImpl::OnSimpleLoaderComplete(
}

parse_json_.Run(*response_body,
base::Bind(&PopularSitesImpl::OnJsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&PopularSitesImpl::OnJsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&PopularSitesImpl::OnJsonParsed,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&PopularSitesImpl::OnJsonParseFailed,
weak_ptr_factory_.GetWeakPtr()));
}

void PopularSitesImpl::OnJsonParsed(base::Value json) {
Expand Down
Loading

0 comments on commit ada1d84

Please sign in to comment.