Commit c92df469 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

Ensure test settings are fully reset between tests

Javascript can run after BlinkTestRunner::OnResetRendererAfterWebTest,
dirtying TestRunner state for the next test. A navigation to about:blank
is done after every test. This patch resets the TestRunner state after
the navigation to about:blank, when we are sure no test javascript can
change TestRunner state. This fixes a class of flaky bugs.

This behavior happened with video-overlay-scroll.html which would set a
custom layout dump (setCustomTextOutput) in a fullscreenchange event
handler. This event handler was not removed at the end of the test
(this will be fixed in a followup), and would run a second time, after
the custom text output setting was reset. This caused later tests to
run with custom text output from video-overlay-scroll.html. With this
patch, video-scrolled-iframe.html is no longer flaky.

Code in WebFrameTestProxy::BeginNavigation to intercept navigations
relied on TestRunner::PolicyDelegateEnabled being reset before it ran.
This patch now skips WebFrameTestProxy::BeginNavigation when the test
is not running.

Bug: 1048597
Change-Id: I888f7cc2ae91d7d3cfdd143a7573f62cb052c1cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275348Reviewed-by: default avatardanakj <danakj@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Auto-Submit: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784110}
parent 98c00679
...@@ -465,6 +465,7 @@ void BlinkTestRunner::DidCommitNavigationInMainFrame() { ...@@ -465,6 +465,7 @@ void BlinkTestRunner::DidCommitNavigationInMainFrame() {
return; return;
waiting_for_reset_navigation_to_about_blank_ = false; waiting_for_reset_navigation_to_about_blank_ = false;
web_view_test_proxy_->test_interfaces()->ResetAll();
GetWebTestControlHostRemote()->ResetRendererAfterWebTestDone(); GetWebTestControlHostRemote()->ResetRendererAfterWebTestDone();
} }
...@@ -553,8 +554,10 @@ void BlinkTestRunner::OnResetRendererAfterWebTest() { ...@@ -553,8 +554,10 @@ void BlinkTestRunner::OnResetRendererAfterWebTest() {
// BlinkTestMsg_Reset should always be sent to the *current* view. // BlinkTestMsg_Reset should always be sent to the *current* view.
DCHECK(web_view_test_proxy_->GetMainRenderFrame()); DCHECK(web_view_test_proxy_->GetMainRenderFrame());
TestInterfaces* interfaces = web_view_test_proxy_->test_interfaces(); // Instead of resetting for the next test here, delay until after the
interfaces->ResetAll(); // navigation to about:blank (this is in |DidCommitNavigationInMainFrame()|).
// This ensures we reset settings that are set between now and the load of
// about:blank.
// Navigating to about:blank will make sure that no new loads are initiated // Navigating to about:blank will make sure that no new loads are initiated
// by the renderer. // by the renderer.
......
...@@ -478,6 +478,13 @@ void WebFrameTestProxy::WillSendRequest(blink::WebURLRequest& request) { ...@@ -478,6 +478,13 @@ void WebFrameTestProxy::WillSendRequest(blink::WebURLRequest& request) {
void WebFrameTestProxy::BeginNavigation( void WebFrameTestProxy::BeginNavigation(
std::unique_ptr<blink::WebNavigationInfo> info) { std::unique_ptr<blink::WebNavigationInfo> info) {
// This check for whether the test is running ensures we do not intercept
// the about:blank navigation between tests.
if (!test_runner()->TestIsRunning()) {
RenderFrameImpl::BeginNavigation(std::move(info));
return;
}
if (test_runner()->ShouldDumpNavigationPolicy()) { if (test_runner()->ShouldDumpNavigationPolicy()) {
blink_test_runner()->PrintMessage( blink_test_runner()->PrintMessage(
"Default policy for navigation to '" + "Default policy for navigation to '" +
......
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