Skip to content

Commit

Permalink
CodeHealth: Remove DictionaryValue from chrome/test/chromedriver p1
Browse files Browse the repository at this point in the history
Test: existing tests
Bug: 1187061
Change-Id: I9150f9c74fb760e81efceefebb9dd5b9c661d384
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4028841
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Vladimir Nechaev <nechaev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073410}
  • Loading branch information
Sammie Quon authored and Chromium LUCI CQ committed Nov 18, 2022
1 parent 831888d commit 9ffd105
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 192 deletions.
9 changes: 4 additions & 5 deletions chrome/test/chromedriver/capabilities.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,13 @@ Status ParseFilePath(base::FilePath* to_set,
return Status(kOk);
}

Status ParseDict(std::unique_ptr<base::DictionaryValue>* to_set,
Status ParseDict(std::unique_ptr<base::Value::Dict>* to_set,
const base::Value& option,
Capabilities* capabilities) {
const base::DictionaryValue* dict = nullptr;
if (!option.GetAsDictionary(&dict))
const base::Value::Dict* dict = option.GetIfDict();
if (!dict)
return Status(kInvalidArgument, "must be a dictionary");
*to_set =
base::DictionaryValue::From(base::Value::ToUniquePtrValue(dict->Clone()));
*to_set = std::make_unique<base::Value::Dict>(dict->Clone());
return Status(kOk);
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/test/chromedriver/capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ struct Capabilities {
// Time to wait for extension background page to appear. If 0, no waiting.
base::TimeDelta extension_load_timeout;

std::unique_ptr<base::DictionaryValue> local_state;
std::unique_ptr<base::Value::Dict> local_state;

std::string log_path;

Expand All @@ -183,7 +183,7 @@ struct Capabilities {

base::Value devtools_events_logging_prefs;

std::unique_ptr<base::DictionaryValue> prefs;
std::unique_ptr<base::Value::Dict> prefs;

Switches switches;

Expand Down
4 changes: 2 additions & 2 deletions chrome/test/chromedriver/chrome/chrome.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class Chrome {

// Sets the rect of the specified WebView
virtual Status SetWindowRect(const std::string& target_id,
const base::DictionaryValue& params) = 0;
const base::Value::Dict& params) = 0;

// Maximizes specified WebView.
virtual Status MaximizeWindow(const std::string& target_id) = 0;
Expand All @@ -87,7 +87,7 @@ class Chrome {

// Requests altering permission setting for given permission.
virtual Status SetPermission(
std::unique_ptr<base::DictionaryValue> permission_descriptor,
std::unique_ptr<base::Value::Dict> permission_descriptor,
PermissionState desired_state,
bool one_realm,
WebView* current_view) = 0;
Expand Down
73 changes: 36 additions & 37 deletions chrome/test/chromedriver/chrome/chrome_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,8 @@ Status ChromeImpl::MaximizeWindow(const std::string& target_id) {
if (window.state == "maximized")
return Status(kOk);

auto bounds = std::make_unique<base::DictionaryValue>();
bounds->SetStringKey("windowState", "maximized");
auto bounds = std::make_unique<base::Value::Dict>();
bounds->Set("windowState", "maximized");
return SetWindowBounds(&window, target_id, std::move(bounds));
}

Expand All @@ -358,8 +358,8 @@ Status ChromeImpl::MinimizeWindow(const std::string& target_id) {
if (window.state == "minimized")
return Status(kOk);

auto bounds = std::make_unique<base::DictionaryValue>();
bounds->SetStringKey("windowState", "minimized");
auto bounds = std::make_unique<base::Value::Dict>();
bounds->Set("windowState", "minimized");
return SetWindowBounds(&window, target_id, std::move(bounds));
}

Expand All @@ -372,33 +372,33 @@ Status ChromeImpl::FullScreenWindow(const std::string& target_id) {
if (window.state == "fullscreen")
return Status(kOk);

auto bounds = std::make_unique<base::DictionaryValue>();
bounds->SetStringKey("windowState", "fullscreen");
auto bounds = std::make_unique<base::Value::Dict>();
bounds->Set("windowState", "fullscreen");
return SetWindowBounds(&window, target_id, std::move(bounds));
}

Status ChromeImpl::SetWindowRect(const std::string& target_id,
const base::DictionaryValue& params) {
const base::Value::Dict& params) {
Window window;
Status status = GetWindow(target_id, &window);
if (status.IsError())
return status;

auto bounds = std::make_unique<base::DictionaryValue>();
auto bounds = std::make_unique<base::Value::Dict>();

// window position
absl::optional<int> x = params.FindIntKey("x");
absl::optional<int> y = params.FindIntKey("y");
absl::optional<int> x = params.FindInt("x");
absl::optional<int> y = params.FindInt("y");
if (x.has_value() && y.has_value()) {
bounds->SetIntKey("left", *x);
bounds->SetIntKey("top", *y);
bounds->Set("left", *x);
bounds->Set("top", *y);
}
// window size
absl::optional<int> width = params.FindIntKey("width");
absl::optional<int> height = params.FindIntKey("height");
absl::optional<int> width = params.FindInt("width");
absl::optional<int> height = params.FindInt("height");
if (width.has_value() && height.has_value()) {
bounds->SetIntKey("width", *width);
bounds->SetIntKey("height", *height);
bounds->Set("width", *width);
bounds->Set("height", *height);
}

return SetWindowBounds(&window, target_id, std::move(bounds));
Expand All @@ -420,10 +420,9 @@ Status ChromeImpl::GetWindowBounds(int window_id, Window* window) {
return ParseWindowBounds(result, window);
}

Status ChromeImpl::SetWindowBounds(
Window* window,
const std::string& target_id,
std::unique_ptr<base::DictionaryValue> bounds) {
Status ChromeImpl::SetWindowBounds(Window* window,
const std::string& target_id,
std::unique_ptr<base::Value::Dict> bounds) {
Status status = devtools_websocket_client_->ConnectIfNecessary();
if (status.IsError())
return status;
Expand Down Expand Up @@ -452,7 +451,7 @@ Status ChromeImpl::SetWindowBounds(
return MakeFailedStatus(normal, window->state);
}

const std::string* desired_state = bounds->FindStringKey("windowState");
const std::string* desired_state = bounds->FindString("windowState");

if (desired_state && *desired_state == "fullscreen" &&
!GetBrowserInfo()->is_headless) {
Expand Down Expand Up @@ -535,11 +534,11 @@ Status ChromeImpl::SetWindowBounds(
result->FindKeyOfType("height", base::Value::Type::INTEGER);
if (width == nullptr || height == nullptr)
return Status(kUnknownError, "unexpected JavaScript result");
auto window_bounds = std::make_unique<base::DictionaryValue>();
window_bounds->SetIntKey("width", width->GetInt());
window_bounds->SetIntKey("height", height->GetInt());
window_bounds->SetIntKey("left", 0);
window_bounds->SetIntKey("top", 0);
auto window_bounds = std::make_unique<base::Value::Dict>();
window_bounds->Set("width", width->GetInt());
window_bounds->Set("height", height->GetInt());
window_bounds->Set("left", 0);
window_bounds->Set("top", 0);
params.Set("bounds", window_bounds->Clone());
return devtools_websocket_client_->SendCommand("Browser.setWindowBounds",
params);
Expand Down Expand Up @@ -599,22 +598,22 @@ Status ChromeImpl::GetWebViewsInfo(WebViewsInfo* views_info) {
for (const base::Value& info_value : target_infos->GetList()) {
if (!info_value.is_dict())
return Status(kUnknownError, "DevTools contains non-dictionary item");
const base::DictionaryValue& info =
base::Value::AsDictionaryValue(info_value);
std::string id;
if (!info.GetString("targetId", &id))
const base::Value::Dict* info = info_value.GetIfDict();
DCHECK(info);
const std::string* id = info->FindString("targetId");
if (!id)
return Status(kUnknownError, "DevTools did not include id");
std::string type_as_string;
if (!info.GetString("type", &type_as_string))
const std::string* type_as_string = info->FindString("type");
if (!type_as_string)
return Status(kUnknownError, "DevTools did not include type");
std::string url;
if (!info.GetString("url", &url))
const std::string* url = info->FindString("url");
if (!url)
return Status(kUnknownError, "DevTools did not include url");
WebViewInfo::Type type;
Status parse_status = ParseType(type_as_string, &type);
Status parse_status = ParseType(*type_as_string, &type);
if (parse_status.IsError())
return parse_status;
temp_views_info.emplace_back(id, std::string(), url, type);
temp_views_info.emplace_back(*id, std::string(), *url, type);
}
*views_info = WebViewsInfo(temp_views_info);
return status;
Expand Down Expand Up @@ -739,7 +738,7 @@ Status ChromeImpl::SetAcceptInsecureCerts() {
}

Status ChromeImpl::SetPermission(
std::unique_ptr<base::DictionaryValue> permission_descriptor,
std::unique_ptr<base::Value::Dict> permission_descriptor,
PermissionState desired_state,
bool unused_one_realm, // This is ignored. https://crbug.com/977612.
WebView* current_view) {
Expand Down
15 changes: 7 additions & 8 deletions chrome/test/chromedriver/chrome/chrome_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,17 @@ class ChromeImpl : public Chrome {
std::string* window_handle) override;
Status GetWindowRect(const std::string& id, WindowRect* rect) override;
Status SetWindowRect(const std::string& target_id,
const base::DictionaryValue& params) override;
const base::Value::Dict& params) override;
Status MaximizeWindow(const std::string& target_id) override;
Status MinimizeWindow(const std::string& target_id) override;
Status FullScreenWindow(const std::string& target_id) override;
Status CloseWebView(const std::string& id) override;
Status ActivateWebView(const std::string& id) override;
Status SetAcceptInsecureCerts() override;
Status SetPermission(
std::unique_ptr<base::DictionaryValue> permission_descriptor,
PermissionState desired_state,
bool unused_one_realm,
WebView* current_view) override;
Status SetPermission(std::unique_ptr<base::Value::Dict> permission_descriptor,
PermissionState desired_state,
bool unused_one_realm,
WebView* current_view) override;
bool IsMobileEmulationEnabled() const override;
bool HasTouchScreen() const override;
std::string page_load_strategy() const override;
Expand Down Expand Up @@ -91,7 +90,7 @@ class ChromeImpl : public Chrome {
Status GetWindowBounds(int window_id, Window* window);
Status SetWindowBounds(Window* window,
const std::string& target_id,
std::unique_ptr<base::DictionaryValue> bounds);
std::unique_ptr<base::Value::Dict> bounds);
Status GetWebViewsInfo(WebViewsInfo* views_info);

bool quit_ = false;
Expand All @@ -102,7 +101,7 @@ class ChromeImpl : public Chrome {

private:
static Status PermissionNameToChromePermissions(
const base::DictionaryValue& permission_descriptor,
const base::Value::Dict& permission_descriptor,
Chrome::PermissionState setting,
std::vector<std::string>* chrome_permissions);

Expand Down
4 changes: 2 additions & 2 deletions chrome/test/chromedriver/chrome/stub_chrome.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Status StubChrome::GetWindowRect(const std::string& id, WindowRect* rect) {
}

Status StubChrome::SetWindowRect(const std::string& target_id,
const base::DictionaryValue& params) {
const base::Value::Dict& params) {
return Status(kOk);
}

Expand Down Expand Up @@ -76,7 +76,7 @@ Status StubChrome::SetAcceptInsecureCerts() {
}

Status StubChrome::SetPermission(
std::unique_ptr<base::DictionaryValue> permission_descriptor,
std::unique_ptr<base::Value::Dict> permission_descriptor,
Chrome::PermissionState desired_state,
bool one_realm,
WebView* current_view) {
Expand Down
11 changes: 5 additions & 6 deletions chrome/test/chromedriver/chrome/stub_chrome.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,17 @@ class StubChrome : public Chrome {
std::string* window_handle) override;
Status GetWindowRect(const std::string& id, WindowRect* rect) override;
Status SetWindowRect(const std::string& target_id,
const base::DictionaryValue& params) override;
const base::Value::Dict& params) override;
Status MaximizeWindow(const std::string& target_id) override;
Status MinimizeWindow(const std::string& target_id) override;
Status FullScreenWindow(const std::string& target_id) override;
Status CloseWebView(const std::string& id) override;
Status ActivateWebView(const std::string& id) override;
Status SetAcceptInsecureCerts() override;
Status SetPermission(
std::unique_ptr<base::DictionaryValue> permission_descriptor,
Chrome::PermissionState desired_state,
bool one_realm,
WebView* current_view) override;
Status SetPermission(std::unique_ptr<base::Value::Dict> permission_descriptor,
Chrome::PermissionState desired_state,
bool one_realm,
WebView* current_view) override;
std::string GetOperatingSystemName() override;
bool IsMobileEmulationEnabled() const override;
bool HasTouchScreen() const override;
Expand Down

0 comments on commit 9ffd105

Please sign in to comment.