From c8edc4c427a5ff191039204965ff04808bf3b585 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Fri, 7 May 2021 23:57:21 +0000 Subject: [PATCH] [Code Health] Remove uses of Value::GetAsDictionary() in remoting/ Bug: 1187011 Change-Id: Ibecabce4df1cd169bbdb5bbb0ae6ffc2951657e1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2875960 Auto-Submit: Austin Sullivan Commit-Queue: Yuwei Huang Reviewed-by: Yuwei Huang Cr-Commit-Position: refs/heads/master@{#880632} --- .../host/pairing_registry_delegate_linux.cc | 6 +-- .../security_key_extension_session.cc | 49 +++++++++++-------- .../security_key_extension_session.h | 9 ++-- remoting/host/token_validator_base.cc | 15 +++--- remoting/protocol/ice_config.cc | 39 ++++++++------- 5 files changed, 63 insertions(+), 55 deletions(-) diff --git a/remoting/host/pairing_registry_delegate_linux.cc b/remoting/host/pairing_registry_delegate_linux.cc index cbe034c66fc535..ca9cd35331a68d 100644 --- a/remoting/host/pairing_registry_delegate_linux.cc +++ b/remoting/host/pairing_registry_delegate_linux.cc @@ -98,13 +98,13 @@ PairingRegistry::Pairing PairingRegistryDelegateLinux::Load( return PairingRegistry::Pairing(); } - base::DictionaryValue* pairing_dictionary; - if (!pairing->GetAsDictionary(&pairing_dictionary)) { + if (!pairing->is_dict()) { LOG(WARNING) << "Failed to parse pairing information: not a dictionary."; return PairingRegistry::Pairing(); } - return PairingRegistry::Pairing::CreateFromValue(*pairing_dictionary); + return PairingRegistry::Pairing::CreateFromValue( + base::Value::AsDictionaryValue(*pairing)); } bool PairingRegistryDelegateLinux::Save( diff --git a/remoting/host/security_key/security_key_extension_session.cc b/remoting/host/security_key/security_key_extension_session.cc index 643f46689df53e..e9d66f0337efc8 100644 --- a/remoting/host/security_key/security_key_extension_session.cc +++ b/remoting/host/security_key/security_key_extension_session.cc @@ -4,6 +4,7 @@ #include "remoting/host/security_key/security_key_extension_session.h" +#include #include #include "base/bind.h" @@ -42,18 +43,18 @@ unsigned int GetCommandCode(const std::string& data) { // Creates a string of byte data from a ListValue of numbers. Returns true if // all of the list elements are numbers. -bool ConvertListValueToString(base::ListValue* bytes, std::string* out) { +bool ConvertListToString(const base::Value& bytes, std::string* out) { out->clear(); - unsigned int byte_count = bytes->GetSize(); + unsigned int byte_count = bytes.GetList().size(); if (byte_count != 0) { out->reserve(byte_count); for (unsigned int i = 0; i < byte_count; i++) { - int value; - if (!bytes->GetInteger(i, &value)) { + auto value = bytes.GetList()[i].GetIfInt(); + if (!value.has_value()) { return false; } - out->push_back(static_cast(value)); + out->push_back(static_cast(value.value())); } } return true; @@ -97,18 +98,19 @@ bool SecurityKeyExtensionSession::OnExtensionMessage( std::unique_ptr value = base::JSONReader::ReadDeprecated(message.data()); - base::DictionaryValue* client_message; - if (!value || !value->GetAsDictionary(&client_message)) { + if (!value || !value->is_dict()) { LOG(WARNING) << "Failed to retrieve data from gnubby-auth message."; return true; } - std::string type; - if (!client_message->GetString(kMessageType, &type)) { + std::string* maybe_type = value->FindStringKey(kMessageType); + if (!maybe_type) { LOG(WARNING) << "Invalid gnubby-auth message format."; return true; } + std::string type = *maybe_type; + base::Value::DictStorage client_message = value->TakeDict(); if (type == kControlMessage) { ProcessControlMessage(client_message); } else if (type == kDataMessage) { @@ -123,12 +125,13 @@ bool SecurityKeyExtensionSession::OnExtensionMessage( } void SecurityKeyExtensionSession::ProcessControlMessage( - base::DictionaryValue* message_data) const { - std::string option; - if (!message_data->GetString(kControlOption, &option)) { + const base::Value::DictStorage& message_data) const { + auto option_iter = message_data.find(kControlOption); + if (option_iter == message_data.end() || !option_iter->second.is_string()) { LOG(WARNING) << "Could not extract control option from message."; return; } + auto option = option_iter->second.GetString(); if (option == kSecurityKeyAuthV1) { security_key_auth_handler_->CreateSecurityKeyConnection(); @@ -138,12 +141,14 @@ void SecurityKeyExtensionSession::ProcessControlMessage( } void SecurityKeyExtensionSession::ProcessDataMessage( - base::DictionaryValue* message_data) const { - int connection_id; - if (!message_data->GetInteger(kConnectionId, &connection_id)) { + const base::Value::DictStorage& message_data) const { + auto connection_id_iter = message_data.find(kConnectionId); + if (connection_id_iter == message_data.end() || + !connection_id_iter->second.is_int()) { LOG(WARNING) << "Could not extract connection id from message."; return; } + auto connection_id = connection_id_iter->second.GetInt(); if (!security_key_auth_handler_->IsValidConnectionId(connection_id)) { LOG(WARNING) << "Unknown gnubby-auth data connection: '" << connection_id @@ -151,10 +156,10 @@ void SecurityKeyExtensionSession::ProcessDataMessage( return; } - base::ListValue* bytes; std::string response; - if (message_data->GetList(kDataPayload, &bytes) && - ConvertListValueToString(bytes, &response)) { + auto bytes_iter = message_data.find(kDataPayload); + if (bytes_iter != message_data.end() && + ConvertListToString(bytes_iter->second, &response)) { HOST_LOG << "Sending security key response: " << GetCommandCode(response); security_key_auth_handler_->SendClientResponse(connection_id, response); } else { @@ -165,12 +170,14 @@ void SecurityKeyExtensionSession::ProcessDataMessage( } void SecurityKeyExtensionSession::ProcessErrorMessage( - base::DictionaryValue* message_data) const { - int connection_id; - if (!message_data->GetInteger(kConnectionId, &connection_id)) { + const base::Value::DictStorage& message_data) const { + auto connection_id_iter = message_data.find(kConnectionId); + if (connection_id_iter == message_data.end() || + !connection_id_iter->second.is_int()) { LOG(WARNING) << "Could not extract connection id from message."; return; } + auto connection_id = connection_id_iter->second.GetInt(); if (security_key_auth_handler_->IsValidConnectionId(connection_id)) { HOST_LOG << "Sending security key error"; diff --git a/remoting/host/security_key/security_key_extension_session.h b/remoting/host/security_key/security_key_extension_session.h index f0365a59d6a4d1..1858eec0519415 100644 --- a/remoting/host/security_key/security_key_extension_session.h +++ b/remoting/host/security_key/security_key_extension_session.h @@ -12,10 +12,10 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/threading/thread_checker.h" +#include "base/values.h" #include "remoting/host/host_extension_session.h" namespace base { -class DictionaryValue; class SingleThreadTaskRunner; } // namespace base @@ -48,9 +48,10 @@ class SecurityKeyExtensionSession : public HostExtensionSession { private: // These methods process specific security key extension message types. - void ProcessControlMessage(base::DictionaryValue* message_data) const; - void ProcessDataMessage(base::DictionaryValue* message_data) const; - void ProcessErrorMessage(base::DictionaryValue* message_data) const; + void ProcessControlMessage( + const base::Value::DictStorage& message_data) const; + void ProcessDataMessage(const base::Value::DictStorage& message_data) const; + void ProcessErrorMessage(const base::Value::DictStorage& message_data) const; void SendMessageToClient(int connection_id, const std::string& data) const; diff --git a/remoting/host/token_validator_base.cc b/remoting/host/token_validator_base.cc index c022970083c0cf..52e08736c5b55f 100644 --- a/remoting/host/token_validator_base.cc +++ b/remoting/host/token_validator_base.cc @@ -301,24 +301,21 @@ std::string TokenValidatorBase::ProcessResponse(int net_result) { : data_; base::Optional value = base::JSONReader::Read(responseData); - base::DictionaryValue* dict; - if (!value || !value->GetAsDictionary(&dict)) { + if (!value || !value->is_dict()) { LOG(ERROR) << "Invalid token validation response: '" << data_ << "'"; return std::string(); } - std::string token_scope; - dict->GetStringWithoutPathExpansion("scope", &token_scope); - if (!IsValidScope(token_scope)) { - LOG(ERROR) << "Invalid scope: '" << token_scope << "', expected: '" + std::string* token_scope = value->FindStringKey("scope"); + if (!token_scope || !IsValidScope(*token_scope)) { + LOG(ERROR) << "Invalid scope: '" << *token_scope << "', expected: '" << token_scope_ << "'."; return std::string(); } - std::string shared_secret; // Everything is valid, so return the shared secret to the caller. - dict->GetStringWithoutPathExpansion("access_token", &shared_secret); - return shared_secret; + std::string* shared_secret = value->FindStringKey("access_token"); + return shared_secret ? *shared_secret : std::string(); } } // namespace remoting diff --git a/remoting/protocol/ice_config.cc b/remoting/protocol/ice_config.cc index c7bf3e3f28a797..ba80a52b110f39 100644 --- a/remoting/protocol/ice_config.cc +++ b/remoting/protocol/ice_config.cc @@ -138,33 +138,37 @@ IceConfig IceConfig::Parse(const base::DictionaryValue& dictionary) { // Parse iceServers list and store them in |ice_config|. bool errors_found = false; ice_config.max_bitrate_kbps = 0; - for (const auto& server : ice_servers_list->GetList()) { - const base::DictionaryValue* server_dict; - if (!server.GetAsDictionary(&server_dict)) { + for (const auto& server : *ice_servers_list) { + if (!server.is_dict()) { errors_found = true; continue; } - const base::ListValue* urls_list = nullptr; - if (!server_dict->GetList("urls", &urls_list)) { + const base::Value* urls_list = server.FindListKey("urls"); + if (!urls_list) { errors_found = true; continue; } std::string username; - server_dict->GetString("username", &username); + const std::string* maybe_username = server.FindStringKey("username"); + if (maybe_username) + username = *maybe_username; std::string password; - server_dict->GetString("credential", &password); + const std::string* maybe_password = server.FindStringKey("credential"); + if (maybe_password) + password = *maybe_password; // Compute the lowest specified bitrate of all the ICE servers. // Ideally the bitrate would be stored per ICE server, but it is not // possible (at the application level) to look up which particular // ICE server was used for the P2P connection. - double new_bitrate_double; - if (server_dict->GetDouble("maxRateKbps", &new_bitrate_double)) { - ice_config.max_bitrate_kbps = MinimumSpecified( - ice_config.max_bitrate_kbps, static_cast(new_bitrate_double)); + auto new_bitrate_double = server.FindDoubleKey("maxRateKbps"); + if (new_bitrate_double.has_value()) { + ice_config.max_bitrate_kbps = + MinimumSpecified(ice_config.max_bitrate_kbps, + static_cast(new_bitrate_double.value())); } for (const auto& url : urls_list->GetList()) { @@ -206,20 +210,19 @@ IceConfig IceConfig::Parse(const std::string& config_json) { return IceConfig(); } - base::DictionaryValue* dictionary = nullptr; - if (!json->GetAsDictionary(&dictionary)) { + if (!json->is_dict()) { return IceConfig(); } // Handle the case when the config is wrapped in 'data', i.e. as {'data': { // 'iceServers': {...} }}. - base::DictionaryValue* data_dictionary = nullptr; - if (!dictionary->HasKey("iceServers") && - dictionary->GetDictionary("data", &data_dictionary)) { - return Parse(*data_dictionary); + if (!json->FindKey("iceServers")) { + base::Value* data_dictionary = json->FindDictKey("data"); + if (data_dictionary) + return Parse(base::Value::AsDictionaryValue(*data_dictionary)); } - return Parse(*dictionary); + return Parse(base::Value::AsDictionaryValue(*json)); } // static