diff --git a/DEPS b/DEPS index 9d06c46275248e..d3db05a343ae1e 100644 --- a/DEPS +++ b/DEPS @@ -350,7 +350,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling freetype # and whatever else without interference from each other. - 'freetype_revision': '713580f41dcc2054d7fa56265f03eeb9c67e0937', + 'freetype_revision': '7f9499044e3baa901de99251a007aa66e750b26c', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling freetype # and whatever else without interference from each other. diff --git a/content/browser/back_forward_cache_no_store_browsertest.cc b/content/browser/back_forward_cache_no_store_browsertest.cc index f179ae65fa1538..93239cd083bc33 100644 --- a/content/browser/back_forward_cache_no_store_browsertest.cc +++ b/content/browser/back_forward_cache_no_store_browsertest.cc @@ -11,6 +11,7 @@ #include "content/browser/renderer_host/render_frame_host_impl.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/public/test/browser_test.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h" #include "content/public/test/test_navigation_observer.h" #include "content/shell/browser/shell.h" @@ -115,13 +116,11 @@ class BackForwardCacheBrowserTestAllowCacheControlNoStore IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestAllowCacheControlNoStore, PagesWithCacheControlNoStoreEnterBfcacheAndEvicted) { net::test_server::ControllableHttpResponse response(embedded_test_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(embedded_test_server()->Start()); - GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); - GURL url_b(embedded_test_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", "/title2.html")); // 1) Load the document and specify no-store for the main resource. TestNavigationObserver observer(web_contents()); @@ -138,12 +137,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestAllowCacheControlNoStore, EXPECT_TRUE(rfh_a->IsInBackForwardCache()); // 3) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer2(web_contents()); - web_contents()->GetController().GoBack(); - response2.WaitForRequest(); - response2.Send(kResponseWithNoCache); - response2.Done(); - observer2.Wait(); + ASSERT_TRUE(HistoryGoBack(web_contents())); ExpectNotRestored({NotRestoredReason::kCacheControlNoStore}, {}, {}, {}, {}, FROM_HERE); @@ -169,14 +163,12 @@ IN_PROC_BROWSER_TEST_F( BackForwardCacheBrowserTestAllowCacheControlNoStore, MAYBE_PagesWithCacheControlNoStoreCookieModifiedThroughJavaScript) { net::test_server::ControllableHttpResponse response(embedded_test_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(embedded_test_server()->Start()); - GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title1.html")); - GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html")); + GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title2.html")); + GURL url_b(embedded_test_server()->GetURL("b.com", "/title3.html")); Shell* tab_to_be_bfcached = shell(); Shell* tab_to_modify_cookie = CreateBrowser(); @@ -207,12 +199,7 @@ IN_PROC_BROWSER_TEST_F( EXPECT_EQ("foo=baz", EvalJs(tab_to_modify_cookie, "document.cookie")); // 5) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer2(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response2.WaitForRequest(); - response2.Send(kResponseWithNoCache); - response2.Done(); - observer2.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); EXPECT_EQ("foo=baz", EvalJs(tab_to_be_bfcached, "document.cookie")); ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, @@ -309,14 +296,12 @@ IN_PROC_BROWSER_TEST_F( BackForwardCacheBrowserTestAllowCacheControlNoStore, MAYBE_PagesWithCacheControlNoStoreCookieModifiedThroughJavaScriptOnDifferentDomain) { net::test_server::ControllableHttpResponse response(embedded_test_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(embedded_test_server()->Start()); - GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title1.html")); - GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html")); + GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title2.html")); + GURL url_b(embedded_test_server()->GetURL("b.com", "/title3.html")); Shell* tab_to_be_bfcached = shell(); Shell* tab_to_modify_cookie = CreateBrowser(); @@ -331,28 +316,18 @@ IN_PROC_BROWSER_TEST_F( observer.Wait(); rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); - // 2) Set a normal cookie from JavaScript. - EXPECT_TRUE(ExecJs(tab_to_be_bfcached, "document.cookie='foo=bar'")); - EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); - - // 3) Navigate away. |rfh_a| should enter bfcache. + // 2) Navigate away. |rfh_a| should enter bfcache. EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); EXPECT_TRUE(rfh_a->IsInBackForwardCache()); - // 4) Navigate to b.com in |tab_to_modify_cookie| and modify cookie from + // 3) Navigate to b.com in |tab_to_modify_cookie| and modify cookie from // JavaScript. EXPECT_TRUE(NavigateToURL(tab_to_modify_cookie, url_b)); EXPECT_TRUE(ExecJs(tab_to_modify_cookie, "document.cookie='foo=baz'")); EXPECT_EQ("foo=baz", EvalJs(tab_to_modify_cookie, "document.cookie")); - // 5) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer2(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response2.WaitForRequest(); - response2.Send(kResponseWithNoCache); - response2.Done(); - observer2.Wait(); - EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); ExpectNotRestored({NotRestoredReason::kCacheControlNoStore}, {}, {}, {}, {}, FROM_HERE); @@ -498,6 +473,14 @@ const char kResponseWithNoCacheWithHTTPOnlyCookie2[] = "Cache-Control: no-store\r\n" "\r\n" "The server speaks HTTP!"; + +const char kResponseWithNoCacheWithRedirectionWithHTTPOnlyCookie[] = + "HTTP/1.1 302 Moved Temporarily\r\n" + "Location: /redirected\r\n" + "Content-Type: text/html; charset=utf-8\r\n" + "Set-Cookie: foo=baz; Secure; HttpOnly;\r\n" + "Cache-Control: no-store\r\n" + "\r\n"; } // namespace // TODO(crbug.com/1336055): It may be possible to re-enable this test now that @@ -521,14 +504,12 @@ IN_PROC_BROWSER_TEST_F( BackForwardCacheBrowserTestAllowCacheControlNoStore, MAYBE_PagesWithCacheControlNoStoreSetFromResponseHeader) { net::test_server::ControllableHttpResponse response(embedded_test_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(embedded_test_server()->Start()); - GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title1.html")); - GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html")); + GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title2.html")); + GURL url_b(embedded_test_server()->GetURL("b.com", "/title3.html")); Shell* tab_to_be_bfcached = shell(); Shell* tab_to_modify_cookie = CreateBrowser(); @@ -556,14 +537,7 @@ IN_PROC_BROWSER_TEST_F( EXPECT_EQ("foo=baz", EvalJs(tab_to_modify_cookie, "document.cookie")); // 4) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer2(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response2.WaitForRequest(); - // Send the response without the cookie header to avoid overwriting the - // cookie. - response2.Send(kResponseWithNoCache); - response2.Done(); - observer2.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); EXPECT_EQ("foo=baz", EvalJs(tab_to_be_bfcached, "document.cookie")); ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, {}, {}, {}, FROM_HERE); @@ -593,16 +567,14 @@ IN_PROC_BROWSER_TEST_F( // HTTPOnly cookie can be only set over HTTPS. CreateHttpsServer(); net::test_server::ControllableHttpResponse response(https_server(), - "/main_document"); + "/title1.html"); net::test_server::ControllableHttpResponse response2(https_server(), - "/main_document2"); - net::test_server::ControllableHttpResponse response3(https_server(), - "/main_document"); + "/title2.html"); ASSERT_TRUE(https_server()->Start()); - GURL url_a(https_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(https_server()->GetURL("a.com", "/main_document2")); - GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); + GURL url_b(https_server()->GetURL("b.com", "/title3.html")); Shell* tab_to_be_bfcached = shell(); Shell* tab_to_modify_cookie = CreateBrowser(); @@ -633,12 +605,7 @@ IN_PROC_BROWSER_TEST_F( observer2.Wait(); // 4) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer3(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response3.WaitForRequest(); - response3.Send(kResponseWithNoCacheWithHTTPOnlyCookie); - response3.Done(); - observer3.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); ExpectNotRestored( {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, {}, {}, FROM_HERE); @@ -659,18 +626,16 @@ IN_PROC_BROWSER_TEST_F( PagesWithCacheControlNoStoreHTTPOnlyCookieModifiedBackTwice) { CreateHttpsServer(); net::test_server::ControllableHttpResponse response(https_server(), - "/main_document"); + "/title1.html"); net::test_server::ControllableHttpResponse response2(https_server(), - "/main_document2"); + "/title2.html"); net::test_server::ControllableHttpResponse response3(https_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response4(https_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(https_server()->Start()); - GURL url_a(https_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(https_server()->GetURL("a.com", "/main_document2")); - GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); + GURL url_b(https_server()->GetURL("b.com", "/title3.html")); Shell* tab_to_be_bfcached = shell(); Shell* tab_to_modify_cookie = CreateBrowser(); @@ -725,12 +690,7 @@ IN_PROC_BROWSER_TEST_F( // 6) Navigate back to a.com. This time the cookie change has to be reset and // gets evicted with a different reason. - TestNavigationObserver observer4(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response4.WaitForRequest(); - response4.Send(kResponseWithNoCache); - response4.Done(); - observer4.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); ExpectNotRestored({NotRestoredReason::kCacheControlNoStore}, {}, {}, {}, {}, FROM_HERE); EXPECT_THAT(GetTreeResult()->GetDocumentResult(), @@ -1036,8 +996,6 @@ IN_PROC_BROWSER_TEST_F( PagesWithCacheControlNoStoreRestoreFromBackForwardCache) { net::test_server::ControllableHttpResponse response(embedded_test_server(), "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); ASSERT_TRUE(embedded_test_server()->Start()); GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); @@ -1099,13 +1057,11 @@ IN_PROC_BROWSER_TEST_F( BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, PagesWithCacheControlNoStoreEvictedIfCookieChange) { net::test_server::ControllableHttpResponse response(embedded_test_server(), - "/main_document"); - net::test_server::ControllableHttpResponse response2(embedded_test_server(), - "/main_document"); + "/title1.html"); ASSERT_TRUE(embedded_test_server()->Start()); - GURL url_a(embedded_test_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title1.html")); + GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(embedded_test_server()->GetURL("a.com", "/title2.html")); GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html")); Shell* tab_to_be_bfcached = shell(); @@ -1121,28 +1077,18 @@ IN_PROC_BROWSER_TEST_F( observer.Wait(); rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); - // 2) Set a normal cookie from JavaScript. - EXPECT_TRUE(ExecJs(tab_to_be_bfcached, "document.cookie='foo=bar'")); - EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); - - // 3) Navigate away. |rfh_a| should enter bfcache. + // 2) Navigate away. |rfh_a| should enter bfcache. EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); EXPECT_TRUE(rfh_a->IsInBackForwardCache()); - // 4) Navigate to a.com in |tab_to_modify_cookie| and modify cookie from + // 3) Navigate to a.com in |tab_to_modify_cookie| and modify cookie from // JavaScript. EXPECT_TRUE(NavigateToURL(tab_to_modify_cookie, url_a_2)); - EXPECT_EQ("foo=bar", EvalJs(tab_to_modify_cookie, "document.cookie")); EXPECT_TRUE(ExecJs(tab_to_modify_cookie, "document.cookie='foo=baz'")); EXPECT_EQ("foo=baz", EvalJs(tab_to_modify_cookie, "document.cookie")); - // 5) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer2(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response2.WaitForRequest(); - response2.Send(kResponseWithNoCache); - response2.Done(); - observer2.Wait(); + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); EXPECT_EQ("foo=baz", EvalJs(tab_to_be_bfcached, "document.cookie")); ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, @@ -1162,15 +1108,13 @@ IN_PROC_BROWSER_TEST_F( PagesWithCacheControlNoStoreEvictedWithBothCookieReasons) { CreateHttpsServer(); net::test_server::ControllableHttpResponse response(https_server(), - "/main_document"); + "/title1.html"); net::test_server::ControllableHttpResponse response2(https_server(), - "/main_document2"); - net::test_server::ControllableHttpResponse response3(https_server(), - "/main_document"); + "/title2.html"); ASSERT_TRUE(https_server()->Start()); - GURL url_a(https_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(https_server()->GetURL("a.com", "/main_document2")); + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); GURL url_b(https_server()->GetURL("b.com", "/title1.html")); Shell* tab_to_be_bfcached = shell(); @@ -1181,7 +1125,7 @@ IN_PROC_BROWSER_TEST_F( tab_to_be_bfcached->LoadURL(url_a); RenderFrameHostImplWrapper rfh_a(current_frame_host()); response.WaitForRequest(); - response.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response.Send(kResponseWithNoCache); response.Done(); observer.Wait(); rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); @@ -1202,12 +1146,7 @@ IN_PROC_BROWSER_TEST_F( observer2.Wait(); // 4) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer3(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response3.WaitForRequest(); - response3.Send(kResponseWithNoCacheWithHTTPOnlyCookie); - response3.Done(); - observer3.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); ExpectNotRestored( {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, {}, {}, FROM_HERE); @@ -1219,6 +1158,276 @@ IN_PROC_BROWSER_TEST_F( BlockListedFeatures())); } +// Test that a page with cache-control:no-store gets evicted if a cookie is set +// when the page is loaded. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithCookieSetInResponse) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document and specify no-store for the main resource, the + // response also sets a cookie. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCacheWithCookie); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 3) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(web_contents())); + ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, + {}, {}, {}, FROM_HERE); + EXPECT_THAT(GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreCookieModified), + BlockListedFeatures())); +} + +// Test that a page with `Cache-control: no-store` header gets evicted if some +// cookie is modified while the server receives the request but has not +// completed the response yet. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithCookieSetAfterRequestIsMade) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + Shell* tab_to_be_bfcached = shell(); + Shell* tab_to_modify_cookie = CreateBrowser(); + + // 1) Load the document and specify no-store for the main resource. + TestNavigationObserver observer(tab_to_be_bfcached->web_contents()); + tab_to_be_bfcached->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + + // 2) Before the response is sent, set a cookie from another tab. + EXPECT_TRUE(NavigateToURL(tab_to_modify_cookie, url_a_2)); + EXPECT_TRUE(ExecJs(tab_to_modify_cookie, "document.cookie='foo=bar'")); + + response.WaitForRequest(); + response.Send(kResponseWithNoCache); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); + ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, + {}, {}, {}, FROM_HERE); + EXPECT_THAT(GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreCookieModified), + BlockListedFeatures())); +} + +// Test that a page with cache-control:no-store gets evicted if some cookie is +// modified before navigating away. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithCookieSetBeforeNavigateAway) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document and specify no-store for the main resource. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCache); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Set a cookie from JavaScript. + EXPECT_TRUE(ExecJs(web_contents(), "document.cookie='foo=bar'")); + EXPECT_EQ("foo=bar", EvalJs(web_contents(), "document.cookie")); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(web_contents())); + ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, + {}, {}, {}, FROM_HERE); + EXPECT_THAT(GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreCookieModified), + BlockListedFeatures())); +} + +// Test that a page with cache-control:no-store gets evicted if some cookie is +// modified from another tab before navigating away. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithCookieSetFromAnotherTabBeforeNavigateAway) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + Shell* tab_to_be_bfcached = shell(); + Shell* tab_to_modify_cookie = CreateBrowser(); + + // 1) Load the document and specify no-store for the main resource. + TestNavigationObserver observer(tab_to_be_bfcached->web_contents()); + tab_to_be_bfcached->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCache); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Set a cookie from another tab. + EXPECT_TRUE(NavigateToURL(tab_to_modify_cookie, url_a_2)); + EXPECT_TRUE(ExecJs(tab_to_modify_cookie, "document.cookie='foo=bar'")); + EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); + ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, + {}, {}, {}, FROM_HERE); + EXPECT_THAT(GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreCookieModified), + BlockListedFeatures())); +} + +// Test that a page with cache-control:no-store gets restored if the cookie is +// modified by another tab before the navigation completes. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreRestoredIfCookieChangeIsMadeBeforeRedirection) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/main_document"); + net::test_server::ControllableHttpResponse response2(https_server(), + "/redirected"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/main_document")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document that will be redirected to another document. + // Both of the documents specify the cache-control:no-store, but only the + // document before redirection sets cookie. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCacheWithRedirectionWithHTTPOnlyCookie); + response.Done(); + response2.WaitForRequest(); + response2.Send(kResponseWithNoCache); + response2.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Navigate away. |rfh_a| should enter bfcache. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 3) Go back. |rfh_a| should be restored from BFCache. + ASSERT_TRUE(HistoryGoBack(web_contents())); + ExpectRestored(FROM_HERE); +} + +// Test that the cookie change information is retained after same document +// navigation. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreCacheControlNoStoreUnlessCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithCookieSetBeforeSameDocumentNavigation) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a2(https_server()->GetURL("a.com", "/title1.html#foo")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document and specify no-store for the main resource. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCache); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Set a cookie from JavaScript, and perform a same document navigation. + EXPECT_TRUE(ExecJs(web_contents(), "document.cookie='foo=bar'")); + EXPECT_EQ("foo=bar", EvalJs(web_contents(), "document.cookie")); + EXPECT_TRUE(ExecJs(shell(), JsReplace("location = $1", url_a2.spec()))); + EXPECT_TRUE(WaitForLoadStop(web_contents())); + EXPECT_EQ(url_a2, + web_contents()->GetPrimaryMainFrame()->GetLastCommittedURL()); + EXPECT_TRUE(rfh_a->IsActive()); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(web_contents())); + ExpectNotRestored({NotRestoredReason::kCacheControlNoStoreCookieModified}, {}, + {}, {}, {}, FROM_HERE); + EXPECT_THAT(GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreCookieModified), + BlockListedFeatures())); +} + class BackForwardCacheBrowserTestRestoreUnlessHTTPOnlyCookieChange : public BackForwardCacheBrowserTest { protected: @@ -1283,22 +1492,17 @@ IN_PROC_BROWSER_TEST_F( EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_a)); RenderFrameHostImplWrapper rfh_a(current_frame_host()); - // 2) Set a normal cookie from JavaScript. - EXPECT_TRUE(ExecJs(tab_to_be_bfcached, "document.cookie='foo=bar'")); - EXPECT_EQ("foo=bar", EvalJs(tab_to_be_bfcached, "document.cookie")); - - // 3) Navigate away. |rfh_a| should enter bfcache. + // 2) Navigate away. |rfh_a| should enter bfcache. EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); EXPECT_TRUE(rfh_a->IsInBackForwardCache()); - // 4) Navigate to a.com in |tab_to_modify_cookie| and modify cookie from + // 3) Navigate to a.com in |tab_to_modify_cookie| and modify cookie from // JavaScript. EXPECT_TRUE(NavigateToURL(tab_to_modify_cookie, url_a)); - EXPECT_EQ("foo=bar", EvalJs(tab_to_modify_cookie, "document.cookie")); EXPECT_TRUE(ExecJs(tab_to_modify_cookie, "document.cookie='foo=baz'")); EXPECT_EQ("foo=baz", EvalJs(tab_to_modify_cookie, "document.cookie")); - // 5) Go back. |rfh_a| should be restored from bfcache. + // 4) Go back. |rfh_a| should be restored from bfcache. ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); EXPECT_EQ("foo=baz", EvalJs(tab_to_be_bfcached, "document.cookie")); @@ -1312,15 +1516,13 @@ IN_PROC_BROWSER_TEST_F( PagesWithCacheControlNoStoreEvictedIfHTTPOnlyCookieChange) { CreateHttpsServer(); net::test_server::ControllableHttpResponse response(https_server(), - "/main_document"); + "/title1.html"); net::test_server::ControllableHttpResponse response2(https_server(), - "/main_document2"); - net::test_server::ControllableHttpResponse response3(https_server(), - "/main_document"); + "/title2.html"); ASSERT_TRUE(https_server()->Start()); - GURL url_a(https_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(https_server()->GetURL("a.com", "/main_document2")); + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); GURL url_b(https_server()->GetURL("b.com", "/title1.html")); Shell* tab_to_be_bfcached = shell(); @@ -1331,7 +1533,7 @@ IN_PROC_BROWSER_TEST_F( tab_to_be_bfcached->LoadURL(url_a); RenderFrameHostImplWrapper rfh_a(current_frame_host()); response.WaitForRequest(); - response.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response.Send(kResponseWithNoCache); response.Done(); observer.Wait(); rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); @@ -1350,12 +1552,7 @@ IN_PROC_BROWSER_TEST_F( observer2.Wait(); // 4) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer3(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response3.WaitForRequest(); - response3.Send(kResponseWithNoCacheWithHTTPOnlyCookie); - response3.Done(); - observer3.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); ExpectNotRestored( {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, {}, {}, FROM_HERE); @@ -1374,15 +1571,13 @@ IN_PROC_BROWSER_TEST_F( PagesWithCacheControlNoStoreEvictedIfJSAndHTTPOnlyCookieChange) { CreateHttpsServer(); net::test_server::ControllableHttpResponse response(https_server(), - "/main_document"); + "/title1.html"); net::test_server::ControllableHttpResponse response2(https_server(), - "/main_document2"); - net::test_server::ControllableHttpResponse response3(https_server(), - "/main_document"); + "/title2.html"); ASSERT_TRUE(https_server()->Start()); - GURL url_a(https_server()->GetURL("a.com", "/main_document")); - GURL url_a_2(https_server()->GetURL("a.com", "/main_document2")); + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); GURL url_b(https_server()->GetURL("b.com", "/title1.html")); Shell* tab_to_be_bfcached = shell(); @@ -1393,7 +1588,7 @@ IN_PROC_BROWSER_TEST_F( tab_to_be_bfcached->LoadURL(url_a); RenderFrameHostImplWrapper rfh_a(current_frame_host()); response.WaitForRequest(); - response.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response.Send(kResponseWithNoCache); response.Done(); observer.Wait(); rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); @@ -1414,12 +1609,155 @@ IN_PROC_BROWSER_TEST_F( observer2.Wait(); // 4) Go back. |rfh_a| should be evicted upon restoration. - TestNavigationObserver observer3(tab_to_be_bfcached->web_contents()); - tab_to_be_bfcached->web_contents()->GetController().GoBack(); - response3.WaitForRequest(); - response3.Send(kResponseWithNoCacheWithHTTPOnlyCookie); - response3.Done(); - observer3.Wait(); + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); + ExpectNotRestored( + {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, + {}, {}, FROM_HERE); + EXPECT_THAT( + GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified), + BlockListedFeatures())); +} + +// Test that a page with cache-control:no-store gets evicted if an HTTPOnly +// cookie is set when the page is loaded. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreUnlessHTTPOnlyCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithHTTPOnlyCookieSetInResponse) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document and specify no-store for the main resource, the + // response also sets a cookie. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 3) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(web_contents())); + ExpectNotRestored( + {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, + {}, {}, FROM_HERE); + EXPECT_THAT( + GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified), + BlockListedFeatures())); +} + +// Test that a page with cache-control:no-store gets evicted if some HTTPOnly +// cookie is modified from another tab before navigating away. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreUnlessHTTPOnlyCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithHTTPOnlyCookieSetFromAnotherTabBeforeNavigateAway) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + net::test_server::ControllableHttpResponse response2(https_server(), + "/title2.html"); + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a_2(https_server()->GetURL("a.com", "/title2.html")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + Shell* tab_to_be_bfcached = shell(); + Shell* tab_to_modify_cookie = CreateBrowser(); + + // 1) Load the document and specify no-store for the main resource. + TestNavigationObserver observer(tab_to_be_bfcached->web_contents()); + tab_to_be_bfcached->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCache); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Set an HTTPOnly cookie from another tab. + TestNavigationObserver observer2(tab_to_modify_cookie->web_contents()); + tab_to_modify_cookie->LoadURL(url_a_2); + response2.WaitForRequest(); + response2.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response2.Done(); + observer2.Wait(); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached, url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(tab_to_be_bfcached->web_contents())); + ExpectNotRestored( + {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, + {}, {}, FROM_HERE); + EXPECT_THAT( + GetTreeResult()->GetDocumentResult(), + MatchesDocumentResult( + NotRestoredReasons( + NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified), + BlockListedFeatures())); +} + +// Test that the cookie change information is retained after same document +// navigation. +IN_PROC_BROWSER_TEST_F( + BackForwardCacheBrowserTestRestoreUnlessHTTPOnlyCookieChange, + PagesWithCacheControlNoStoreNotBFCachedWithHTTPOnlyCookieSetBeforeSameDocumentNavigation) { + CreateHttpsServer(); + net::test_server::ControllableHttpResponse response(https_server(), + "/title1.html"); + + ASSERT_TRUE(https_server()->Start()); + + GURL url_a(https_server()->GetURL("a.com", "/title1.html")); + GURL url_a2(https_server()->GetURL("a.com", "/title1.html#foo")); + GURL url_b(https_server()->GetURL("b.com", "/title1.html")); + + // 1) Load the document and specify no-store for the main resource, the + // response also sets a cookie. + TestNavigationObserver observer(web_contents()); + shell()->LoadURL(url_a); + RenderFrameHostImplWrapper rfh_a(current_frame_host()); + response.WaitForRequest(); + response.Send(kResponseWithNoCacheWithHTTPOnlyCookie); + response.Done(); + observer.Wait(); + rfh_a->GetBackForwardCacheMetrics()->SetObserverForTesting(this); + + // 2) Perform a same document navigation. + EXPECT_TRUE(ExecJs(shell(), JsReplace("location = $1", url_a2.spec()))); + EXPECT_TRUE(WaitForLoadStop(web_contents())); + EXPECT_EQ(url_a2, + web_contents()->GetPrimaryMainFrame()->GetLastCommittedURL()); + EXPECT_TRUE(rfh_a->IsActive()); + + // 3) Navigate away. |rfh_a| should enter the bfcache since we only evict + // before restoration. + EXPECT_TRUE(NavigateToURL(shell(), url_b)); + EXPECT_TRUE(rfh_a->IsInBackForwardCache()); + + // 4) Go back. |rfh_a| should be evicted upon restoration. + ASSERT_TRUE(HistoryGoBack(web_contents())); ExpectNotRestored( {NotRestoredReason::kCacheControlNoStoreHTTPOnlyCookieModified}, {}, {}, {}, {}, FROM_HERE); diff --git a/content/browser/renderer_host/back_forward_cache_impl.cc b/content/browser/renderer_host/back_forward_cache_impl.cc index 882bed99b45e6f..5b647c421b23a2 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.cc +++ b/content/browser/renderer_host/back_forward_cache_impl.cc @@ -482,12 +482,7 @@ BackForwardCacheImpl::GetChannelAssociatedMessageHandlingPolicy() { } BackForwardCacheImpl::Entry::Entry(std::unique_ptr stored_page) - : stored_page_(std::move(stored_page)) { - if (BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore()) { - cookie_modified_ = {/*http_only_cookie_modified*/ false, - /*cookie_modified*/ false}; - } -} + : stored_page_(std::move(stored_page)) {} BackForwardCacheImpl::Entry::~Entry() = default; @@ -497,26 +492,6 @@ void BackForwardCacheImpl::Entry::WriteIntoTrace( dict.Add("render_frame_host", render_frame_host()); } -void BackForwardCacheImpl::Entry::StartMonitoringCookieChange() { - RenderFrameHostImpl* rfh = stored_page_->render_frame_host(); - StoragePartition* storage_partition = rfh->GetStoragePartition(); - auto* cookie_manager = storage_partition->GetCookieManagerForBrowserProcess(); - if (!cookie_listener_receiver_.is_bound()) { - // Listening only to the main document's URL, not the documents inside the - // subframes. - cookie_manager->AddCookieChangeListener( - rfh->GetLastCommittedURL(), absl::nullopt, - cookie_listener_receiver_.BindNewPipeAndPassRemote()); - } -} - -void BackForwardCacheImpl::Entry::OnCookieChange( - const net::CookieChangeInfo& change) { - DCHECK(cookie_modified_.has_value()); - cookie_modified_->http_only_cookie_modified = change.cookie.IsHttpOnly(); - cookie_modified_->cookie_modified = true; -} - void BackForwardCacheImpl::RenderProcessBackgroundedChanged( RenderProcessHostImpl* host) { EnforceCacheSizeLimit(); @@ -630,20 +605,13 @@ void BackForwardCacheImpl::UpdateCanStoreToIncludeCacheControlNoStore( return; } - auto* matching_entry = FindMatchingEntry(render_frame_host->GetPage()); - // |matching_entry| can be nullptr for tests because this can be called from - // |GetCurrentBackForwardCacheEligibility()|, at which point |rfh| may not - // have a matching entry yet. - if (!matching_entry) - return; - // Note that kCacheControlNoStoreHTTPOnlyCookieModified, // kCacheControlNoStoreCookieModified and kCacheControlNoStore are mutually // exclusive. - if (matching_entry->cookie_modified_->http_only_cookie_modified) { + if (render_frame_host->GetCookieChangeInfo().http_only_cookie_modified) { result.No(BackForwardCacheMetrics::NotRestoredReason:: kCacheControlNoStoreHTTPOnlyCookieModified); - } else if (matching_entry->cookie_modified_->cookie_modified) { + } else if (render_frame_host->GetCookieChangeInfo().cookie_modified) { // JavaScript cookies are modified but not HTTP cookies. Only restore based // on the experiment level. if (GetCacheControlNoStoreLevel() <= @@ -1089,14 +1057,6 @@ void BackForwardCacheImpl::StoreEntry( #endif entry->render_frame_host()->DidEnterBackForwardCache(); - if (AllowStoringPagesWithCacheControlNoStore()) { - if (entry->render_frame_host()->GetBackForwardCacheDisablingFeatures().Has( - WebSchedulerTrackedFeature::kMainResourceHasCacheControlNoStore)) { - // Start monitoring the cookie change only when cache-control:no-store - // header is present. - entry->StartMonitoringCookieChange(); - } - } entry->SetStoredPageDelegate(this); entries_.push_front(std::move(entry)); AddProcessesForEntry(*entries_.front()); @@ -1417,6 +1377,7 @@ void BackForwardCacheImpl::WillCommitNavigationToCachedEntry( } } +// static bool BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore() { return GetCacheControlNoStoreLevel() > CacheControlNoStoreExperimentLevel::kDoNotStore; diff --git a/content/browser/renderer_host/back_forward_cache_impl.h b/content/browser/renderer_host/back_forward_cache_impl.h index 442ca10d956dc1..a3f2b7cca69c18 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.h +++ b/content/browser/renderer_host/back_forward_cache_impl.h @@ -28,7 +28,6 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host_observer.h" #include "content/public/browser/site_instance.h" -#include "content/public/browser/storage_partition.h" #include "content/public/common/content_features.h" #include "net/cookies/canonical_cookie.h" #include "services/network/public/mojom/cookie_manager.mojom.h" @@ -139,16 +138,13 @@ class CONTENT_EXPORT BackForwardCacheImpl GetChannelAssociatedMessageHandlingPolicy(); // BackForwardCache entry, consisting of the page and associated metadata. - class Entry : public ::network::mojom::CookieChangeListener { + class Entry { public: explicit Entry(std::unique_ptr stored_page); - ~Entry() override; + ~Entry(); void WriteIntoTrace(perfetto::TracedValue context); - // Starts monitoring the cookie change in this entry. - void StartMonitoringCookieChange(); - // Indicates whether or not all the |render_view_hosts| in this entry have // received the acknowledgement from renderer that it finished running // handlers. @@ -184,23 +180,6 @@ class CONTENT_EXPORT BackForwardCacheImpl private: friend class BackForwardCacheImpl; - // ::network::mojom::CookieChangeListener - void OnCookieChange(const net::CookieChangeInfo& change) override; - - mojo::Receiver<::network::mojom::CookieChangeListener> - cookie_listener_receiver_{this}; - - struct CookieModified { - // Indicates whether or not cookie on the bfcache entry has been modified - // while the entry is in bfcache. - bool cookie_modified = false; - // Indicates whether or not HTTPOnly cookie on the bfcache entry - // has been modified while the entry is in bfcache. - bool http_only_cookie_modified = false; - }; - // Only populated when |AllowStoringPagesWithCacheControlNoStore()| is true. - absl::optional cookie_modified_; - std::unique_ptr stored_page_; }; @@ -393,6 +372,10 @@ class CONTENT_EXPORT BackForwardCacheImpl RenderFrameHostImpl& rfh, BackForwardCacheCanStoreDocumentResult& eviction_reason); + // Returns true if the flag is on for pages with cache-control:no-store to + // get restored from back/forward cache unless cookies change. + static bool AllowStoringPagesWithCacheControlNoStore(); + private: // Destroys all evicted frames in the BackForwardCache. void DestroyEvictedFrames(); @@ -456,10 +439,6 @@ class CONTENT_EXPORT BackForwardCacheImpl void AddProcessesForEntry(Entry& entry); void RemoveProcessesForEntry(Entry& entry); - // Returns true if the flag is on for pages with cache-control:no-store to - // get restored from back/forward cache unless cookies change. - static bool AllowStoringPagesWithCacheControlNoStore(); - static BlockListedFeatures GetAllowedFeatures( RequestedFeatures requested_features); static BlockListedFeatures GetDisallowedFeatures( diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index a4dc9781f9499f..e2a93031d84bf0 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -4,6 +4,7 @@ #include "content/browser/renderer_host/navigation_request.h" +#include #include #include #include @@ -62,6 +63,7 @@ #include "content/browser/preloading/prerender/prerender_host_registry.h" #include "content/browser/process_lock.h" #include "content/browser/reduce_accept_language/reduce_accept_language_utils.h" +#include "content/browser/renderer_host/back_forward_cache_impl.h" #include "content/browser/renderer_host/cookie_utils.h" #include "content/browser/renderer_host/debug_urls.h" #include "content/browser/renderer_host/frame_tree.h" @@ -2511,6 +2513,23 @@ void NavigationRequest::SetWaitingForRendererResponse() { SetState(WAITING_FOR_RENDERER_RESPONSE); } +bool NavigationRequest::ShouldAddCookieChangeListener() { + // The `CookieChangeListener` will only be set up if all of these are true: + // (1) the navigation's protocol is HTTP(s). + // (2) we allow a document with `Cache-control: no-store` header to + // enter BFCache. + // (3) the navigation is neither a same-document navigation nor a page + // activation, since in these cases, an existing `RenderFrameHost` will be + // used, and it would already have an existing listener, so we should skip the + // initialization. + // (4) the navigation is a primary main frame navigation, as the cookie + // change information will only be used in the inactive document control + // logic. + return BackForwardCacheImpl::AllowStoringPagesWithCacheControlNoStore() && + !IsPageActivation() && !IsSameDocument() && IsInPrimaryMainFrame() && + common_params_->url.SchemeIsHTTPOrHTTPS(); +} + void NavigationRequest::StartNavigation() { DCHECK(frame_tree_node_->navigation_request() == this || is_synchronous_renderer_commit_); @@ -2529,6 +2548,24 @@ void NavigationRequest::StartNavigation() { frame_tree_node->current_frame_host()->GetSiteInstance(); site_info_ = GetSiteInfoForCommonParamsURL(); + // It's important to start listening to the cookie changes before the network + // request of the navigation begins in order to ensure the listener won't miss + // any cookie changes that happen after the network request is sent that + // potentially modify some cookie values that are used in this request. + // The information of cookie modification will be used to determine if the + // document that this navigation will load should be eligible for BFCache. + // The listener eventually will be transferred over to the committed + // `RenderFrameHost`. + if (ShouldAddCookieChangeListener()) { + // The listener should receive the change events of the cookies from the + // the domain of the main-frame navigation url. + // If the navigation gets redirected, it will be reset with the new URL when + // `NavigationRequest::OnRequestRedirected()` is called. + cookie_change_listener_ = + std::make_unique( + GetStoragePartitionWithCurrentSiteInfo(), common_params_->url); + } + // Compute the redirect chain. // TODO(clamy): Try to simplify this and have the redirects be part of // CommonNavigationParams. @@ -3033,6 +3070,16 @@ void NavigationRequest::OnRequestRedirected( commit_params_->redirect_infos.back().new_referrer = common_params_->referrer->url.spec(); + // When the redirection happens, the cookie_change_listener_ should be + // re-initialized if needed. + if (ShouldAddCookieChangeListener()) { + cookie_change_listener_ = + std::make_unique( + GetStoragePartitionWithCurrentSiteInfo(), common_params_->url); + } else { + cookie_change_listener_.reset(); + } + // Check Content Security Policy before the NavigationThrottles run. This // gives CSP a chance to modify requests that NavigationThrottles would // otherwise block. @@ -4443,12 +4490,7 @@ void NavigationRequest::OnStartChecksComplete( return; } - // |site_info_|'s StoragePartitionConfig should refer to the correct - // StoragePartition for this navigation. - BrowserContext* browser_context = - frame_tree_node_->navigator().controller().GetBrowserContext(); - StoragePartition* partition = browser_context->GetStoragePartition( - site_info_.storage_partition_config()); + StoragePartition* partition = GetStoragePartitionWithCurrentSiteInfo(); DCHECK(partition); // |loader_| should not exist if the service worker handle @@ -4600,6 +4642,9 @@ void NavigationRequest::OnStartChecksComplete( // Reset the compositor lock before starting the loader. compositor_lock_.reset(); + BrowserContext* browser_context = + frame_tree_node_->navigator().controller().GetBrowserContext(); + loader_ = NavigationURLLoader::Create( browser_context, partition, std::make_unique( @@ -8783,4 +8828,13 @@ void NavigationRequest::CheckSoftNavigationHeuristicsInvariants() { DCHECK(!frame_tree_node()->IsFencedFrameRoot()); } +StoragePartition* NavigationRequest::GetStoragePartitionWithCurrentSiteInfo() { + // `site_info_`'s StoragePartitionConfig should refer to the correct + // `StoragePartition` for this navigation. + return frame_tree_node_->navigator() + .controller() + .GetBrowserContext() + ->GetStoragePartition(site_info_.storage_partition_config()); +} + } // namespace content diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h index 11b8fca857267a..3d5357832b4878 100644 --- a/content/browser/renderer_host/navigation_request.h +++ b/content/browser/renderer_host/navigation_request.h @@ -1124,6 +1124,11 @@ class CONTENT_EXPORT NavigationRequest void AddDeferredSubframeNavigationThrottle( base::WeakPtr throttle); + std::unique_ptr + TakeCookieChangeListener() { + return std::move(cookie_change_listener_); + } + private: friend class NavigationRequestTest; @@ -1767,6 +1772,13 @@ class CONTENT_EXPORT NavigationRequest // IsInMainFrame(). void UnblockPendingSubframeNavigationRequestsIfNeeded(); + // Returns if we should add/reset the `CookieChangeListener` for the current + // navigation. + bool ShouldAddCookieChangeListener(); + + // Returns the `StoragePartition` based on the config from the `site_info_`. + StoragePartition* GetStoragePartitionWithCurrentSiteInfo(); + // Never null. The pointee node owns this navigation request instance. FrameTreeNode* const frame_tree_node_; @@ -2407,6 +2419,16 @@ class CONTENT_EXPORT NavigationRequest absl::optional main_frame_same_document_navigation_token_; + // The listener that receives cookie change events and maintains cookie change + // information for the domain of the URL that this `NavigationRequest` is + // navigating to. The listener will observe all the cookie changes starting + // from the navigation/redirection, and it will be moved to the + // `RenderFrameHostImpl` when the navigation is committed and continues + // observing until the destruction of the document. + // See `RenderFrameHostImpl::CookieChangeListener`. + std::unique_ptr + cookie_change_listener_; + base::WeakPtrFactory weak_factory_{this}; }; diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc index 978ffd44bf948e..f782b499524679 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -12180,6 +12180,11 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal( } RecordDocumentCreatedUkmEvent(params->origin, document_ukm_source_id, ukm_recorder); + + // We only replace the `CookieChangeListener` with the one initialized by + // the navigation request when navigating to a new document. Otherwise, the + // existing `CookieChangeListener` will be reused. + cookie_change_listener_ = navigation_request->TakeCookieChangeListener(); } if (!is_same_document_navigation) { @@ -14700,4 +14705,31 @@ net::CookieSettingOverrides RenderFrameHostImpl::GetCookieSettingOverrides() { return subresource_loader_factories_config.cookie_setting_overrides(); } +RenderFrameHostImpl::CookieChangeListener::CookieChangeListener( + StoragePartition* storage_partition, + GURL& url) { + DCHECK(storage_partition); + auto* cookie_manager = storage_partition->GetCookieManagerForBrowserProcess(); + cookie_manager->AddCookieChangeListener( + url, absl::nullopt, + cookie_change_listener_receiver_.BindNewPipeAndPassRemote()); +} + +RenderFrameHostImpl::CookieChangeListener::~CookieChangeListener() = default; + +void RenderFrameHostImpl::CookieChangeListener::OnCookieChange( + const net::CookieChangeInfo& change) { + // TODO (https://crbug.com/1399741): After adding the invalidation signals + // API, we could mark the page as ineligible for BFCache as soon as the cookie + // change event is received. + cookie_change_info_.http_only_cookie_modified |= change.cookie.IsHttpOnly(); + cookie_change_info_.cookie_modified = true; +} + +RenderFrameHostImpl::CookieChangeListener::CookieChangeInfo +RenderFrameHostImpl::GetCookieChangeInfo() { + return cookie_change_listener_ ? cookie_change_listener_->cookie_change_info() + : CookieChangeListener::CookieChangeInfo{}; +} + } // namespace content diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h index 555225f08d9de9..baecec586a9d9c 100644 --- a/content/browser/renderer_host/render_frame_host_impl.h +++ b/content/browser/renderer_host/render_frame_host_impl.h @@ -79,6 +79,7 @@ #include "content/public/browser/global_routing_id.h" #include "content/public/browser/javascript_dialog_manager.h" #include "content/public/browser/render_frame_host.h" +#include "content/public/browser/storage_partition.h" #include "content/public/browser/web_ui.h" #include "content/public/common/content_client.h" #include "content/public/common/extra_mojo_js_features.mojom-forward.h" @@ -1371,6 +1372,38 @@ class CONTENT_EXPORT RenderFrameHostImpl // log_level == kError messages. void SetWantErrorMessageStackTrace(); + // Listens to the change events of the cookies associated with the domain of + // the specified URL during initialization. It also contains the information + // of whether any cookie/HTTPOnly cookie had been changed before, which can be + // used to determine if a document with Cache-control: no-store header set is + // eligible for BFCache. + class CookieChangeListener : public network::mojom::CookieChangeListener { + public: + struct CookieChangeInfo { + // Indicates whether any cookie modification has been observed. + bool cookie_modified = false; + // Indicates whether any HTTPOnly cookie modification has been observed. + bool http_only_cookie_modified = false; + }; + + CookieChangeListener(StoragePartition* storage_partition, GURL& url); + ~CookieChangeListener() override; + CookieChangeListener(const CookieChangeListener&) = delete; + CookieChangeListener& operator=(const CookieChangeListener&) = delete; + + // Returns a copy of the `cookie_change_info_`. + CookieChangeInfo cookie_change_info() { return cookie_change_info_; } + + private: + // network::mojom::CookieChangeListener + void OnCookieChange(const net::CookieChangeInfo& change) override; + + mojo::Receiver + cookie_change_listener_receiver_{this}; + + CookieChangeInfo cookie_change_info_; + }; + // Indicates that a navigation is ready to commit and can be // handled by this RenderFrame. // |subresource_loader_params| is used in network service land to pass @@ -2790,6 +2823,10 @@ class CONTENT_EXPORT RenderFrameHostImpl // frame. Can only be called on a frame with a committed navigation. net::CookieSettingOverrides GetCookieSettingOverrides(); + // Retrieves the information about the cookie changes that are observed on the + // last committed document. + CookieChangeListener::CookieChangeInfo GetCookieChangeInfo(); + protected: friend class RenderFrameHostFactory; @@ -4839,6 +4876,13 @@ class CONTENT_EXPORT RenderFrameHostImpl mojo::Receiver broker_receiver_{ &broker_}; + // The listener should be moved from the `NavigationRequest` when committing + // a navigation in this `RenderFrameHostImpl`. It will be owned by the + // `RenderFrameHostImpl` and continue receiving the cookie change events until + // the destruction of the document. See the comments of the + // `cookie_change_listener_` in `NavigationRequest`. + std::unique_ptr cookie_change_listener_; + // WeakPtrFactories are the last members, to ensure they are destroyed before // all other fields of `this`. base::WeakPtrFactory weak_ptr_factory_{this}; diff --git a/third_party/freetype/README.chromium b/third_party/freetype/README.chromium index 80b14f1da9741a..b7b4d84136b91c 100644 --- a/third_party/freetype/README.chromium +++ b/third_party/freetype/README.chromium @@ -1,7 +1,7 @@ Name: FreeType URL: http://www.freetype.org/ -Version: VER-2-13-0-7-g713580f41 -Revision: 713580f41dcc2054d7fa56265f03eeb9c67e0937 +Version: VER-2-13-0-12-g7f9499044 +Revision: 7f9499044e3baa901de99251a007aa66e750b26c CPEPrefix: cpe:/a:freetype:freetype:2.12.1 License: Custom license "inspired by the BSD, Artistic, and IJG (Independent JPEG Group) licenses"