Skip to content

GitLab

T
tangled
Commit 09c4f971 authored by Maksim Orlovich's avatar Maksim Orlovich Committed by Commit Bot
Browse files
Options

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

This reverts commit a88c5fe6.

Reason for reland: The test was/is flaky on that bot in the first place,
                   the name change just makes it look different.

Original change's description:
> 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/+/1865249
> Reviewed-by: Parastoo Geranmayeh <parastoog@google.com>
> Commit-Queue: Parastoo Geranmayeh <parastoog@google.com>
> Cr-Commit-Position: refs/heads/master@{#706453}

TBR=avi@chromium.org,parastoog@google.com,morlovich@chromium.org

Change-Id: I51c863c89ade36edddc4b1b7a3f16d82c9f855f3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1014956, 1006816
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866614Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707918}
parent e2ad5f4f
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 114 additions and 49 deletions
content/browser/frame_host/render_frame_host_impl.h
View file @ 09c4f971
......@@ -1349,9 +1349,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
FRIEND_TEST_ALL_PREFIXES(
SitePerProcessBrowserTest,
IsDetachedSubframeObservableDuringUnloadHandlerCrossProcess);
FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
FRIEND_TEST_ALL_PREFIXES(SitePerProcessSSLBrowserTest,
UnloadHandlersArePowerful);
FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
FRIEND_TEST_ALL_PREFIXES(SitePerProcessSSLBrowserTest,
UnloadHandlersArePowerfulGrandChild);
class DroppedInterfaceRequestLogger;
......
content/browser/navigation_browsertest.cc
View file @ 09c4f971
This diff is collapsed.
content/browser/site_per_process_unload_browsertest.cc
View file @ 09c4f971
......@@ -22,6 +22,7 @@
#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"
......@@ -1292,6 +1293,32 @@ 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).
......@@ -1308,10 +1335,11 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
//
// This test is similar to UnloadHandlersArePowerfulGrandChild, but with a
// different frame hierarchy.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, UnloadHandlersArePowerful) {
IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
UnloadHandlersArePowerful) {
// Navigate to a page hosting a cross-origin frame.
GURL url = embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)");
GURL url =
https_server()->GetURL("a.com", "/cross_site_iframe_factory.html?a(b)");
EXPECT_TRUE(NavigateToURL(shell(), url));
RenderFrameHostImpl* A1 = web_contents()->GetMainFrame();
......@@ -1340,7 +1368,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, UnloadHandlersArePowerful) {
// 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';
document.cookie = 'cookie_test_key=' +
'cookie_test_value; SameSite=none; Secure';
});
)"));
......@@ -1352,7 +1381,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, UnloadHandlersArePowerful) {
RenderFrameDeletedObserver B2_deleted(B2);
// Navigate
GURL away_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL away_url(https_server()->GetURL("a.com", "/title1.html"));
ASSERT_TRUE(ExecJs(A1, JsReplace("location = $1", away_url)));
// Observers must be reached.
......@@ -1397,11 +1426,11 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, UnloadHandlersArePowerful) {
//
// This test is similar to UnloadHandlersArePowerful, but with a different frame
// hierarchy.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
IN_PROC_BROWSER_TEST_F(SitePerProcessSSLBrowserTest,
UnloadHandlersArePowerfulGrandChild) {
// Navigate to a page hosting a cross-origin frame.
GURL url = embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b(c))");
GURL url = https_server()->GetURL("a.com",
"/cross_site_iframe_factory.html?a(b(c))");
EXPECT_TRUE(NavigateToURL(shell(), url));
RenderFrameHostImpl* A1 = web_contents()->GetMainFrame();
......@@ -1432,7 +1461,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
// 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';
document.cookie = 'cookie_test_key=' +
'cookie_test_value; SameSite=none; Secure';
});
)"));
......@@ -1445,7 +1475,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
RenderFrameDeletedObserver C3_deleted(C3);
// Navigate
GURL away_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
GURL away_url(https_server()->GetURL("a.com", "/title1.html"));
ASSERT_TRUE(ExecJs(A1, JsReplace("location = $1", away_url)));
// Observers must be reached.
......
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