Skip to content

Commit

Permalink
ChromeDriver uses proper WebDriver BiDi extensions
Browse files Browse the repository at this point in the history 
Use proper named custom WebDriver BiDi properties with `goog:` prefix. GoogleChromeLabs/chromium-bidi#2757

Breaking change: legacy BiDi `channel` property is not allowed anymore.

Change-Id: Ib551008ccac59196362ed1899a0e222ef72f6a7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6179410
Commit-Queue: Maksim Sadym <[email protected]>
Reviewed-by: Alex Rudenko <[email protected]>
Reviewed-by: Maksim Sadym <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1409612}
  • Loading branch information
Maksim Sadym authored and Chromium LUCI CQ committed Jan 22, 2025
1 parent 59ae0a6 commit 3e43b97
Show file tree
Hide file tree
Showing 13 changed files with 212 additions and 131 deletions.
4 changes: 2 additions & 2 deletions chrome/test/chromedriver/chrome/bidi_tracker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ Status BidiTracker::OnEvent(DevToolsClient* client,
if (payload == nullptr) {
return Status(kUnknownError, "Runtime.bindingCalled missing 'payload'");
}
const std::string* channel = payload->FindString("channel");
const std::string* channel = payload->FindString("goog:channel");
if (!channel || channel->empty()) {
// Internally we set non-empty channel to any BiDi command.
// Missing or empty channel in the response means that there is a bug.
return Status{kUnknownError, "channel is missing in the payload"};
return Status{kUnknownError, "goog:channel is missing in the payload"};
}
if (!base::EndsWith(*channel, channel_suffix_)) {
return Status{kOk};
Expand Down
21 changes: 11 additions & 10 deletions chrome/test/chromedriver/chrome/bidi_tracker_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ base::Value::Dict CreateValidParams(std::optional<std::string> channel,
payload.Set("id", 1);
payload.Set("result", std::move(result));
if (channel.has_value()) {
payload.Set("channel", std::move(*channel));
payload.Set("goog:channel", std::move(*channel));
}

base::Value::Dict event_params;
Expand Down Expand Up @@ -86,7 +86,7 @@ TEST(BidiTrackerTest, ChannelAndFilter) {
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
EXPECT_TRUE(StatusOk(
tracker.OnEvent(nullptr, "Runtime.bindingCalled", std::move(params))));
EXPECT_THAT(actual_payload.FindString("channel"), Pointee(Eq("/some")));
EXPECT_THAT(actual_payload.FindString("goog:channel"), Pointee(Eq("/some")));
EXPECT_THAT(actual_payload.FindIntByDottedPath("result.pong"),
Optional(Eq(222)));
}
Expand All @@ -99,7 +99,8 @@ TEST(BidiTrackerTest, ChannelLongerThanFilter) {
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
EXPECT_TRUE(StatusOk(
tracker.OnEvent(nullptr, "Runtime.bindingCalled", std::move(params))));
EXPECT_THAT(actual_payload.FindString("channel"), Pointee(Eq("/one/two")));
EXPECT_THAT(actual_payload.FindString("goog:channel"),
Pointee(Eq("/one/two")));
EXPECT_THAT(actual_payload.FindIntByDottedPath("result.pong"),
Optional(Eq(333)));
}
Expand All @@ -122,7 +123,7 @@ TEST(BidiTrackerTest, ChannelAndNoFilter) {
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
EXPECT_TRUE(StatusOk(
tracker.OnEvent(nullptr, "Runtime.bindingCalled", std::move(params))));
EXPECT_THAT(actual_payload.FindString("channel"), Pointee(Eq("/some")));
EXPECT_THAT(actual_payload.FindString("goog:channel"), Pointee(Eq("/some")));
EXPECT_THAT(actual_payload.FindIntByDottedPath("result.pong"),
Optional(Eq(321)));
}
Expand All @@ -131,28 +132,28 @@ TEST(BidiTrackerTest, NoChannelNoFilter) {
// The infrastructure ensures that that there are no missing or empty channels
// If such a channel appears in the response we treat it as an error.
base::Value::Dict params = CreateValidParams("/to-be-removed");
params.RemoveByDottedPath("payload.channel");
params.RemoveByDottedPath("payload.goog:channel");
BidiTracker tracker;
base::Value::Dict actual_payload;
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
Status status = tracker.OnEvent(nullptr, "Runtime.bindingCalled", params);
EXPECT_TRUE(StatusCodeIs<kUnknownError>(status));
EXPECT_THAT(status.message(), ContainsRegex("channel is missing"));
EXPECT_THAT(status.message(), ContainsRegex("goog:channel is missing"));
EXPECT_TRUE(actual_payload.empty());
}

TEST(BidiTrackerTest, NoChannelAndFilter) {
// The infrastructure ensures that that there are no missing or empty channels
// If such a channel appears in the response we treat it as an error.
base::Value::Dict params = CreateValidParams("/to-be-removed");
params.RemoveByDottedPath("payload.channel");
params.RemoveByDottedPath("payload.goog:channel");
BidiTracker tracker;
tracker.SetChannelSuffix("/yyy");
base::Value::Dict actual_payload;
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
Status status = tracker.OnEvent(nullptr, "Runtime.bindingCalled", params);
EXPECT_TRUE(StatusCodeIs<kUnknownError>(status));
EXPECT_THAT(status.message(), ContainsRegex("channel is missing"));
EXPECT_THAT(status.message(), ContainsRegex("goog:channel is missing"));
EXPECT_TRUE(actual_payload.empty());
}

Expand All @@ -165,7 +166,7 @@ TEST(BidiTrackerTest, EmptyChannelNoFilter) {
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
Status status = tracker.OnEvent(nullptr, "Runtime.bindingCalled", params);
EXPECT_TRUE(StatusCodeIs<kUnknownError>(status));
EXPECT_THAT(status.message(), ContainsRegex("channel is missing"));
EXPECT_THAT(status.message(), ContainsRegex("goog:channel is missing"));
EXPECT_TRUE(actual_payload.empty());
}

Expand All @@ -179,7 +180,7 @@ TEST(BidiTrackerTest, EmptyChannelAndFilter) {
tracker.SetBidiCallback(CopyMessageTo(actual_payload));
Status status = tracker.OnEvent(nullptr, "Runtime.bindingCalled", params);
EXPECT_TRUE(StatusCodeIs<kUnknownError>(status));
EXPECT_THAT(status.message(), ContainsRegex("channel is missing"));
EXPECT_THAT(status.message(), ContainsRegex("goog:channel is missing"));
EXPECT_TRUE(actual_payload.empty());
}

Expand Down
32 changes: 17 additions & 15 deletions chrome/test/chromedriver/chrome/devtools_client_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,19 @@ Status WrapCdpCommandInBidiCommand(base::Value::Dict cdp_cmd,
base::Value::Dict* cdp_params = cdp_cmd.FindDict("params");

base::Value::Dict params;
params.Set("cdpMethod", std::move(*cdp_method));
params.Set("method", std::move(*cdp_method));
if (cdp_session_id) {
params.Set("cdpSession", std::move(*cdp_session_id));
params.Set("session", std::move(*cdp_session_id));
}
if (cdp_params) {
params.Set("cdpParams", std::move(*cdp_params));
params.Set("params", std::move(*cdp_params));
}

base::Value::Dict dict;
dict.Set("id", *cdp_cmd_id);
dict.Set("method", "cdp.sendCommand");
dict.Set("method", "goog:cdp.sendCommand");
dict.Set("params", std::move(params));
dict.Set("channel", DevToolsClientImpl::kCdpTunnelChannel);
dict.Set("goog:channel", DevToolsClientImpl::kCdpTunnelChannel);
*bidi_cmd = std::move(dict);
return Status{kOk};
}
Expand Down Expand Up @@ -193,26 +193,28 @@ bool ParseCdpTunnelMessage(base::Value::Dict payload,
// handle CDP over BiDi events and responses
const std::string* payload_method = payload.FindString("method");

if (payload_method && *payload_method == "cdp.eventReceived") { // CDP event
// CDP events in BiDi have format "goog:cdp.<CDP EVENT NAME>".
if (payload_method && base::StartsWith(*payload_method, "goog:cdp.",
base::CompareCase::SENSITIVE)) {
base::Value::Dict* payload_params = payload.FindDict("params");
if (!payload_params) {
LOG(WARNING) << "params field is missing in the payload of "
"Runtime.bindingCalled message";
return false;
}
const std::string* cdp_method = payload_params->FindString("cdpMethod");
const std::string* cdp_method = payload_params->FindString("method");
if (!cdp_method) {
LOG(WARNING) << "params.cdpMethod is missing in the payload of "
LOG(WARNING) << "params.method is missing in the payload of "
"Runtime.bindingCalled message";
return false;
}

type = internal::kEventMessageType;
event.method = *cdp_method;
const std::string* cdp_session = payload_params->FindString("cdpSession");
const std::string* cdp_session = payload_params->FindString("session");
session_id = cdp_session ? *cdp_session : "";

base::Value::Dict* cdp_params = payload_params->FindDict("cdpParams");
base::Value::Dict* cdp_params = payload_params->FindDict("params");
if (cdp_params) {
event.params = std::move(*cdp_params);
} else {
Expand All @@ -226,7 +228,7 @@ bool ParseCdpTunnelMessage(base::Value::Dict payload,
return false;
}

const std::string* cdp_session = payload.FindString("cdpSession");
const std::string* cdp_session = payload.FindString("session");
session_id = cdp_session ? *cdp_session : "";

base::Value::Dict* cdp_result = payload.FindDict("result");
Expand Down Expand Up @@ -424,7 +426,7 @@ Status DevToolsClientImpl::StartBidiServer(

if (event_tunneling_is_enabled_) {
base::Value::Dict params;
params.Set("events", "cdp.eventReceived");
params.Set("events", "goog:cdp");
base::Value::Dict bidi_cmd;
bidi_cmd.Set("id", AdvanceNextMessageId());
bidi_cmd.Set("method", "session.subscribe");
Expand Down Expand Up @@ -601,7 +603,7 @@ Status DevToolsClientImpl::SetUpDevTools() {
}

Status DevToolsClientImpl::PostBidiCommand(base::Value::Dict command) {
std::string* maybe_channel = command.FindString("channel");
std::string* maybe_channel = command.FindString("goog:channel");
std::string channel =
maybe_channel ? *maybe_channel + DevToolsClientImpl::kBidiChannelSuffix
: std::string();
Expand Down Expand Up @@ -817,7 +819,7 @@ Status DevToolsClientImpl::PostBidiCommandInternal(std::string channel,
"uanble to send BiDi commands without BiDi server session id"};
}
if (!channel.empty()) {
command.Set("channel", std::move(channel));
command.Set("goog:channel", std::move(channel));
}

std::string json;
Expand Down Expand Up @@ -1398,7 +1400,7 @@ bool ParseInspectorMessage(const std::string& message,
return false;
}

std::string* channel = payload.FindString("channel");
std::string* channel = payload.FindString("goog:channel");

if (channel && *channel == DevToolsClientImpl::kCdpTunnelChannel) {
return ParseCdpTunnelMessage(std::move(payload), session_id, type,
Expand Down
Loading

0 comments on commit 3e43b97

Please sign in to comment.