Revert "Adjust tests for upcoming cross-site cookie requirements"

This reverts commit 83b71438.

Reason for revert: SitePerProcessSSLBrowserTest.UnloadHandlersArePowerful*
failing on Android. Please see bug.

Bug: 1014956
Original change's description:
> Adjust tests for upcoming cross-site cookie requirements
> 
> They will need to be marked explicitly and secure, so adjust some tests to
> be over SSL and mark those cookies appropriately
> 
> Bug: 1006816
> Change-Id: I6da53ea64a7c03530335913fadb9099992e8ca33
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863389
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#706108}

TBR=avi@chromium.org,morlovich@chromium.org

Change-Id: I982bc1590171f92883000c8315d3be77f7b35b9f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1006816
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865249Reviewed-by: Parastoo Geranmayeh <parastoog@google.com>
Commit-Queue: Parastoo Geranmayeh <parastoog@google.com>
Cr-Commit-Position: refs/heads/master@{#706453}

content/browser/frame_host/render_frame_host_impl.h

@@ -1332,9 +1332,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   FRIEND_TEST_ALL_PREFIXES(
       SitePerProcessBrowserTest,
       IsDetachedSubframeObservableDuringUnloadHandlerCrossProcess);
-  FRIEND_TEST_ALL_PREFIXES(SitePerProcessSSLBrowserTest,
+  FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
                            UnloadHandlersArePowerful);
-  FRIEND_TEST_ALL_PREFIXES(SitePerProcessSSLBrowserTest,
+  FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
                            UnloadHandlersArePowerfulGrandChild);
 
   class DroppedInterfaceRequestLogger;

content/browser/navigation_browsertest.cc

@@ -16,7 +16,6 @@
 #include "base/test/test_timeouts.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
-#include "components/network_session_configurator/common/network_switches.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -2568,47 +2567,19 @@ IN_PROC_BROWSER_TEST_P(NavigationBaseBrowserTest,
   console_delegate_2->Wait();
 }
 
-// Tests for cookies. Provides an HTTPS server.
-class NavigationCookiesBrowserTest : public NavigationBaseBrowserTest {
- protected:
-  NavigationCookiesBrowserTest()
-      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
-
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    NavigationBaseBrowserTest::SetUpCommandLine(command_line);
-
-    // This is necessary to use https with arbitrary hostnames.
-    command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
-  }
-
-  void SetUpOnMainThread() override {
-    https_server()->AddDefaultHandlers(GetTestDataFilePath());
-    NavigationBaseBrowserTest::SetUpOnMainThread();
-  }
-
-  net::EmbeddedTestServer* https_server() { return &https_server_; }
-
- private:
-  net::EmbeddedTestServer https_server_;
-};
-
-INSTANTIATE_TEST_SUITE_P(/* no prefix */,
-                         NavigationCookiesBrowserTest,
-                         ::testing::Bool());
-
 // Test how cookies are inherited in about:srcdoc iframes.
 //
 // Regression test: https://crbug.com/1003167.
-IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedSrcDoc) {
+IN_PROC_BROWSER_TEST_P(NavigationBaseBrowserTest, CookiesInheritedSrcDoc) {
   using Response = net::test_server::ControllableHttpResponse;
-  Response response_1(https_server(), "/response_1");
-  Response response_2(https_server(), "/response_2");
-  Response response_3(https_server(), "/response_3");
+  Response response_1(embedded_test_server(), "/response_1");
+  Response response_2(embedded_test_server(), "/response_2");
+  Response response_3(embedded_test_server(), "/response_3");
 
-  ASSERT_TRUE(https_server()->Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
 
-  GURL url_a(https_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(https_server()->GetURL("b.com", "/title1.html"));
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url_a));
 
   EXPECT_TRUE(ExecJs(shell(), R"(
@@ -2665,8 +2636,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedSrcDoc) {
   EXPECT_EQ("", EvalJs(sub_document_2, "document.cookie"));
 
   // 6. Set a cookie in the child. It doesn't affect its parent.
-  EXPECT_TRUE(ExecJs(sub_document_2,
-                     "document.cookie = 'd=0; SameSite=none; Secure';"));
+  EXPECT_TRUE(ExecJs(sub_document_2, "document.cookie = 'd=0';"));
 
   EXPECT_EQ("a=0; b=0; c=0", EvalJs(main_document, "document.cookie"));
   EXPECT_EQ("d=0", EvalJs(sub_document_2, "document.cookie"));
@@ -2713,21 +2683,20 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedSrcDoc) {
 }
 
 // Test how cookies are inherited in about:blank iframes.
-IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
-                       CookiesInheritedAboutBlank) {
+IN_PROC_BROWSER_TEST_P(NavigationBaseBrowserTest, CookiesInheritedAboutBlank) {
   // This test expects several cross-site navigation to happen.
   if (!AreAllSitesIsolatedForTesting())
     return;
 
   using Response = net::test_server::ControllableHttpResponse;
-  Response response_1(https_server(), "/response_1");
-  Response response_2(https_server(), "/response_2");
-  Response response_3(https_server(), "/response_3");
+  Response response_1(embedded_test_server(), "/response_1");
+  Response response_2(embedded_test_server(), "/response_2");
+  Response response_3(embedded_test_server(), "/response_3");
 
-  ASSERT_TRUE(https_server()->Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
 
-  GURL url_a(https_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(https_server()->GetURL("b.com", "/title1.html"));
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url_a));
 
   EXPECT_TRUE(
@@ -2769,7 +2738,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
   EXPECT_EQ("a=0; b=0", EvalJs(sub_document_1, "document.cookie"));
 
   // 3. Checks cookies are sent while requesting resources.
-  GURL url_response_1 = https_server()->GetURL("a.com", "/response_1");
+  GURL url_response_1 = embedded_test_server()->GetURL("a.com", "/response_1");
   EXPECT_TRUE(ExecJs(sub_document_1, JsReplace("fetch($1)", url_response_1)));
   response_1.WaitForRequest();
   EXPECT_EQ("a=0; b=0", response_1.http_request()->headers.at("Cookie"));
@@ -2790,8 +2759,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
   EXPECT_EQ("", EvalJs(sub_document_2, "document.cookie"));
 
   // 6. Set a cookie in the child. It doesn't affect its parent.
-  EXPECT_TRUE(ExecJs(sub_document_2,
-                     "document.cookie = 'd=0; SameSite=none; Secure';"));
+  EXPECT_TRUE(ExecJs(sub_document_2, "document.cookie = 'd=0';"));
 
   EXPECT_EQ("a=0; b=0; c=0", EvalJs(main_document, "document.cookie"));
   EXPECT_EQ("d=0", EvalJs(sub_document_2, "document.cookie"));
@@ -2838,25 +2806,23 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
 
 // Test how cookies are inherited in about:blank iframes.
 //
-// This is a variation of
-// NavigationCookiesBrowserTest.CookiesInheritedAboutBlank. Instead of
-// requesting an history navigation, a new navigation is requested from the main
-// frame. The navigation is cross-site instead of being same-site.
-IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
-                       CookiesInheritedAboutBlank2) {
+// This is a variation of NavigationBaseBrowserTest.CookiesInheritedAboutBlank.
+// Instead of requesting an history navigation, a new navigation is requested
+// from the main frame. The navigation is cross-site instead of being same-site.
+IN_PROC_BROWSER_TEST_P(NavigationBaseBrowserTest, CookiesInheritedAboutBlank2) {
   // This test expects several cross-site navigation to happen.
   if (!AreAllSitesIsolatedForTesting())
     return;
 
   using Response = net::test_server::ControllableHttpResponse;
-  Response response_1(https_server(), "/response_1");
-  Response response_2(https_server(), "/response_2");
-  Response response_3(https_server(), "/response_3");
+  Response response_1(embedded_test_server(), "/response_1");
+  Response response_2(embedded_test_server(), "/response_2");
+  Response response_3(embedded_test_server(), "/response_3");
 
-  ASSERT_TRUE(https_server()->Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
 
-  GURL url_a(https_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(https_server()->GetURL("b.com", "/title1.html"));
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url_a));
 
   EXPECT_TRUE(
@@ -2917,8 +2883,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
   EXPECT_EQ("", EvalJs(sub_document_2, "document.cookie"));
 
   // 6. Set a cookie in the child. It doesn't affect its parent.
-  EXPECT_TRUE(ExecJs(sub_document_2,
-                     "document.cookie = 'd=0; SameSite=none; Secure';"));
+  EXPECT_TRUE(ExecJs(sub_document_2, "document.cookie = 'd=0';"));
 
   EXPECT_EQ("a=0; b=0; c=0", EvalJs(main_document, "document.cookie"));
   EXPECT_EQ("d=0", EvalJs(sub_document_2, "document.cookie"));
@@ -2966,16 +2931,16 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest,
 }
 
 // Test how cookies are inherited in data-URL iframes.
-IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
+IN_PROC_BROWSER_TEST_P(NavigationBaseBrowserTest, CookiesInheritedDataUrl) {
   using Response = net::test_server::ControllableHttpResponse;
-  Response response_1(https_server(), "/response_1");
-  Response response_2(https_server(), "/response_2");
-  Response response_3(https_server(), "/response_3");
+  Response response_1(embedded_test_server(), "/response_1");
+  Response response_2(embedded_test_server(), "/response_2");
+  Response response_3(embedded_test_server(), "/response_3");
 
-  ASSERT_TRUE(https_server()->Start());
+  ASSERT_TRUE(embedded_test_server()->Start());
 
-  GURL url_a(https_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(https_server()->GetURL("b.com", "/title1.html"));
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url_a));
 
   EXPECT_TRUE(ExecJs(shell(), R"(
@@ -3016,7 +2981,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
   // the data-URL.
   EXPECT_TRUE(ExecJs(main_document, "document.cookie = 'a=0;SameSite=Lax'"));
   EXPECT_TRUE(ExecJs(main_document, "document.cookie = 'b=0;SameSite=Strict'"));
-  GURL url_response_1 = https_server()->GetURL("a.com", "/response_1");
+  GURL url_response_1 = embedded_test_server()->GetURL("a.com", "/response_1");
   EXPECT_TRUE(ExecJs(sub_document_1, JsReplace("fetch($1)", url_response_1)));
   response_1.WaitForRequest();
   EXPECT_EQ(0u, response_1.http_request()->headers.count("Cookie"));
@@ -3053,7 +3018,7 @@ IN_PROC_BROWSER_TEST_P(NavigationCookiesBrowserTest, CookiesInheritedDataUrl) {
   console_delegate_4->Wait();
 
   // 7. No cookies are sent when requested from the data-URL.
-  GURL url_response_2 = https_server()->GetURL("a.com", "/response_2");
+  GURL url_response_2 = embedded_test_server()->GetURL("a.com", "/response_2");
   EXPECT_TRUE(ExecJs(sub_document_2, JsReplace("fetch($1)", url_response_2)));
   response_2.WaitForRequest();
   EXPECT_EQ(0u, response_2.http_request()->headers.count("Cookie"));

content/browser/site_per_process_unload_browsertest.cc

@@ -22,7 +22,6 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
-#include "components/network_session_configurator/common/network_switches.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
@@ -1293,32 +1292,6 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   delete_B1.WaitUntilDeleted();
 }
 
-// Some tests need an https server because third-party cookies are used, and
-// SameSite=None cookies must be Secure. This is a separate fixture due to
-// kIgnoreCertificateErrors flag.
-class SitePerProcessSSLBrowserTest : public SitePerProcessBrowserTest {
- protected:
-  SitePerProcessSSLBrowserTest()
-      : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}
-
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    SitePerProcessBrowserTest::SetUpCommandLine(command_line);
-    // This is necessary to use https with arbitrary hostnames.
-    command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
-  }
-
-  void SetUpOnMainThread() override {
-    https_server()->AddDefaultHandlers(GetTestDataFilePath());
-    ASSERT_TRUE(https_server()->Start());
-    SitePerProcessBrowserTest::SetUpOnMainThread();
-  }
-
-  net::EmbeddedTestServer* https_server() { return &https_server_; }
-
- private:
-  net::EmbeddedTestServer https_server_;
-};
-
 // Unload handlers should be able to do things that might require for instance
 // the RenderFrameHostImpl to stay alive.
 // - use console.log (handled via RFHI::DidAddMessageToConsole).
@@ -1335,11 +1308,10 @@ class SitePerProcessSSLBrowserTest : public SitePerProcessBrowserTest {
 //
 // This test is similar to UnloadHandlersArePowerfulGrandChild, but with a
 // different frame hierarchy.
-IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
-                       UnloadHandlersArePowerful) {
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, UnloadHandlersArePowerful) {
   // Navigate to a page hosting a cross-origin frame.
-  GURL url =
-      https_server()->GetURL("a.com", "/cross_site_iframe_factory.html?a(b)");
+  GURL url = embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b)");
   EXPECT_TRUE(NavigateToURL(shell(), url));
 
   RenderFrameHostImpl* A1 = web_contents()->GetMainFrame();
@@ -1368,8 +1340,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
 
       // As a sanity check, test that RFHI-independent things also work fine.
       localStorage.localstorage_test_key = 'localstorage_test_value';
-      document.cookie = 'cookie_test_key=' +
-                        'cookie_test_value; SameSite=none; Secure';
+      document.cookie = 'cookie_test_key=' + 'cookie_test_value';
     });
   )"));
 
@@ -1381,7 +1352,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
     RenderFrameDeletedObserver B2_deleted(B2);
 
     // Navigate
-    GURL away_url(https_server()->GetURL("a.com", "/title1.html"));
+    GURL away_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
     ASSERT_TRUE(ExecJs(A1, JsReplace("location = $1", away_url)));
 
     // Observers must be reached.
@@ -1426,11 +1397,11 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
 //
 // This test is similar to UnloadHandlersArePowerful, but with a different frame
 // hierarchy.
-IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                        UnloadHandlersArePowerfulGrandChild) {
   // Navigate to a page hosting a cross-origin frame.
-  GURL url = https_server()->GetURL("a.com",
-                                    "/cross_site_iframe_factory.html?a(b(c))");
+  GURL url = embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b(c))");
   EXPECT_TRUE(NavigateToURL(shell(), url));
 
   RenderFrameHostImpl* A1 = web_contents()->GetMainFrame();
@@ -1461,8 +1432,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
 
       // As a sanity check, test that RFHI-independent things also work fine.
       localStorage.localstorage_test_key = 'localstorage_test_value';
-      document.cookie = 'cookie_test_key=' +
-                        'cookie_test_value; SameSite=none; Secure';
+      document.cookie = 'cookie_test_key=' + 'cookie_test_value';
     });
   )"));
 
@@ -1475,7 +1445,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
     RenderFrameDeletedObserver C3_deleted(C3);
 
     // Navigate
-    GURL away_url(https_server()->GetURL("a.com", "/title1.html"));
+    GURL away_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
     ASSERT_TRUE(ExecJs(A1, JsReplace("location = $1", away_url)));
 
     // Observers must be reached.