Commit fbbfe86d authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

[Autofill Assistant] Fix possible ordering problem in WebController test

The devtools backend receives and will handle protocol messages in
order, but there is no guarantee of ordering with other non-protocol
things. This is true for content::ExecJs.

This test sent a Runtime.Evaluate message over the devtools protocol
and then called ExecJs. Due to the way scheduling is handled in the
devtools backend, it's possible that the ExecJs code runs before the
protocol message is handled by the backend.

Callers can get around this by waiting for the first Runtime.Evaluate
call to finish and then only continuing in the callback provided. In
this case, the first JS evaluation sets up a promise that will only
resolve once the second JS evaluation happens, so that won't work.

The solution here is to use a second Runtime.Evaluate call, which
does have an ordering guarantee with the first.

Upcoming changes to the scheduling uncovered this as a flake on
the trybots.

Change-Id: I643539d094407a43b4ffff17dd052e5f49e3e44e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939409Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719538}
parent 4fa85682
...@@ -470,6 +470,15 @@ class WebControllerBrowserTest : public content::ContentBrowserTest, ...@@ -470,6 +470,15 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
EXPECT_LT(top, container_bottom); EXPECT_LT(top, container_bottom);
} }
// Send a Runtime.Evaluate protocol message. Useful for evaluating JS in the
// page as there is no ordering guarantee between protocol messages and e.g.
// ExecJs().
void RuntimeEvaluate(const std::string& code) {
web_controller_->devtools_client_->GetRuntime()->Evaluate(
code,
/* node_frame_id= */ std::string());
}
protected: protected:
std::unique_ptr<WebController> web_controller_; std::unique_ptr<WebController> web_controller_;
...@@ -1307,8 +1316,7 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, WaitForHeightChange) { ...@@ -1307,8 +1316,7 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, WaitForHeightChange) {
base::BindOnce(&WebControllerBrowserTest::OnClientStatus, base::BindOnce(&WebControllerBrowserTest::OnClientStatus,
base::Unretained(this), run_loop.QuitClosure(), &result)); base::Unretained(this), run_loop.QuitClosure(), &result));
EXPECT_TRUE( RuntimeEvaluate("window.dispatchEvent(new Event('resize'))");
content::ExecJs(shell(), "window.dispatchEvent(new Event('resize'))"));
run_loop.Run(); run_loop.Run();
EXPECT_EQ(ACTION_APPLIED, result.proto_status()); EXPECT_EQ(ACTION_APPLIED, result.proto_status());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment