bfcache: Disable bfcache for error pages.

This includes adding a new member to RenderFrameHostImpl to store the
latest http status code.

Bug: 990812
Change-Id: I83e61b328df4e37194cc3e1b17952986dd48cf1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742152
Commit-Queue: Mohamed Abdelhalim <zetamoo@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Lowell Manners <lowell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686402}

content/browser/back_forward_cache_browsertest.cc

@@ -12,7 +12,9 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/navigation_handle_observer.h"
 #include "content/public/test/test_utils.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "net/dns/mock_host_resolver.h"
@@ -818,6 +820,58 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfWebGL) {
   // so it shouldn't have been cached.
 }
 
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, DoesNotCacheIfHttpError) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL error_url(embedded_test_server()->GetURL("a.com", "/page404.html"));
+  GURL url(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // Navigate to an error page.
+  EXPECT_TRUE(NavigateToURL(shell(), error_url));
+  EXPECT_EQ(net::HTTP_NOT_FOUND, current_frame_host()->last_http_status_code());
+  RenderFrameDeletedObserver delete_rfh_a(current_frame_host());
+
+  // Navigate away.
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  // The page did not return 200 (OK), so it shouldn't have been cached.
+  delete_rfh_a.WaitUntilDeleted();
+}
+
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+                       DoesNotCacheIfPageUnreachable) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL error_url(embedded_test_server()->GetURL("a.com", "/empty.html"));
+  GURL url(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  std::unique_ptr<URLLoaderInterceptor> url_interceptor =
+      URLLoaderInterceptor::SetupRequestFailForURL(error_url,
+                                                   net::ERR_DNS_TIMED_OUT);
+
+  // Start with a successful navigation to a document.
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  EXPECT_EQ(net::HTTP_OK, current_frame_host()->last_http_status_code());
+
+  // Navigate to an error page.
+  NavigationHandleObserver observer(shell()->web_contents(), error_url);
+  EXPECT_FALSE(NavigateToURL(shell(), error_url));
+  EXPECT_TRUE(observer.is_error());
+  EXPECT_EQ(net::ERR_DNS_TIMED_OUT, observer.net_error_code());
+  EXPECT_EQ(
+      GURL(kUnreachableWebDataURL),
+      shell()->web_contents()->GetMainFrame()->GetSiteInstance()->GetSiteURL());
+  EXPECT_EQ(net::OK, current_frame_host()->last_http_status_code());
+
+  RenderFrameDeletedObserver delete_rfh_a(current_frame_host());
+
+  // Navigate away.
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  // The page had a networking error, so it shouldn't have been cached.
+  delete_rfh_a.WaitUntilDeleted();
+}
+
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                        DisableBackforwardCacheForTesting) {
   ASSERT_TRUE(embedded_test_server()->Start());

content/browser/frame_host/back_forward_cache.cc

@@ -11,6 +11,7 @@
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/common/page_messages.h"
 #include "content/public/common/navigation_policy.h"
+#include "net/http/http_status_code.h"
 #include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"
 
 namespace content {
@@ -123,6 +124,11 @@ bool BackForwardCache::CanStoreDocument(RenderFrameHostImpl* rfh) {
   if (rfh->GetSiteInstance()->GetRelatedActiveContentsCount() != 0)
     return false;
 
+  // Only store documents that have successful http status code.
+  // Note that for non-http navigations, |last_http_status_code| is equal to 0.
+  if (rfh->last_http_status_code() != net::HTTP_OK)
+    return false;
+
   return true;
 }
 

content/browser/frame_host/render_frame_host_impl.cc

@@ -6704,6 +6704,7 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal(
   navigation_request->set_has_user_gesture(validated_params->gesture ==
                                            NavigationGestureUser);
 
+  last_http_status_code_ = validated_params->http_status_code;
   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

@@ -440,6 +440,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // cases, use GetLastCommittedURL instead.
   const GURL& last_successful_url() { return last_successful_url_; }
 
+  // Return the http status code of the last committed navigation.
+  int last_http_status_code() { return last_http_status_code_; }
+
   // Computes site_for_cookies to be used when navigating this frame to
   // |destination|.
   GURL ComputeSiteForCookiesForNavigation(const GURL& destination) const;
@@ -1821,6 +1824,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // See https://crbug.com/588314.
   GURL last_successful_url_;
 
+  // The http status code of the last committed navigation.
+  int last_http_status_code_ = 0;
+
   std::map<uint64_t, VisualStateCallback> visual_state_callbacks_;
 
   // Local root subframes directly own their RenderWidgetHost.