Skip to content

Commit

Permalink
[Code Health] Refactor ListValue::Insert in gpu compositor
Browse files Browse the repository at this point in the history
Also refactors a number of uses of base::ListValue and
base::DictionaryValue.

Bug: 1187105
Change-Id: I4838d8d1da732dd41766fcd067dd356d3350c665
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2869869
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jonathan Backer <backer@chromium.org>
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880594}
  • Loading branch information
a-sully authored and Chromium LUCI CQ committed May 7, 2021
1 parent bc241bd commit bf2dfe3
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 214 deletions.
3 changes: 2 additions & 1 deletion content/browser/devtools/protocol/system_info_handler.cc
Expand Up @@ -230,7 +230,8 @@ void SendGetInfoResponse(std::unique_ptr<GetInfoCallback> callback) {
enumerator.EndAuxAttributes();

std::unique_ptr<base::DictionaryValue> base_feature_status =
GetFeatureStatus();
base::DictionaryValue::From(
std::make_unique<base::Value>(GetFeatureStatus()));
std::unique_ptr<protocol::DictionaryValue> feature_status =
protocol::DictionaryValue::cast(
protocol::toProtocolValue(base_feature_status.get(), 1000));
Expand Down
60 changes: 30 additions & 30 deletions content/browser/gpu/compositor_util.cc
Expand Up @@ -19,6 +19,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/system/sys_info.h"
#include "base/values.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "cc/base/switches.h"
Expand Down Expand Up @@ -195,8 +196,7 @@ const GpuFeatureData GetGpuFeatureData(
return kGpuFeatureData[index];
}

std::unique_ptr<base::DictionaryValue> GetFeatureStatusImpl(
GpuFeatureInfoType type) {
base::Value GetFeatureStatusImpl(GpuFeatureInfoType type) {
GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
std::string gpu_access_blocked_reason;
bool gpu_access_blocked;
Expand All @@ -214,7 +214,7 @@ std::unique_ptr<base::DictionaryValue> GetFeatureStatusImpl(
manager->IsGpuCompositingDisabledForHardwareGpu();
}

auto feature_status_dict = std::make_unique<base::DictionaryValue>();
auto feature_status_dict = base::Value(base::Value::Type::DICTIONARY);

bool eof = false;
for (size_t i = 0; !eof; ++i) {
Expand Down Expand Up @@ -262,12 +262,12 @@ std::unique_ptr<base::DictionaryValue> GetFeatureStatusImpl(
status += "_on";
}
}
feature_status_dict->SetString(gpu_feature_data.name, status);
feature_status_dict.SetStringKey(gpu_feature_data.name, status);
}
return feature_status_dict;
}

std::unique_ptr<base::ListValue> GetProblemsImpl(GpuFeatureInfoType type) {
base::Value GetProblemsImpl(GpuFeatureInfoType type) {
GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
std::string gpu_access_blocked_reason;
bool gpu_access_blocked;
Expand All @@ -285,29 +285,29 @@ std::unique_ptr<base::ListValue> GetProblemsImpl(GpuFeatureInfoType type) {
manager->IsGpuCompositingDisabledForHardwareGpu();
}

auto problem_list = std::make_unique<base::ListValue>();
auto problem_list = base::Value(base::Value::Type::LIST);
if (!gpu_feature_info.applied_gpu_blocklist_entries.empty()) {
std::unique_ptr<gpu::GpuBlocklist> blocklist(gpu::GpuBlocklist::Create());
blocklist->GetReasons(problem_list.get(), "disabledFeatures",
blocklist->GetReasons(problem_list, "disabledFeatures",
gpu_feature_info.applied_gpu_blocklist_entries);
}
if (!gpu_feature_info.applied_gpu_driver_bug_list_entries.empty()) {
std::unique_ptr<gpu::GpuDriverBugList> bug_list(
gpu::GpuDriverBugList::Create());
bug_list->GetReasons(problem_list.get(), "workarounds",
bug_list->GetReasons(problem_list, "workarounds",
gpu_feature_info.applied_gpu_driver_bug_list_entries);
}

if (gpu_access_blocked) {
auto problem = std::make_unique<base::DictionaryValue>();
problem->SetString("description", "GPU process was unable to boot: " +
gpu_access_blocked_reason);
problem->Set("crBugs", std::make_unique<base::ListValue>());
auto disabled_features = std::make_unique<base::ListValue>();
disabled_features->AppendString("all");
problem->Set("affectedGpuSettings", std::move(disabled_features));
problem->SetString("tag", "disabledFeatures");
problem_list->Insert(0, std::move(problem));
auto problem = base::Value(base::Value::Type::DICTIONARY);
problem.SetStringKey("description", "GPU process was unable to boot: " +
gpu_access_blocked_reason);
problem.SetKey("crBugs", base::Value(base::Value::Type::LIST));
auto disabled_features = base::Value(base::Value::Type::LIST);
disabled_features.Append("all");
problem.SetKey("affectedGpuSettings", std::move(disabled_features));
problem.SetStringKey("tag", "disabledFeatures");
problem_list.Insert(problem_list.GetList().begin(), std::move(problem));
}

bool eof = false;
Expand All @@ -316,15 +316,15 @@ std::unique_ptr<base::ListValue> GetProblemsImpl(GpuFeatureInfoType type) {
gpu_feature_info, i, is_gpu_compositing_disabled, &eof);
if (gpu_feature_data.disabled &&
gpu_feature_data.disabled_info.is_problem) {
auto problem = std::make_unique<base::DictionaryValue>();
problem->SetString("description",
gpu_feature_data.disabled_info.description);
problem->Set("crBugs", std::make_unique<base::ListValue>());
auto disabled_features = std::make_unique<base::ListValue>();
disabled_features->AppendString(gpu_feature_data.name);
problem->Set("affectedGpuSettings", std::move(disabled_features));
problem->SetString("tag", "disabledFeatures");
problem_list->Append(std::move(problem));
auto problem = base::Value(base::Value::Type::DICTIONARY);
problem.SetStringKey("description",
gpu_feature_data.disabled_info.description);
problem.SetKey("crBugs", base::Value(base::Value::Type::LIST));
auto disabled_features = base::Value(base::Value::Type::LIST);
disabled_features.Append(gpu_feature_data.name);
problem.SetKey("affectedGpuSettings", std::move(disabled_features));
problem.SetStringKey("tag", "disabledFeatures");
problem_list.Insert(problem_list.GetList().begin(), std::move(problem));
}
}
return problem_list;
Expand Down Expand Up @@ -473,23 +473,23 @@ bool IsMainFrameBeforeActivationEnabled() {
return true;
}

std::unique_ptr<base::DictionaryValue> GetFeatureStatus() {
base::Value GetFeatureStatus() {
return GetFeatureStatusImpl(GpuFeatureInfoType::kCurrent);
}

std::unique_ptr<base::ListValue> GetProblems() {
base::Value GetProblems() {
return GetProblemsImpl(GpuFeatureInfoType::kCurrent);
}

std::vector<std::string> GetDriverBugWorkarounds() {
return GetDriverBugWorkaroundsImpl(GpuFeatureInfoType::kCurrent);
}

std::unique_ptr<base::DictionaryValue> GetFeatureStatusForHardwareGpu() {
base::Value GetFeatureStatusForHardwareGpu() {
return GetFeatureStatusImpl(GpuFeatureInfoType::kForHardwareGpu);
}

std::unique_ptr<base::ListValue> GetProblemsForHardwareGpu() {
base::Value GetProblemsForHardwareGpu() {
return GetProblemsImpl(GpuFeatureInfoType::kForHardwareGpu);
}

Expand Down
9 changes: 4 additions & 5 deletions content/browser/gpu/compositor_util.h
Expand Up @@ -37,13 +37,12 @@ CONTENT_EXPORT int NumberOfRendererRasterThreads();
// Returns true if main thread can be pipelined with activation.
CONTENT_EXPORT bool IsMainFrameBeforeActivationEnabled();

CONTENT_EXPORT std::unique_ptr<base::DictionaryValue> GetFeatureStatus();
CONTENT_EXPORT std::unique_ptr<base::ListValue> GetProblems();
CONTENT_EXPORT base::Value GetFeatureStatus();
CONTENT_EXPORT base::Value GetProblems();
CONTENT_EXPORT std::vector<std::string> GetDriverBugWorkarounds();

CONTENT_EXPORT std::unique_ptr<base::DictionaryValue>
GetFeatureStatusForHardwareGpu();
CONTENT_EXPORT std::unique_ptr<base::ListValue> GetProblemsForHardwareGpu();
CONTENT_EXPORT base::Value GetFeatureStatusForHardwareGpu();
CONTENT_EXPORT base::Value GetProblemsForHardwareGpu();
CONTENT_EXPORT std::vector<std::string> GetDriverBugWorkaroundsForHardwareGpu();

} // namespace content
Expand Down

0 comments on commit bf2dfe3

Please sign in to comment.