[bfcache] Only cache main frame if last commit was an HTTP GET.

We only want to add pages to the cache if the initial fetch is expected
to have had no side effects on the server (HTTP GET).

Before this CL, we could cache pages that were the result of an
HTTP POST.

There are two main cases we need to handle:
1) The last commit's http method != GET (e.g. POST):
We should not cache.

2) The last commit's http method == GET:
We should cache the page, if it is otherwise eligible for caching.

An HTTP POST may result in a redirect, or chain of redirects,
however for caching purposes we only care about the last commit
in the chain of redirects (if the last commit was a successful GET, we
can cache the page. Otherwise, we cannot).

TBR=clamy@chromium.org

Change-Id: I475eb73bf5ac50f500d760df5a947b2a8e1f5e4a
Bug: 1014901
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864681Reviewed-by: Lowell Manners <lowell@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Lowell Manners <lowell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706874}

content/browser/back_forward_cache_browsertest.cc

@@ -2938,4 +2938,35 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, RestoreWhilePendingCommit) {
   EXPECT_EQ(rfh1, current_frame_host());
 }
 
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+                       DoesNotCacheCrossSiteHttpPost) {
+  SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Note we do a cross-site post because same-site navigations of any kind
+  // aren't cached currently.
+  GURL form_url(embedded_test_server()->GetURL(
+      "a.com", "/form_that_posts_cross_site.html"));
+  GURL redirect_target_url(embedded_test_server()->GetURL("x.com", "/echoall"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // Navigate to the page with form that posts via 307 redirection to
+  // |redirect_target_url| (cross-site from |form_url|).
+  EXPECT_TRUE(NavigateToURL(shell(), form_url));
+
+  // Submit the form.
+  TestNavigationObserver form_post_observer(shell()->web_contents(), 1);
+  EXPECT_TRUE(ExecJs(shell(), "document.getElementById('text-form').submit()"));
+  form_post_observer.Wait();
+
+  // Verify that we arrived at the expected, redirected location.
+  EXPECT_EQ(redirect_target_url,
+            shell()->web_contents()->GetLastCommittedURL());
+  RenderFrameDeletedObserver delete_observer_rfh(current_frame_host());
+
+  // Navigate away. |redirect_target_url|'s page should not be cached.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+  delete_observer_rfh.WaitUntilDeleted();
+}
+
 }  // namespace content

content/browser/frame_host/back_forward_cache_impl.cc

@@ -17,6 +17,7 @@
 #include "content/common/page_messages.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/navigation_policy.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_status_code.h"
 #include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"
 
@@ -190,6 +191,8 @@ std::string BackForwardCacheImpl::CanStoreDocumentResult::ToString() {
       return "No: BackForwardCache::DisableForRenderFrameHost() was called";
     case Reason::kDomainNotAllowed:
       return "No: This domain is not allowed to be stored in BackForwardCache";
+    case Reason::kHTTPMethodNotGET:
+      return "No: HTTP method is not GET";
   }
 }
 
@@ -293,6 +296,12 @@ BackForwardCacheImpl::CanStoreDocument(RenderFrameHostImpl* rfh) {
         BackForwardCacheMetrics::CanNotStoreDocumentReason::kHTTPStatusNotOK);
   }
 
+  // Only store documents that were fetched via HTTP GET method.
+  if (rfh->last_http_method() != net::HttpRequestHeaders::kGetMethod) {
+    return CanStoreDocumentResult::No(
+        BackForwardCacheMetrics::CanNotStoreDocumentReason::kHTTPMethodNotGET);
+  }
+
   // Do not store main document with non HTTP/HTTPS URL scheme. In particular,
   // this excludes the new tab page.
   if (!rfh->GetLastCommittedURL().SchemeIsHTTPOrHTTPS()) {

content/browser/frame_host/back_forward_cache_metrics.h

@@ -42,7 +42,8 @@ class BackForwardCacheMetrics
     kWasGrantedMediaAccess,
     kBlocklistedFeatures,
     kDisableForRenderFrameHostCalled,
-    kDomainNotAllowed
+    kDomainNotAllowed,
+    kHTTPMethodNotGET
   };
 
   // Please keep in sync with BackForwardCacheHistoryNavigationOutcome in

content/browser/frame_host/render_frame_host_impl.cc

@@ -7129,6 +7129,7 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal(
                                            NavigationGestureUser);
 
   last_http_status_code_ = validated_params->http_status_code;
+  last_http_method_ = validated_params->method;
   UpdateSiteURL(validated_params->url, validated_params->url_is_unreachable);
 
   // Set the state whether this navigation is to an MHTML document, since there

content/browser/frame_host/render_frame_host_impl.h

@@ -469,6 +469,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // cases, use GetLastCommittedURL instead.
   const GURL& last_successful_url() { return last_successful_url_; }
 
+  // Return the http method of the last committed navigation.
+  const std::string& last_http_method() { return last_http_method_; }
+
   // Return the http status code of the last committed navigation.
   int last_http_status_code() { return last_http_status_code_; }
 
@@ -1995,6 +1998,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // See https://crbug.com/588314.
   GURL last_successful_url_;
 
+  // The http method of the last committed navigation.
+  std::string last_http_method_;
+
   // The http status code of the last committed navigation.
   int last_http_status_code_ = 0;
 

testing/buildbot/filters/bfcache.content_browsertests.filter

@@ -101,7 +101,3 @@
 -RenderViewHostTest.IsFocusedElementEditable
 -TouchInputBrowserTest.*
 -TouchpadPinchBrowserTest.*
-
-# Started failing around when https://crrev.com/c/1819251 was submitted.
-# https://crbug.com/1013550
--SessionHistoryTest.GoBackToCrossSitePostWithRedirect