Skip to content

Commit

Permalink
CodeHealth: Remove DictionaryValue from chrome/test/chromedriver p4
Browse files Browse the repository at this point in the history
Removes DictionaryValue and ListValue from HeapSnapshotTacker and
JavascriptDialogManager and MobileEmulationOverrideManager. Keeps an
override which still has DictionaryValue but that function just
forwards to the Value::Dict version. This is to keep changes smaller.

Test: existing tests
Bug: 1187061
Change-Id: I9b7bf63b11b1cac07b3f49eafab6decfe2487c02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4035487
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Vladimir Nechaev <nechaev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073377}
  • Loading branch information
Sammie Quon authored and Chromium LUCI CQ committed Nov 18, 2022
1 parent 3042cee commit 707327f
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 27 deletions.
13 changes: 9 additions & 4 deletions chrome/test/chromedriver/chrome/heap_snapshot_taker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <utility>

#include "base/json/json_reader.h"
#include "base/values.h"
#include "chrome/test/chromedriver/chrome/devtools_client.h"
#include "chrome/test/chromedriver/chrome/status.h"

Expand Down Expand Up @@ -62,13 +61,19 @@ bool HeapSnapshotTaker::ListensToConnections() const {
Status HeapSnapshotTaker::OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) {
return OnEvent(client, method, params.GetDict());
}

Status HeapSnapshotTaker::OnEvent(DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params) {
if (method == "HeapProfiler.addHeapSnapshotChunk") {
std::string chunk;
if (!params.GetString("chunk", &chunk)) {
const std::string* chunk = params.FindString("chunk");
if (!chunk) {
return Status(kUnknownError,
"HeapProfiler.addHeapSnapshotChunk has no 'chunk'");
}
snapshot_.append(chunk);
snapshot_.append(*chunk);
}
return Status(kOk);
}
9 changes: 4 additions & 5 deletions chrome/test/chromedriver/chrome/heap_snapshot_taker.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "chrome/test/chromedriver/chrome/devtools_event_listener.h"

namespace base {
class DictionaryValue;
class Value;
}

class DevToolsClient;
class Status;

Expand All @@ -36,6 +32,9 @@ class HeapSnapshotTaker : public DevToolsEventListener {
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) override;
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params);

private:
Status TakeSnapshotInternal();
Expand Down
24 changes: 16 additions & 8 deletions chrome/test/chromedriver/chrome/javascript_dialog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "chrome/test/chromedriver/chrome/javascript_dialog_manager.h"

#include "base/values.h"
#include "chrome/test/chromedriver/chrome/browser_info.h"
#include "chrome/test/chromedriver/chrome/devtools_client.h"
#include "chrome/test/chromedriver/chrome/status.h"
Expand Down Expand Up @@ -79,22 +78,31 @@ Status JavaScriptDialogManager::OnConnected(DevToolsClient* client) {
Status JavaScriptDialogManager::OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) {
return OnEvent(client, method, params.GetDict());
}

Status JavaScriptDialogManager::OnEvent(DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params) {
if (method == "Page.javascriptDialogOpening") {
std::string message;
if (!params.GetString("message", &message))
const std::string* message = params.FindString("message");
if (!message)
return Status(kUnknownError, "dialog event missing or invalid 'message'");

unhandled_dialog_queue_.push_back(message);
unhandled_dialog_queue_.push_back(*message);

std::string type;
if (!params.GetString("type", &type))
const std::string* type = params.FindString("type");
if (!type)
return Status(kUnknownError, "dialog has invalid 'type'");

dialog_type_queue_.push_back(type);
dialog_type_queue_.push_back(*type);

if (!params.GetString("defaultPrompt", &prompt_text_))
const std::string* prompt_text = params.FindString("defaultPrompt");
if (!prompt_text) {
return Status(kUnknownError,
"dialog event missing or invalid 'defaultPrompt'");
}
prompt_text_ = *prompt_text;
} else if (method == "Page.javascriptDialogClosed") {
// Inspector only sends this event when all dialogs have been closed.
// Clear the unhandled queue in case the user closed a dialog manually.
Expand Down
8 changes: 4 additions & 4 deletions chrome/test/chromedriver/chrome/javascript_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "chrome/test/chromedriver/chrome/devtools_event_listener.h"

namespace base {
class DictionaryValue;
}

struct BrowserInfo;
class DevToolsClient;
class Status;
Expand Down Expand Up @@ -43,6 +40,9 @@ class JavaScriptDialogManager : public DevToolsEventListener {
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) override;
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params);

private:
raw_ptr<DevToolsClient> client_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "chrome/test/chromedriver/chrome/mobile_emulation_override_manager.h"

#include "base/values.h"
#include "chrome/test/chromedriver/chrome/device_metrics.h"
#include "chrome/test/chromedriver/chrome/devtools_client.h"
#include "chrome/test/chromedriver/chrome/status.h"
Expand All @@ -29,8 +28,15 @@ Status MobileEmulationOverrideManager::OnEvent(
DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) {
return OnEvent(client, method, params.GetDict());
}

Status MobileEmulationOverrideManager::OnEvent(
DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params) {
if (method == "Page.frameNavigated") {
if (!params.FindPath("frame.parentId"))
if (!params.FindByDottedPath("frame.parentId"))
return ApplyOverrideIfNeeded();
}
return Status(kOk);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "chrome/test/chromedriver/chrome/devtools_event_listener.h"

namespace base {
class DictionaryValue;
}

class DevToolsClient;
struct DeviceMetrics;
class Status;
Expand All @@ -38,6 +35,9 @@ class MobileEmulationOverrideManager : public DevToolsEventListener {
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) override;
Status OnEvent(DevToolsClient* client,
const std::string& method,
const base::Value::Dict& params);

bool IsEmulatingTouch() const;
bool HasOverrideMetrics() const;
Expand Down

0 comments on commit 707327f

Please sign in to comment.