From 015f45ff045a334cc59dec3eee0c4ebc24f3885c Mon Sep 17 00:00:00 2001 From: Rakina Zata Amni Date: Thu, 22 Oct 2020 03:36:00 +0000 Subject: [PATCH] Fix tests that 404s or navigates to an empty page with non-OK status These tests do not intend to have failed navigations, but they actually failed silently, mostly due to 404s because of bad URLs. Soon (after crrev.com/c/2487024 lands) these tests will actually fail and result in error pages. See navigation-dev thread for more context: https://groups.google.com/a/chromium.org/g/navigation-dev/c/WbNkf2alpPU/m/2tQQ-cXWBgAJ Bug:1133115 Change-Id: Ib75a04aa90635b9eb778ce2da56fcee83965b053 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2483748 Commit-Queue: Rakina Zata Amni Reviewed-by: Avi Drissman Reviewed-by: David Schinazi Reviewed-by: Bo Cr-Commit-Position: refs/heads/master@{#819688} --- .../test/AwMetricsIntegrationTest.java | 4 ++-- .../test/AwNetworkConfigurationTest.java | 2 +- .../android_webview/test/PopupWindowTest.java | 2 +- .../ssl/security_state_tab_helper_browsertest.cc | 6 +++--- ...ervised_user_navigation_throttle_browsertest.cc | 4 ++-- .../blocked_content/popup_blocker_browsertest.cc | 8 +++++--- .../signin_reauth_view_controller_browsertest.cc | 14 ++++++++++++-- ...ings_clear_browsing_data_handler_browsertest.cc | 2 +- .../components/web_app_url_loader_browsertest.cc | 14 +++++++++----- .../ssl/page_with_form_targeting_http_url.html | 2 +- .../org/chromium/net/test/util/TestWebServer.java | 4 ++-- 11 files changed, 39 insertions(+), 23 deletions(-) diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java index c26d4218e55c4c..0fd60c6bab24c9 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java @@ -334,7 +334,7 @@ public void testRendererHistograms() throws Throwable { // Load a page to ensure the renderer process is created. mRule.loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), - embeddedTestServer.getURL("/simple_page.html")); + embeddedTestServer.getURL("/android_webview/test/data/hello_world.html")); helper.waitForCallback(finalMetricsCollectedCount, 1); // At this point we know one of two things must be true: @@ -353,7 +353,7 @@ public void testRendererHistograms() throws Throwable { // have been copied into the browser process. mRule.loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), - embeddedTestServer.getURL("/simple_page.html")); + embeddedTestServer.getURL("/android_webview/test/data/hello_world.html")); helper.waitForCallback(finalMetricsCollectedCount, 2); Assert.assertEquals(1, diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwNetworkConfigurationTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwNetworkConfigurationTest.java index 317d8c3d126687..5d659581d01251 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/AwNetworkConfigurationTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwNetworkConfigurationTest.java @@ -196,7 +196,7 @@ public void testRequestedWithHeaderShouldInterceptRequest() throws Throwable { mTestServer = EmbeddedTestServer.createAndStartServer( InstrumentationRegistry.getInstrumentation().getContext()); try { - final String url = mTestServer.getURL("/any-http-url-will-suffice.html"); + final String url = mTestServer.getURL("/android_webview/test/data/hello_world.html"); mActivityTestRule.loadUrlSync( mAwContents, mContentsClient.getOnPageFinishedHelper(), url); AwWebResourceRequest request = diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java index f1a822f849a20b..2fb68cf6d1f38f 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java @@ -230,7 +230,7 @@ public void testOnPageFinishedCalledOnDomModificationAfterNavigation() throws Th + "}"); mActivityTestRule.triggerPopup(mParentContents, mParentContentsClient, mWebServer, - parentPageHtml, null /* 204 response */, popupPath, "tryOpenWindow()"); + parentPageHtml, "", popupPath, "tryOpenWindow()"); PopupInfo popupInfo = mActivityTestRule.createPopupContents(mParentContents); TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper = popupInfo.popupContentsClient.getOnPageFinishedHelper(); diff --git a/chrome/browser/ssl/security_state_tab_helper_browsertest.cc b/chrome/browser/ssl/security_state_tab_helper_browsertest.cc index 00a0bfcf1a2810..60c92327eeefeb 100644 --- a/chrome/browser/ssl/security_state_tab_helper_browsertest.cc +++ b/chrome/browser/ssl/security_state_tab_helper_browsertest.cc @@ -2262,8 +2262,8 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, FormSecurityLevelHistogram) { ASSERT_TRUE(broken_https_server.Start()); // Make the form target the expired certificate server. - net::HostPortPair host_port_pair = - net::HostPortPair::FromURL(broken_https_server.GetURL("/google.html")); + net::HostPortPair host_port_pair = net::HostPortPair::FromURL( + broken_https_server.GetURL("/ssl/google.html")); std::string replacement_path = GetFilePathWithHostAndPortReplacement( "/ssl/page_with_form_targeting_insecure_url.html", host_port_pair); ui_test_utils::NavigateToURL(browser(), @@ -2301,7 +2301,7 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, SafetyTipFormHistogram) { // Use a different host for targeting the form so that a Safety Tip doesn't // trigger on the form submission navigation. net::HostPortPair host_port_pair = net::HostPortPair::FromURL( - form_server.GetURL("example.test", "/google.html")); + form_server.GetURL("example.test", "/ssl/google.html")); std::string replacement_path = GetFilePathWithHostAndPortReplacement( "/ssl/page_with_form_targeting_http_url.html", host_port_pair); ui_test_utils::NavigateToURL(browser(), server.GetURL(replacement_path)); diff --git a/chrome/browser/supervised_user/supervised_user_navigation_throttle_browsertest.cc b/chrome/browser/supervised_user/supervised_user_navigation_throttle_browsertest.cc index 5c04ba8550a5e6..31314b83a51c02 100644 --- a/chrome/browser/supervised_user/supervised_user_navigation_throttle_browsertest.cc +++ b/chrome/browser/supervised_user/supervised_user_navigation_throttle_browsertest.cc @@ -604,7 +604,7 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserIframeFilterTest, BlockHost(kExampleHost); GURL blocked_url = embedded_test_server()->GetURL( - kExampleHost, "/supervised_user/with_frames.html"); + kExampleHost, "/supervised_user/with_iframes.html"); ui_test_utils::NavigateToURL(browser(), blocked_url); EXPECT_TRUE(IsInterstitialBeingShownInMainFrame(browser())); @@ -623,7 +623,7 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserIframeFilterTest, // Navigate to another allowed url. GURL allowed_url = embedded_test_server()->GetURL( - kExampleHost2, "/supervised_user/with_frames.html"); + kExampleHost2, "/supervised_user/with_iframes.html"); ui_test_utils::NavigateToURL(browser(), allowed_url); EXPECT_FALSE(IsInterstitialBeingShownInMainFrame(browser())); diff --git a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc index 2aa3f3f962eeb7..6376220ffb66bd 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc +++ b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc @@ -453,7 +453,8 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, NoPopupsLaunchWhenTabIsClosed) { embedded_test_server()->GetURL("/popup_blocker/popup-on-unload.html")); ui_test_utils::NavigateToURL(browser(), url); - GURL url2(embedded_test_server()->GetURL("/popup_blocker/")); + GURL url2( + embedded_test_server()->GetURL("/popup_blocker/popup-success.html")); ui_test_utils::NavigateToURL(browser(), url2); // Expect no popup. @@ -520,8 +521,9 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerSpecialPolicyBrowserTest, DisableProactiveBrowsingInstanceSwapFor( browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame()); - NavigateAndCheckPopupShown(embedded_test_server()->GetURL("/popup_blocker/"), - kExpectPopup); + NavigateAndCheckPopupShown( + embedded_test_server()->GetURL("/popup_blocker/popup-success.html"), + kExpectPopup); } // Verify that when you unblock popup, the popup shows in history and omnibox. diff --git a/chrome/browser/ui/signin_reauth_view_controller_browsertest.cc b/chrome/browser/ui/signin_reauth_view_controller_browsertest.cc index 248ee1f53e1230..59a83bafe0725b 100644 --- a/chrome/browser/ui/signin_reauth_view_controller_browsertest.cc +++ b/chrome/browser/ui/signin_reauth_view_controller_browsertest.cc @@ -79,6 +79,14 @@ std::unique_ptr CreateEmptyResponse( return http_response; } +std::unique_ptr CreateNonEmptyResponse( + net::HttpStatusCode code) { + auto http_response = std::make_unique(); + http_response->set_code(code); + http_response->set_content(""); + return http_response; +} + std::unique_ptr HandleReauthURL( const GURL& base_url, const net::test_server::HttpRequest& request) { @@ -101,8 +109,10 @@ std::unique_ptr HandleReauthURL( } if (parameter == "unexpected") { - // Returns a response that isn't expected by Chrome. - return CreateEmptyResponse(net::HTTP_NOT_IMPLEMENTED); + // Returns a response that isn't expected by Chrome. Note that we shouldn't + // return an empty response here because that will result in an error page + // being committed for the navigation. + return CreateNonEmptyResponse(net::HTTP_NOT_IMPLEMENTED); } NOTREACHED(); diff --git a/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler_browsertest.cc b/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler_browsertest.cc index 15988e98141c07..466b9de8f6c4be 100644 --- a/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler_browsertest.cc +++ b/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler_browsertest.cc @@ -79,7 +79,7 @@ class ClearBrowsingDataHandlerBrowserTest }; IN_PROC_BROWSER_TEST_F(ClearBrowsingDataHandlerBrowserTest, GetInstalledApps) { - GURL url(https_server()->GetURL("/")); + GURL url(https_server()->GetURL("/title1.html")); InstallAndLaunchApp(url); base::ListValue args; args.AppendString(kWebUiFunctionName); diff --git a/chrome/browser/web_applications/components/web_app_url_loader_browsertest.cc b/chrome/browser/web_applications/components/web_app_url_loader_browsertest.cc index a79314fc4e6437..92e2a58da80fa7 100644 --- a/chrome/browser/web_applications/components/web_app_url_loader_browsertest.cc +++ b/chrome/browser/web_applications/components/web_app_url_loader_browsertest.cc @@ -44,14 +44,18 @@ std::unique_ptr HandleServerRedirect( } // Run |handler| on requests that exactly match the |relative_url|. -std::unique_ptr HandleMatchingRequest( +std::unique_ptr +HandleMatchingRequestOrReturnEmptyPage( const std::string& relative_url, const net::EmbeddedTestServer::HandleRequestCallback& handler, const net::test_server::HttpRequest& request) { GURL match = request.GetURL().Resolve(relative_url); if (request.GetURL() == match) return handler.Run(request); - return nullptr; + + auto http_response = std::make_unique(); + http_response->set_code(net::HTTP_OK); + return http_response; } class WebAppUrlLoaderTest : public InProcessBrowserTest { @@ -100,9 +104,9 @@ class WebAppUrlLoaderTest : public InProcessBrowserTest { // Set up a server redirect from |src_relative| (a relative URL) to |dest|. // Must be called before the server is started. void SetupRedirect(const std::string& src_relative, const std::string& dest) { - embedded_test_server()->RegisterRequestHandler( - base::BindRepeating(&HandleMatchingRequest, src_relative, - base::BindRepeating(&HandleServerRedirect, dest))); + embedded_test_server()->RegisterRequestHandler(base::BindRepeating( + &HandleMatchingRequestOrReturnEmptyPage, src_relative, + base::BindRepeating(&HandleServerRedirect, dest))); } private: diff --git a/chrome/test/data/ssl/page_with_form_targeting_http_url.html b/chrome/test/data/ssl/page_with_form_targeting_http_url.html index 4e3a86f22803f1..92ba15d14afd7b 100644 --- a/chrome/test/data/ssl/page_with_form_targeting_http_url.html +++ b/chrome/test/data/ssl/page_with_form_targeting_http_url.html @@ -2,7 +2,7 @@ Page with form that targets an http URL -
+
diff --git a/net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java b/net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java index b47c142e044068..79595faa9e506b 100644 --- a/net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java +++ b/net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java @@ -416,13 +416,13 @@ private void outputResponse(HTTPRequest request, WebServerPrintStream stream) copyHeadersToResponse = false; } } else if (response.mIsNoContent) { - stream.println("HTTP/1.0 200 OK"); + stream.println("HTTP/1.0 204 No Content"); copyHeadersToResponse = false; } else if (response.mIsRedirect) { stream.println("HTTP/1.0 302 Found"); textBody.append(String.format(bodyTemplate, "Found", "Found")); } else if (response.mIsEmptyResponse) { - stream.println("HTTP/1.0 403 Forbidden"); + stream.println("HTTP/1.0 200 OK"); copyHeadersToResponse = false; } else { if (response.mResponseAction != null) response.mResponseAction.run();