Skip to content

Commit

Permalink
[CodeHealth] Settings WebUI base::Value refactoring - Part 2
Browse files Browse the repository at this point in the history 
This CL corrects the use of base::Value for the webui handlers of
most files under webui/settings/chromeos.

Bug: 1187001
Change-Id: I58e154738210b11b4f43dca098fdd9a296cb2575
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3810454
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: Claudio DeSouza <cdesouza@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1031661}
  • Loading branch information
@cdesouza-chromium
cdesouza-chromium authored and Chromium LUCI CQ committed Aug 4, 2022
1 parent f9fc5a1 commit 8172da8
Show file tree
Hide file tree
Showing 21 changed files with 244 additions and 283 deletions.
18 changes: 9 additions & 9 deletions chrome/browser/ui/webui/settings/chromeos/accessibility_handler.cc
View file
Expand Up @@ -242,23 +242,23 @@ void AccessibilityHandler::MaybeAddDictationLocales() {
ui_languages.insert(language::SplitIntoMainAndTail(enabled_language).first);
}

base::Value locales_list(base::Value::Type::LIST);
base::Value::List locales_list;
for (auto& locale : locales) {
base::Value option(base::Value::Type::DICTIONARY);
option.SetKey("value", base::Value(locale.first));
option.SetKey("name",
base::Value(l10n_util::GetDisplayNameForLocale(
locale.first, application_locale, /*is_for_ui=*/true)));
option.SetKey("worksOffline", base::Value(locale.second.works_offline));
option.SetKey("installed", base::Value(locale.second.installed));
base::Value::Dict option;
option.Set("value", locale.first);
option.Set("name",
l10n_util::GetDisplayNameForLocale(
locale.first, application_locale, /*is_for_ui=*/true));
option.Set("worksOffline", locale.second.works_offline);
option.Set("installed", locale.second.installed);

// We can recommend languages that match the current application
// locale, IME languages or enabled preferred languages.
std::pair<base::StringPiece, base::StringPiece> lang_and_locale =
language::SplitIntoMainAndTail(locale.first);
bool is_recommended = base::Contains(ui_languages, lang_and_locale.first);

option.SetKey("recommended", base::Value(is_recommended));
option.Set("recommended", is_recommended);
locales_list.Append(std::move(option));
}

Expand Down
74 changes: 36 additions & 38 deletions chrome/browser/ui/webui/settings/chromeos/ambient_mode_handler.cc
View file
Expand Up @@ -294,86 +294,84 @@ void AmbientModeHandler::SendTemperatureUnit() {

void AmbientModeHandler::SendTopicSource() {
DCHECK(settings_);
base::Value topic_source(base::Value::Type::DICTIONARY);
topic_source.SetKey("hasGooglePhotosAlbums",
base::Value(!personal_albums_.albums.empty()));
topic_source.SetKey("topicSource",
base::Value(static_cast<int>(settings_->topic_source)));
FireWebUIListener("topic-source-changed",
base::Value(std::move(topic_source)));
base::Value::Dict topic_source;
topic_source.Set("hasGooglePhotosAlbums",
base::Value(!personal_albums_.albums.empty()));
topic_source.Set("topicSource",
base::Value(static_cast<int>(settings_->topic_source)));
FireWebUIListener("topic-source-changed", topic_source);
}

void AmbientModeHandler::SendAlbums(ash::AmbientModeTopicSource topic_source) {
DCHECK(settings_);

base::Value dictionary(base::Value::Type::DICTIONARY);
base::Value albums(base::Value::Type::LIST);
base::Value::Dict dictionary;
base::Value::List albums;
switch (topic_source) {
case ash::AmbientModeTopicSource::kGooglePhotos:
for (const auto& album : personal_albums_.albums) {
base::Value value(base::Value::Type::DICTIONARY);
value.SetKey("albumId", base::Value(album.album_id));
value.SetKey("checked", base::Value(album.selected));
value.SetKey("description", base::Value(GetAlbumDescription(album)));
value.SetKey("title", base::Value(album.album_name));
base::Value::Dict value;
value.Set("albumId", album.album_id);
value.Set("checked", album.selected);
value.Set("description", GetAlbumDescription(album));
value.Set("title", album.album_name);
albums.Append(std::move(value));
}
break;
case ash::AmbientModeTopicSource::kArtGallery:
for (const auto& setting : settings_->art_settings) {
if (!setting.visible)
continue;
base::Value value(base::Value::Type::DICTIONARY);
value.SetKey("albumId", base::Value(setting.album_id));
value.SetKey("checked", base::Value(setting.enabled));
value.SetKey("description", base::Value(setting.description));
value.SetKey("title", base::Value(setting.title));
base::Value::Dict value;
value.Set("albumId", setting.album_id);
value.Set("checked", setting.enabled);
value.Set("description", setting.description);
value.Set("title", setting.title);
albums.Append(std::move(value));
}
break;
}

dictionary.SetKey("topicSource", base::Value(static_cast<int>(topic_source)));
dictionary.SetKey("selectedTopicSource",
base::Value(static_cast<int>(settings_->topic_source)));
dictionary.SetKey("albums", std::move(albums));
FireWebUIListener("albums-changed", std::move(dictionary));
dictionary.Set("topicSource", static_cast<int>(topic_source));
dictionary.Set("selectedTopicSource",
static_cast<int>(settings_->topic_source));
dictionary.Set("albums", std::move(albums));
FireWebUIListener("albums-changed", dictionary);
}

void AmbientModeHandler::SendAlbumPreview(
ash::AmbientModeTopicSource topic_source,
const std::string& album_id,
std::string&& png_data_url) {
base::Value album(base::Value::Type::DICTIONARY);
album.SetKey("albumId", base::Value(album_id));
album.SetKey("topicSource", base::Value(static_cast<int>(topic_source)));
album.SetKey("url", base::Value(png_data_url));
FireWebUIListener("album-preview-changed", std::move(album));
base::Value::Dict album;
album.Set("albumId", album_id);
album.Set("topicSource", static_cast<int>(topic_source));
album.Set("url", png_data_url);
FireWebUIListener("album-preview-changed", album);
}

void AmbientModeHandler::SendRecentHighlightsPreviews() {
if (!FindPersonalAlbumById(ash::kAmbientModeRecentHighlightsAlbumId))
return;

base::Value png_data_urls(base::Value::Type::LIST);
base::Value::List png_data_urls;
for (const auto& image : recent_highlights_preview_images_) {
if (image.isNull())
continue;

std::vector<unsigned char> encoded_image_bytes;
EncodeImage(image, &encoded_image_bytes);
if (!encoded_image_bytes.empty()) {
png_data_urls.Append(base::Value(webui::GetPngDataUrl(
&encoded_image_bytes.front(), encoded_image_bytes.size())));
png_data_urls.Append(webui::GetPngDataUrl(&encoded_image_bytes.front(),
encoded_image_bytes.size()));
}
}

base::Value album(base::Value::Type::DICTIONARY);
album.SetKey("albumId",
base::Value(ash::kAmbientModeRecentHighlightsAlbumId));
album.SetKey("topicSource", base::Value(static_cast<int>(
ash::AmbientModeTopicSource::kGooglePhotos)));
album.SetKey("recentHighlightsUrls", std::move(png_data_urls));
base::Value::Dict album;
album.Set("albumId", ash::kAmbientModeRecentHighlightsAlbumId);
album.Set("topicSource",
static_cast<int>(ash::AmbientModeTopicSource::kGooglePhotos));
album.Set("recentHighlightsUrls", std::move(png_data_urls));
FireWebUIListener("album-preview-changed", std::move(album));
}

Expand Down
17 changes: 6 additions & 11 deletions chrome/browser/ui/webui/settings/chromeos/android_apps_handler.cc
View file
Expand Up @@ -5,7 +5,6 @@
#include "chrome/browser/ui/webui/settings/chromeos/android_apps_handler.h"

#include "base/bind.h"
#include "base/values.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/apps/app_service/launch_utils.h"
Expand Down Expand Up @@ -81,17 +80,14 @@ void AndroidAppsHandler::OnArcPlayStoreEnabledChanged(bool enabled) {
SendAndroidAppsInfo();
}

std::unique_ptr<base::DictionaryValue>
AndroidAppsHandler::BuildAndroidAppsInfo() {
std::unique_ptr<base::DictionaryValue> info(new base::DictionaryValue);
info->SetBoolKey("playStoreEnabled",
arc::IsArcPlayStoreEnabledForProfile(profile_));
base::Value::Dict AndroidAppsHandler::BuildAndroidAppsInfo() {
base::Value::Dict info;
info.Set("playStoreEnabled", arc::IsArcPlayStoreEnabledForProfile(profile_));
const ArcAppListPrefs* arc_apps_pref = ArcAppListPrefs::Get(profile_);
// TODO(khmel): Inverstigate why in some browser tests
// playStoreEnabled is true but arc_apps_pref is not set.
info->SetBoolKey(
"settingsAppAvailable",
arc_apps_pref && arc_apps_pref->IsRegistered(arc::kSettingsAppId));
info.Set("settingsAppAvailable",
arc_apps_pref && arc_apps_pref->IsRegistered(arc::kSettingsAppId));
return info;
}

Expand All @@ -102,8 +98,7 @@ void AndroidAppsHandler::HandleRequestAndroidAppsInfo(
}

void AndroidAppsHandler::SendAndroidAppsInfo() {
std::unique_ptr<base::DictionaryValue> info = BuildAndroidAppsInfo();
FireWebUIListener("android-apps-info-update", *info);
FireWebUIListener("android-apps-info-update", BuildAndroidAppsInfo());
}

void AndroidAppsHandler::ShowAndroidAppsSettings(
Expand Down
7 changes: 2 additions & 5 deletions chrome/browser/ui/webui/settings/chromeos/android_apps_handler.h
View file
Expand Up @@ -10,6 +10,7 @@

#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "base/values.h"
#include "chrome/browser/apps/app_service/app_service_proxy_forward.h"
#include "chrome/browser/ash/arc/session/arc_session_manager.h"
#include "chrome/browser/ash/arc/session/arc_session_manager_observer.h"
Expand All @@ -18,10 +19,6 @@

class Profile;

namespace base {
class DictionaryValue;
}

namespace chromeos {
namespace settings {

Expand Down Expand Up @@ -53,7 +50,7 @@ class AndroidAppsHandler : public ::settings::SettingsPageUIHandler,
void OnArcPlayStoreEnabledChanged(bool enabled) override;

private:
std::unique_ptr<base::DictionaryValue> BuildAndroidAppsInfo();
base::Value::Dict BuildAndroidAppsInfo();
void HandleRequestAndroidAppsInfo(const base::Value::List& args);
void HandleAppChanged(const std::string& app_id);
void SendAndroidAppsInfo();
Expand Down
24 changes: 10 additions & 14 deletions chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler.cc
View file
Expand Up @@ -140,24 +140,20 @@ void KeyboardHandler::UpdateShowKeys() {
!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kHasChromeOSKeyboard);

base::Value keyboard_params(base::Value::Type::DICTIONARY);
keyboard_params.SetKey("showCapsLock", base::Value(has_caps_lock));
keyboard_params.SetKey(
"showExternalMetaKey",
base::Value(keyboards_state.has_external_generic_keyboard));
keyboard_params.SetKey(
"showAppleCommandKey",
base::Value(keyboards_state.has_external_apple_keyboard));
base::Value::Dict keyboard_params;
keyboard_params.Set("showCapsLock", has_caps_lock);
keyboard_params.Set("showExternalMetaKey",
keyboards_state.has_external_generic_keyboard);
keyboard_params.Set("showAppleCommandKey",
keyboards_state.has_external_apple_keyboard);
// An external (USB/BT) ChromeOS keyboard is treated similarly to an internal
// ChromeOS keyboard. i.e. they are functionally the same.
keyboard_params.SetKey(
"hasLauncherKey",
base::Value(keyboards_state.has_launcher_key ||
keyboards_state.has_external_chromeos_keyboard));
keyboard_params.Set("hasLauncherKey",
keyboards_state.has_launcher_key ||
keyboards_state.has_external_chromeos_keyboard);

const bool show_assistant_key_settings = ui::DeviceKeyboardHasAssistantKey();
keyboard_params.SetKey("hasAssistantKey",
base::Value(show_assistant_key_settings));
keyboard_params.Set("hasAssistantKey", show_assistant_key_settings);

FireWebUIListener(kShowKeysChangedName, keyboard_params);
}
Expand Down
14 changes: 7 additions & 7 deletions chrome/browser/ui/webui/settings/chromeos/device_name_handler.cc
View file
Expand Up @@ -51,13 +51,13 @@ void DeviceNameHandler::RegisterMessages() {
base::Unretained(this)));
}

base::Value DeviceNameHandler::GetDeviceNameMetadata() const {
base::Value metadata(base::Value::Type::DICTIONARY);
base::Value::Dict DeviceNameHandler::GetDeviceNameMetadata() const {
base::Value::Dict metadata;
DeviceNameStore::DeviceNameMetadata device_name_metadata =
device_name_store_->GetDeviceNameMetadata();
metadata.SetStringKey(kMetadataFirstKey, device_name_metadata.device_name);
metadata.SetIntKey(kMetadataSecondKey,
static_cast<int>(device_name_metadata.device_name_state));
metadata.Set(kMetadataFirstKey, device_name_metadata.device_name);
metadata.Set(kMetadataSecondKey,
static_cast<int>(device_name_metadata.device_name_state));
return metadata;
}

Expand All @@ -79,12 +79,12 @@ void DeviceNameHandler::HandleNotifyReadyForDeviceName(
const base::Value::List& args) {
AllowJavascript();
FireWebUIListener("settings.updateDeviceNameMetadata",
base::Value(GetDeviceNameMetadata()));
GetDeviceNameMetadata());
}

void DeviceNameHandler::OnDeviceNameMetadataChanged() {
FireWebUIListener("settings.updateDeviceNameMetadata",
base::Value(GetDeviceNameMetadata()));
GetDeviceNameMetadata());
}

} // namespace settings
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/settings/chromeos/device_name_handler.h
View file
Expand Up @@ -41,7 +41,7 @@ class DeviceNameHandler : public ::settings::SettingsPageUIHandler,

explicit DeviceNameHandler(DeviceNameStore* device_name_store);

base::Value GetDeviceNameMetadata() const;
base::Value::Dict GetDeviceNameMetadata() const;

DeviceNameStore* device_name_store_;

Expand Down

0 comments on commit 8172da8

Please sign in to comment.