Skip to content

Commit

Permalink
[CodeHealth] Remove calls to Value::GetAsDictionary (c/b/ui/webui).
Browse files Browse the repository at this point in the history
Bug: 1187011
Change-Id: I19c1e743d1de101a8b77c1caca59e3bd1edc57a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3935272
Commit-Queue: John Lee <johntlee@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1056025}
  • Loading branch information
anforowicz authored and Chromium LUCI CQ committed Oct 6, 2022
1 parent 2707fe5 commit 80d0f4e
Show file tree
Hide file tree
Showing 3 changed files with 208 additions and 284 deletions.
51 changes: 25 additions & 26 deletions chrome/browser/ui/webui/chromeos/sync/os_sync_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "testing/gmock/include/gmock/gmock-matchers.h"
#include "testing/gtest/include/gtest/gtest.h"

using base::DictionaryValue;
using content::TestWebUI;
using syncer::SyncService;
using syncer::UserSelectableOsType;
Expand All @@ -43,34 +42,34 @@ enum SyncAllConfig { SYNC_ALL_OS_TYPES, CHOOSE_WHAT_TO_SYNC };

// Creates a dictionary with the key/value pairs appropriate for a call to
// HandleSetOsSyncDatatypes().
DictionaryValue CreateOsSyncPrefs(SyncAllConfig sync_all,
UserSelectableOsTypeSet types,
bool wallpaper_enabled) {
DictionaryValue result;
result.SetBoolKey("syncAllOsTypes", sync_all == SYNC_ALL_OS_TYPES);
base::Value::Dict CreateOsSyncPrefs(SyncAllConfig sync_all,
UserSelectableOsTypeSet types,
bool wallpaper_enabled) {
base::Value::Dict result;
result.Set("syncAllOsTypes", sync_all == SYNC_ALL_OS_TYPES);
// Add all of our data types.
result.SetBoolKey("osAppsSynced", types.Has(UserSelectableOsType::kOsApps));
result.SetBoolKey("osPreferencesSynced",
types.Has(UserSelectableOsType::kOsPreferences));
result.SetBoolKey("osWifiConfigurationsSynced",
types.Has(UserSelectableOsType::kOsWifiConfigurations));
result.SetBoolKey("wallpaperEnabled",
sync_all == SYNC_ALL_OS_TYPES || wallpaper_enabled);
result.Set("osAppsSynced", types.Has(UserSelectableOsType::kOsApps));
result.Set("osPreferencesSynced",
types.Has(UserSelectableOsType::kOsPreferences));
result.Set("osWifiConfigurationsSynced",
types.Has(UserSelectableOsType::kOsWifiConfigurations));
result.Set("wallpaperEnabled",
sync_all == SYNC_ALL_OS_TYPES || wallpaper_enabled);
return result;
}

// Checks whether the passed |dictionary| contains a |key| with the given
// |expected_value|.
void CheckBool(const DictionaryValue* dictionary,
void CheckBool(const base::Value::Dict& dictionary,
const std::string& key,
bool expected_value) {
EXPECT_THAT(dictionary->FindBoolPath(key), Optional(expected_value))
EXPECT_THAT(dictionary.FindBool(key), Optional(expected_value))
<< "Key: " << key;
}

// Checks to make sure that the values stored in |dictionary| match the values
// expected by the JS layer.
void CheckConfigDataTypeArguments(const DictionaryValue* dictionary,
void CheckConfigDataTypeArguments(const base::Value::Dict& dictionary,
SyncAllConfig config,
UserSelectableOsTypeSet types,
bool wallpaper_enabled) {
Expand Down Expand Up @@ -136,15 +135,19 @@ class OsSyncHandlerTest : public ChromeRenderViewHostTestHarness {

// Expects that an "os-sync-prefs-changed" event was sent to the WebUI and
// returns the data passed to that event.
void ExpectOsSyncPrefsSent(const DictionaryValue** os_sync_prefs) {
base::Value::Dict ExpectOsSyncPrefsSent() {
const TestWebUI::CallData& call_data = *web_ui_.call_data().back();
EXPECT_EQ("cr.webUIListenerCallback", call_data.function_name());

EXPECT_TRUE(call_data.arg1());
const std::string* event = call_data.arg1()->GetIfString();
EXPECT_TRUE(event);
EXPECT_EQ(*event, "os-sync-prefs-changed");

EXPECT_TRUE(call_data.arg2()->GetAsDictionary(os_sync_prefs));
EXPECT_TRUE(call_data.arg2());
const base::Value::Dict* dict = call_data.arg2()->GetIfDict();
EXPECT_TRUE(dict);
return dict->Clone();
}

void NotifySyncStateChanged() { handler_->OnStateChanged(sync_service_); }
Expand Down Expand Up @@ -260,8 +263,7 @@ TEST_F(OsSyncHandlerTest, ShowSetupSyncEverything) {
SetWallperEnabledPref(true);
handler_->HandleDidNavigateToOsSyncPage(base::Value::List());

const DictionaryValue* dictionary;
ExpectOsSyncPrefsSent(&dictionary);
base::Value::Dict dictionary = ExpectOsSyncPrefsSent();
CheckBool(dictionary, "syncAllOsTypes", true);
CheckBool(dictionary, "osAppsRegistered", true);
CheckBool(dictionary, "osPreferencesRegistered", true);
Expand All @@ -277,8 +279,7 @@ TEST_F(OsSyncHandlerTest, ShowSetupManuallySyncAll) {
SetWallperEnabledPref(true);
handler_->HandleDidNavigateToOsSyncPage(base::Value::List());

const DictionaryValue* dictionary;
ExpectOsSyncPrefsSent(&dictionary);
base::Value::Dict dictionary = ExpectOsSyncPrefsSent();
CheckConfigDataTypeArguments(dictionary, CHOOSE_WHAT_TO_SYNC,
UserSelectableOsTypeSet::All(),
/*wallpaper_enabled=*/true);
Expand All @@ -290,8 +291,7 @@ TEST_F(OsSyncHandlerTest, ShowSetupSyncForAllTypesIndividually) {
user_settings_->SetSelectedOsTypes(/*sync_all_os_types=*/false, types);
handler_->HandleDidNavigateToOsSyncPage(base::Value::List());

const DictionaryValue* dictionary;
ExpectOsSyncPrefsSent(&dictionary);
base::Value::Dict dictionary = ExpectOsSyncPrefsSent();
CheckConfigDataTypeArguments(dictionary, CHOOSE_WHAT_TO_SYNC, types,
/*wallpaper_enabled=*/false);
}
Expand All @@ -300,8 +300,7 @@ TEST_F(OsSyncHandlerTest, ShowSetupSyncForAllTypesIndividually) {
user_settings_->SetSelectedOsTypes(/*sync_all_os_types=*/false, /*types=*/{});
SetWallperEnabledPref(true);
handler_->HandleDidNavigateToOsSyncPage(base::Value::List());
const DictionaryValue* dictionary;
ExpectOsSyncPrefsSent(&dictionary);
base::Value::Dict dictionary = ExpectOsSyncPrefsSent();
CheckConfigDataTypeArguments(dictionary, CHOOSE_WHAT_TO_SYNC, /*types=*/{},
/*wallpaper_enabled=*/true);
}
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/ui/webui/ntp/app_launcher_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,16 @@ class AppLauncherHandlerTest : public BrowserWithTestWindowTest {
app_launcher_handler->call_data()[0]->function_name());

const base::Value* arg1 = app_launcher_handler->call_data()[0]->arg1();
ASSERT_TRUE(arg1->is_dict());

const base::DictionaryValue* app_info;
arg1->GetAsDictionary(&app_info);
ASSERT_TRUE(arg1->is_dict());
const base::Value::Dict& app_info = arg1->GetDict();

const std::string* app_id = app_info->FindStringKey(kKeyAppId);
const std::string* app_id = app_info.FindString(kKeyAppId);
ASSERT_TRUE(app_id);
EXPECT_EQ(*app_id, installed_app_id);

EXPECT_THAT(app_info->FindBoolPath(kKeyIsLocallyInstalled), Optional(true));
EXPECT_THAT(app_info.FindBoolByDottedPath(kKeyIsLocallyInstalled),
Optional(true));
}

std::unique_ptr<content::TestWebUI> CreateTestWebUI(
Expand Down
Loading

0 comments on commit 80d0f4e

Please sign in to comment.