Commit 107a4008 authored by John Delaney's avatar John Delaney Committed by Chromium LUCI CQ

Fix racy NavigationBrowserTest usage of TestNavigationObserver

What:
Replace usage of ShellAddedObserver + TestNavigationObserver with
TestNavigationObserver::StartWatchingNewWebContents().

Why:
Tests are flaking from calls to TestNavigationObserver::Wait()
hanging.

The existing solution assumes that the test will setup the
TestNavigationObserver before the newly opened WebContents starts the
navigation.

However this isn't necessarily true, as we aren't registering the
TestNavigationObserver in the same callstack in which the web contents
is created.

Bug: 1166341
Change-Id: Id2b5f36158f86377b3881e54d793c27af718cb1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627757Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843300}
parent c37ce618
...@@ -1048,15 +1048,14 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, ...@@ -1048,15 +1048,14 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
&success)); &success));
success = false; success = false;
ShellAddedObserver new_shell_observer; TestNavigationObserver observer(url);
observer.StartWatchingNewWebContents();
EXPECT_TRUE(ExecuteScriptAndExtractBool( EXPECT_TRUE(ExecuteScriptAndExtractBool(
shell(), shell(),
"window.domAutomationController.send(clickCrossSiteNewWindowLink());", "window.domAutomationController.send(clickCrossSiteNewWindowLink());",
&success)); &success));
EXPECT_TRUE(success); EXPECT_TRUE(success);
TestNavigationObserver observer(
new_shell_observer.GetShell()->web_contents());
observer.Wait(); observer.Wait();
EXPECT_EQ(url, observer.last_navigation_url()); EXPECT_EQ(url, observer.last_navigation_url());
EXPECT_TRUE(observer.last_navigation_succeeded()); EXPECT_TRUE(observer.last_navigation_succeeded());
...@@ -1090,7 +1089,8 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, ...@@ -1090,7 +1089,8 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
&success)); &success));
success = false; success = false;
ShellAddedObserver new_shell_observer; TestNavigationObserver observer(url);
observer.StartWatchingNewWebContents();
EXPECT_TRUE( EXPECT_TRUE(
ExecuteScriptAndExtractBool(shell(), ExecuteScriptAndExtractBool(shell(),
"window.domAutomationController.send(" "window.domAutomationController.send("
...@@ -1098,8 +1098,6 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, ...@@ -1098,8 +1098,6 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
&success)); &success));
EXPECT_TRUE(success); EXPECT_TRUE(success);
TestNavigationObserver observer(
new_shell_observer.GetShell()->web_contents());
observer.Wait(); observer.Wait();
EXPECT_EQ(url, observer.last_navigation_url()); EXPECT_EQ(url, observer.last_navigation_url());
...@@ -1139,15 +1137,14 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, ...@@ -1139,15 +1137,14 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
&success)); &success));
success = false; success = false;
ShellAddedObserver new_shell_observer; TestNavigationObserver observer(url);
observer.StartWatchingNewWebContents();
EXPECT_TRUE(ExecuteScriptAndExtractBool( EXPECT_TRUE(ExecuteScriptAndExtractBool(
subframe_rfh, subframe_rfh,
"window.domAutomationController.send(clickCrossSiteNewWindowLink());", "window.domAutomationController.send(clickCrossSiteNewWindowLink());",
&success)); &success));
EXPECT_TRUE(success); EXPECT_TRUE(success);
TestNavigationObserver observer(
new_shell_observer.GetShell()->web_contents());
observer.Wait(); observer.Wait();
EXPECT_EQ(url, observer.last_navigation_url()); EXPECT_EQ(url, observer.last_navigation_url());
EXPECT_TRUE(observer.last_navigation_succeeded()); EXPECT_TRUE(observer.last_navigation_succeeded());
...@@ -1222,14 +1219,13 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, ...@@ -1222,14 +1219,13 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
&success)); &success));
success = false; success = false;
ShellAddedObserver new_shell_observer; TestNavigationObserver observer(url);
observer.StartWatchingNewWebContents();
EXPECT_EQ(true, EvalJs(shell(), R"( EXPECT_EQ(true, EvalJs(shell(), R"(
target = document.getElementById('cross_site_link'); target = document.getElementById('cross_site_link');
var evt = new MouseEvent("click", {"button": 1 /* middle_button */}); var evt = new MouseEvent("click", {"button": 1 /* middle_button */});
target.dispatchEvent(evt);)")); target.dispatchEvent(evt);)"));
TestNavigationObserver observer(
new_shell_observer.GetShell()->web_contents());
observer.Wait(); observer.Wait();
EXPECT_EQ(url, observer.last_navigation_url()); EXPECT_EQ(url, observer.last_navigation_url());
EXPECT_TRUE(observer.last_navigation_succeeded()); EXPECT_TRUE(observer.last_navigation_succeeded());
......
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