Skip to content

Commit

Permalink
[CodeHealth] Replace DictionaryValue::GetString in /chrome/browser/ex…
Browse files Browse the repository at this point in the history
…tensions/api/messaging.

This CL was uploaded by git cl split.

R=rdevlin.cronin@chromium.org

Bug: 1187036
Change-Id: I43afb06fbbd780643f04f798565251765939f36b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3828703
Commit-Queue: Brian Begnoche <bcb@chromium.org>
Auto-Submit: Brian Begnoche <bcb@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035073}
  • Loading branch information
brianbegnoche authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent 5ed4956 commit 4d533bb
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -288,25 +288,36 @@ TEST_F(NativeMessagingTest, EchoConnect) {
run_loop_->Run();
ASSERT_FALSE(last_message_.empty());
ASSERT_TRUE(last_message_parsed_);
ASSERT_TRUE(last_message_parsed_->is_dict());

std::string expected_url = std::string("chrome-extension://") +
ScopedTestNativeMessagingHost::kExtensionId + "/";
EXPECT_EQ(1, last_message_parsed_->FindIntKey("id"));
std::string text;
EXPECT_TRUE(last_message_parsed_->GetString("echo.text", &text));
EXPECT_EQ("Hello.", text);
std::string url;
EXPECT_TRUE(last_message_parsed_->GetString("caller_url", &url));
EXPECT_EQ(expected_url, url);
ScopedTestNativeMessagingHost::kExtensionId + "/";

{
const base::Value::Dict& dict = last_message_parsed_->GetDict();
EXPECT_EQ(1, last_message_parsed_->FindIntKey("id"));
const std::string* text = dict.FindStringByDottedPath("echo.text");
ASSERT_TRUE(text);
EXPECT_EQ("Hello.", *text);
const std::string* url = dict.FindString("caller_url");
EXPECT_TRUE(url);
EXPECT_EQ(expected_url, *url);
}

native_message_host_->OnMessage("{\"foo\": \"bar\"}");
run_loop_ = std::make_unique<base::RunLoop>();
run_loop_->Run();
EXPECT_EQ(2, last_message_parsed_->FindIntKey("id"));
EXPECT_TRUE(last_message_parsed_->GetString("echo.foo", &text));
EXPECT_EQ("bar", text);
EXPECT_TRUE(last_message_parsed_->GetString("caller_url", &url));
EXPECT_EQ(expected_url, url);

{
const base::Value::Dict& dict = last_message_parsed_->GetDict();
EXPECT_EQ(2, last_message_parsed_->FindIntKey("id"));
const std::string* text = dict.FindStringByDottedPath("echo.foo");
ASSERT_TRUE(text);
EXPECT_EQ("bar", *text);
const std::string* url = dict.FindString("caller_url");
ASSERT_TRUE(url);
EXPECT_EQ(expected_url, *url);
}

const base::Value* args = nullptr;
ASSERT_TRUE(last_message_parsed_->Get("args", &args));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/check.h"
#include "base/json/json_file_value_serializer.h"
#include "base/strings/string_util.h"
#include "base/values.h"
#include "chrome/common/chrome_features.h"

namespace extensions {
Expand Down Expand Up @@ -54,15 +53,15 @@ std::unique_ptr<NativeMessagingHostManifest> NativeMessagingHostManifest::Load(
return nullptr;
}

base::DictionaryValue* dictionary;
if (!parsed->GetAsDictionary(&dictionary)) {
if (!parsed->is_dict()) {
*error_message = "Invalid manifest file.";
return nullptr;
}
const base::Value::Dict& dict = parsed->GetDict();

std::unique_ptr<NativeMessagingHostManifest> result(
new NativeMessagingHostManifest());
if (!result->Parse(dictionary, error_message)) {
if (!result->Parse(dict, error_message)) {
return nullptr;
}

Expand All @@ -72,45 +71,46 @@ std::unique_ptr<NativeMessagingHostManifest> NativeMessagingHostManifest::Load(
NativeMessagingHostManifest::NativeMessagingHostManifest() {
}

bool NativeMessagingHostManifest::Parse(base::DictionaryValue* dictionary,
bool NativeMessagingHostManifest::Parse(const base::Value::Dict& dict,
std::string* error_message) {
if (!dictionary->GetString("name", &name_) ||
!IsValidName(name_)) {
const std::string* name_str = dict.FindString("name");
if (!name_str || !IsValidName(*name_str)) {
*error_message = "Invalid value for name.";
return false;
}
name_ = *name_str;

if (!dictionary->GetString("description", &description_) ||
description_.empty()) {
const std::string* desc_str = dict.FindString("description");
if (!desc_str || desc_str->empty()) {
*error_message = "Invalid value for description.";
return false;
}
description_ = *desc_str;

std::string type;
const std::string* type = dict.FindString("type");
// stdio is the only host type that's currently supported.
if (!dictionary->GetString("type", &type) ||
type != "stdio") {
if (!type || *type != "stdio") {
*error_message = "Invalid value for type.";
return false;
}
interface_ = HOST_INTERFACE_STDIO;

std::string path;
const std::string* path = dict.FindString("path");
// JSON parsed checks that all strings are valid UTF8.
if (!dictionary->GetString("path", &path) ||
(path_ = base::FilePath::FromUTF8Unsafe(path)).empty()) {
if (!path || (path_ = base::FilePath::FromUTF8Unsafe(*path)).empty()) {
*error_message = "Invalid value for path.";
return false;
}

const base::ListValue* allowed_origins_list;
if (!dictionary->GetList("allowed_origins", &allowed_origins_list)) {
const base::Value::List* allowed_origins_list =
dict.FindList("allowed_origins");
if (!allowed_origins_list) {
*error_message =
"Invalid value for allowed_origins. Expected a list of strings.";
return false;
}
allowed_origins_.ClearPatterns();
for (const auto& entry : allowed_origins_list->GetListDeprecated()) {
for (const auto& entry : *allowed_origins_list) {
if (!entry.is_string()) {
*error_message = "allowed_origins must be list of strings.";
return false;
Expand All @@ -136,7 +136,7 @@ bool NativeMessagingHostManifest::Parse(base::DictionaryValue* dictionary,

if (base::FeatureList::IsEnabled(features::kOnConnectNative)) {
if (const base::Value* supports_native_initiated_connections =
dictionary->FindKey("supports_native_initiated_connections")) {
dict.Find("supports_native_initiated_connections")) {
if (!supports_native_initiated_connections->is_bool()) {
*error_message =
"supports_native_initiated_connections must be a boolean.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@
#include <string>

#include "base/files/file_path.h"
#include "base/values.h"
#include "extensions/common/url_pattern_set.h"

namespace base {
class DictionaryValue;
}

namespace extensions {

class NativeMessagingHostManifest {
Expand Down Expand Up @@ -52,7 +49,7 @@ class NativeMessagingHostManifest {

// Parses manifest |dictionary|. In case of an error sets |error_message| and
// returns false.
bool Parse(base::DictionaryValue* dictionary, std::string* error_message);
bool Parse(const base::Value::Dict& dict, std::string* error_message);

std::string name_;
std::string description_;
Expand Down

0 comments on commit 4d533bb

Please sign in to comment.