Skip to content

Commit

Permalink
Migrate remoting/client/notification/ to use newer base::Value APIs.
Browse files Browse the repository at this point in the history
Only Value::Dict and Value::List should be used to read/write
dictionary and list Values.

Bug: 1338341
Change-Id: I0d338e6a72162b3b568b12d40357cda50c8b325c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4060047
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1076311}
  • Loading branch information
Matt Menke authored and Chromium LUCI CQ committed Nov 28, 2022
1 parent 509fe79 commit 8a70c7b
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 109 deletions.
12 changes: 5 additions & 7 deletions remoting/client/notification/gstatic_json_fetcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/test/gmock_callback_support.h"
#include "base/test/mock_callback.h"
#include "base/test/task_environment.h"
#include "base/test/values_test_util.h"
#include "base/values.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_error_job.h"
Expand All @@ -36,20 +37,17 @@ MATCHER(NoJsonData, "") {
}

MATCHER(IsJsonData1, "") {
if (!arg || !arg->is_dict()) {
if (!arg) {
return false;
}
const base::Value* a = arg->FindKey("a");
return a && a->is_int() && a->GetInt() == 1;
return *arg == base::test::ParseJson(R"({"a":1})");
}

MATCHER(IsJsonData2, "") {
if (!arg || !arg->is_list()) {
if (!arg) {
return false;
}
auto list = arg->GetListDeprecated();
return list.size() == 1 && list[0].is_string() &&
list[0].GetString() == "123";
return *arg == base::test::ParseJson(R"(["123"])");
}

class TestURLRequestInterceptor : public net::URLRequestInterceptor {
Expand Down
32 changes: 19 additions & 13 deletions remoting/client/notification/notification_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
})");

template <typename T, typename IsTChecker, typename TGetter>
bool FindKeyAndGet(const base::Value& dict,
bool FindKeyAndGet(const base::Value::Dict& dict,
const std::string& key,
T* out,
IsTChecker is_t_checker,
TGetter t_getter,
const std::string& type_name) {
const base::Value* value = dict.FindKey(key);
const base::Value* value = dict.Find(key);
if (!value) {
LOG(WARNING) << "|" << key << "| not found in dictionary.";
return false;
Expand All @@ -95,7 +95,7 @@ bool FindKeyAndGet(const base::Value& dict,
return true;
}

bool FindKeyAndGet(const base::Value& dict,
bool FindKeyAndGet(const base::Value::Dict& dict,
const std::string& key,
std::string* out) {
const std::string& (base::Value::*get_string_fp)() const =
Expand All @@ -104,12 +104,16 @@ bool FindKeyAndGet(const base::Value& dict,
"string");
}

bool FindKeyAndGet(const base::Value& dict, const std::string& key, int* out) {
bool FindKeyAndGet(const base::Value::Dict& dict,
const std::string& key,
int* out) {
return FindKeyAndGet(dict, key, out, &base::Value::is_int,
&base::Value::GetInt, "int");
}

bool FindKeyAndGet(const base::Value& dict, const std::string& key, bool* out) {
bool FindKeyAndGet(const base::Value::Dict& dict,
const std::string& key,
bool* out) {
return FindKeyAndGet(dict, key, out, &base::Value::is_bool,
&base::Value::GetBool, "bool");
}
Expand Down Expand Up @@ -173,9 +177,10 @@ class MessageAndLinkTextResults
NotifyFetchFailed();
return;
}

const base::Value::Dict& dict = translations->GetDict();
bool is_translation_found = false;
is_translation_found =
FindKeyAndGet(*translations, locale_, string_to_update);
is_translation_found = FindKeyAndGet(dict, locale_, string_to_update);
if (!is_translation_found) {
LOG(WARNING) << "Failed to find translation for locale " << locale_
<< ". Looking for parent locales instead.";
Expand All @@ -185,7 +190,7 @@ class MessageAndLinkTextResults
// Locales returned by GetParentLocales() use "_" instead of "-", which
// need to be fixed.
std::replace(parent_locale.begin(), parent_locale.end(), '_', '-');
if (FindKeyAndGet(*translations, parent_locale, string_to_update)) {
if (FindKeyAndGet(dict, parent_locale, string_to_update)) {
LOG(WARNING) << "Locale " << parent_locale
<< " is being used instead.";
is_translation_found = true;
Expand All @@ -199,7 +204,7 @@ class MessageAndLinkTextResults
<< " is found in translations. Falling back to default locale: "
<< kDefaultLocale;
is_translation_found =
FindKeyAndGet(*translations, kDefaultLocale, string_to_update);
FindKeyAndGet(dict, kDefaultLocale, string_to_update);
}
if (!is_translation_found) {
LOG(ERROR) << "Failed to find translation for default locale";
Expand Down Expand Up @@ -288,11 +293,12 @@ void NotificationClient::OnRulesFetched(const std::string& user_email,
return;
}

for (const auto& rule : rules->GetListDeprecated()) {
for (const auto& rule : rules->GetList()) {
std::string message_text_filename;
std::string link_text_filename;
auto message = ParseAndMatchRule(rule, user_email, &message_text_filename,
&link_text_filename);
auto message =
ParseAndMatchRule(rule.GetDict(), user_email, &message_text_filename,
&link_text_filename);

if (message) {
FetchTranslatedTexts(message_text_filename, link_text_filename,
Expand All @@ -305,7 +311,7 @@ void NotificationClient::OnRulesFetched(const std::string& user_email,
}

absl::optional<NotificationMessage> NotificationClient::ParseAndMatchRule(
const base::Value& rule,
const base::Value::Dict& rule,
const std::string& user_email,
std::string* out_message_text_filename,
std::string* out_link_text_filename) {
Expand Down
7 changes: 2 additions & 5 deletions remoting/client/notification/notification_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@

#include "base/callback_forward.h"
#include "base/task/single_thread_task_runner.h"
#include "base/values.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace base {
class Value;
} // namespace base

namespace remoting {

class JsonFetcher;
Expand Down Expand Up @@ -67,7 +64,7 @@ class NotificationClient final {
// and the rule should apply to the user. |message_text| and |link_text| will
// not be set and caller needs to call FetchTranslatedText to fill them up.
absl::optional<NotificationMessage> ParseAndMatchRule(
const base::Value& rule,
const base::Value::Dict& rule,
const std::string& user_email,
std::string* out_message_text_filename,
std::string* out_link_text_filename);
Expand Down

0 comments on commit 8a70c7b

Please sign in to comment.