Reland "[WebLayer] Use NetErrorAutoReloader"

This is a reland of 596311e0

Original change's description:
> [WebLayer] Use NetErrorAutoReloader
>
> Navigations which fail with network errors may attempt to
> auto-reload in some cases, in order to more seamlessly recover
> from transient failures.
>
> This behavior was previously implemented renderer-side and copied
> verbatim from Chrome. The Chrome implementation has been moved
> browser-side (see bug for why) and also made into a reusable
> component.
>
> This changes WebLayer to use the same component implementation
> and deletes all the corresponding renderer logic.
>
> Fixed: 1098578
> Change-Id: I10c337abcae36f64be47f31b89e36c7393989077
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311371
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#792028}

Tbr: mmenke@chromium.org
Change-Id: I4dd0aaef47650d14c52b6b2ddcf35cee27d2ef35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324934
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Auto-Submit: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#792826}

weblayer/BUILD.gn

@@ -337,6 +337,7 @@ source_set("weblayer_lib_base") {
     "//components/embedder_support",
     "//components/embedder_support/origin_trials",
     "//components/error_page/common",
+    "//components/error_page/content/browser",
     "//components/find_in_page",
     "//components/infobars/core",
     "//components/js_injection/browser",

weblayer/browser/DEPS

@@ -18,6 +18,7 @@ include_rules = [
   "+components/crash/core/common",
   "+components/download/public/common",
   "+components/embedder_support",
+  "+components/error_page/content/browser",
   "+components/find_in_page",
   "+components/infobars/android",
   "+components/infobars/content",

weblayer/browser/content_browser_client_impl.cc

@@ -20,6 +20,7 @@
 #include "components/blocked_content/popup_blocker.h"
 #include "components/captive_portal/core/buildflags.h"
 #include "components/embedder_support/switches.h"
+#include "components/error_page/content/browser/net_error_auto_reloader.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/page_load_metrics/browser/metrics_navigation_throttle.h"
 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
@@ -558,6 +559,11 @@ ContentBrowserClientImpl::CreateThrottlesForNavigation(
   std::vector<std::unique_ptr<content::NavigationThrottle>> throttles;
 
   if (handle->IsInMainFrame()) {
+    auto auto_reload_throttle =
+        error_page::NetErrorAutoReloader::MaybeCreateThrottleFor(handle);
+    if (auto_reload_throttle)
+      throttles.push_back(std::move(auto_reload_throttle));
+
     // MetricsNavigationThrottle requires that it runs before
     // NavigationThrottles that may delay or cancel navigations, so only
     // NavigationThrottles that don't delay or cancel navigations (e.g.

weblayer/browser/errorpage_browsertest.cc

@@ -7,9 +7,10 @@
 #include "base/macros.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
+#include "components/error_page/content/browser/net_error_auto_reloader.h"
 #include "content/public/test/url_loader_interceptor.h"
-#include "net/base/mock_network_change_notifier.h"
 #include "net/test/url_request/url_request_failed_job.h"
+#include "weblayer/browser/tab_impl.h"
 #include "weblayer/common/features.h"
 #include "weblayer/shell/browser/shell.h"
 #include "weblayer/test/weblayer_browser_test_utils.h"
@@ -58,11 +59,15 @@ class ErrorPageReloadBrowserTest : public ErrorPageBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, ReloadOnNetworkChanged) {
-  // Make sure the renderer thinks it's online, since that is a necessary
-  // condition for the reload.
-  net::test::ScopedMockNetworkChangeNotifier mock_network_change_notifier;
-  mock_network_change_notifier.mock_network_change_notifier()
-      ->SetConnectionType(net::NetworkChangeNotifier::CONNECTION_4G);
+  // Ensure that the NetErrorAutoReloader believes it's online, otherwise it
+  // does not attempt auto-reload on error pages.
+  content::WebContents* web_contents =
+      static_cast<TabImpl*>(shell()->tab())->web_contents();
+  error_page::NetErrorAutoReloader::CreateForWebContents(web_contents);
+  auto* reloader =
+      error_page::NetErrorAutoReloader::FromWebContents(web_contents);
+  reloader->DisableConnectionChangeObservationForTesting();
+  reloader->OnConnectionChanged(network::mojom::ConnectionType::CONNECTION_4G);
 
   ASSERT_TRUE(embedded_test_server()->Start());
   GURL url = embedded_test_server()->GetURL("/error_page");

weblayer/renderer/content_renderer_client_impl.cc

@@ -124,16 +124,6 @@ bool ContentRendererClientImpl::HasErrorPage(int http_status_code) {
   return http_status_code >= 400;
 }
 
-bool ContentRendererClientImpl::ShouldSuppressErrorPage(
-    content::RenderFrame* render_frame,
-    const GURL& url,
-    int error_code) {
-  auto* error_page_helper = ErrorPageHelper::GetForFrame(render_frame);
-  if (error_page_helper)
-    return error_page_helper->ShouldSuppressErrorPage(error_code);
-  return false;
-}
-
 void ContentRendererClientImpl::PrepareErrorPage(
     content::RenderFrame* render_frame,
     const blink::WebURLError& error,
@@ -141,11 +131,9 @@ void ContentRendererClientImpl::PrepareErrorPage(
     std::string* error_html) {
   auto* error_page_helper = ErrorPageHelper::GetForFrame(render_frame);
   if (error_page_helper) {
-    error_page_helper->PrepareErrorPage(
-        error_page::Error::NetError(error.url(), error.reason(),
-                                    error.resolve_error_info(),
-                                    error.has_copy_in_cache()),
-        http_method == "POST");
+    error_page_helper->PrepareErrorPage(error_page::Error::NetError(
+        error.url(), error.reason(), error.resolve_error_info(),
+        error.has_copy_in_cache()));
   }
 
 #if defined(OS_ANDROID)

weblayer/renderer/content_renderer_client_impl.h

@@ -30,9 +30,6 @@ class ContentRendererClientImpl : public content::ContentRendererClient {
   SkBitmap* GetSadPluginBitmap() override;
   SkBitmap* GetSadWebViewBitmap() override;
   bool HasErrorPage(int http_status_code) override;
-  bool ShouldSuppressErrorPage(content::RenderFrame* render_frame,
-                               const GURL& url,
-                               int error_code) override;
   void PrepareErrorPage(content::RenderFrame* render_frame,
                         const blink::WebURLError& error,
                         const std::string& http_method,

weblayer/renderer/error_page_helper.cc

@@ -15,61 +15,12 @@
 #include "weblayer/common/features.h"
 
 namespace weblayer {
-namespace {
-
-base::TimeDelta GetAutoReloadTime(size_t reload_count) {
-  static const int kDelaysMs[] = {0,      5000,   30000,  60000,
-                                  300000, 600000, 1800000};
-  if (reload_count >= base::size(kDelaysMs))
-    reload_count = base::size(kDelaysMs) - 1;
-  return base::TimeDelta::FromMilliseconds(kDelaysMs[reload_count]);
-}
-
-bool IsErrorPage(const GURL& url) {
-  return url.is_valid() && url.spec() == content::kUnreachableWebDataURL;
-}
-
-bool IsReloadableError(const error_page::Error& error, bool was_failed_post) {
-  const GURL& url = error.url();
-  return error.domain() == error_page::Error::kNetErrorDomain &&
-         error.reason() != net::ERR_ABORTED &&
-         // For now, net::ERR_UNKNOWN_URL_SCHEME is only being displayed on
-         // Chrome for Android.
-         error.reason() != net::ERR_UNKNOWN_URL_SCHEME &&
-         // Do not trigger if the server rejects a client certificate.
-         // https://crbug.com/431387
-         !net::IsClientCertificateError(error.reason()) &&
-         // Some servers reject client certificates with a generic
-         // handshake_failure alert.
-         // https://crbug.com/431387
-         error.reason() != net::ERR_SSL_PROTOCOL_ERROR &&
-         // Do not trigger for blacklisted URLs.
-         // https://crbug.com/803839
-         // Do not trigger for requests that were blocked by the browser itself.
-         !net::IsRequestBlockedError(error.reason()) && !was_failed_post &&
-         // Do not trigger for this error code because it is used by Chrome
-         // while an auth prompt is being displayed.
-         error.reason() != net::ERR_INVALID_AUTH_CREDENTIALS &&
-         // Don't auto-reload non-http/https schemas.
-         // https://crbug.com/471713
-         url.SchemeIsHTTPOrHTTPS() &&
-         // Don't auto reload if the error was a secure DNS network error, since
-         // the reload may interfere with the captive portal probe state.
-         // TODO(crbug.com/1016164): Explore how to allow reloads for secure DNS
-         // network errors without interfering with the captive portal probe
-         // state.
-         !error.resolve_error_info().is_secure_network_error;
-}
-
-}  // namespace
 
 struct ErrorPageHelper::ErrorPageInfo {
-  ErrorPageInfo(const error_page::Error& error, bool was_failed_post)
-      : error(error), was_failed_post(was_failed_post) {}
+  explicit ErrorPageInfo(const error_page::Error& error) : error(error) {}
 
   // Information about the failed page load.
   error_page::Error error;
-  bool was_failed_post = false;
 
   // True if a page has completed loading, at which point it can receive
   // updates.
@@ -88,97 +39,23 @@ ErrorPageHelper* ErrorPageHelper::GetForFrame(
   return render_frame->IsMainFrame() ? Get(render_frame) : nullptr;
 }
 
-void ErrorPageHelper::PrepareErrorPage(const error_page::Error& error,
-                                       bool was_failed_post) {
-  pending_error_page_info_ =
-      std::make_unique<ErrorPageInfo>(error, was_failed_post);
-}
-
-bool ErrorPageHelper::ShouldSuppressErrorPage(int error_code) {
-  // If there's no auto reload attempt in flight, this error page didn't come
-  // from auto reload, so don't suppress it.
-  if (!auto_reload_in_flight_)
-    return false;
-
-  // Even with auto_reload_in_flight_ error page may not come from
-  // the auto reload when proceeding from error CERT_AUTHORITY_INVALID
-  // to error INVALID_AUTH_CREDENTIALS, so do not suppress the error page
-  // for the new error code.
-  if (committed_error_page_info_ &&
-      committed_error_page_info_->error.reason() != error_code)
-    return false;
-
-  uncommitted_load_started_ = false;
-  // This serves to terminate the auto-reload in flight attempt. If
-  // ShouldSuppressErrorPage is called, the auto-reload yielded an error, which
-  // means the request was already sent.
-  auto_reload_in_flight_ = false;
-  MaybeStartAutoReloadTimer();
-  return true;
-}
-
-void ErrorPageHelper::DidStartNavigation(
-    const GURL& url,
-    base::Optional<blink::WebNavigationType> navigation_type) {
-  uncommitted_load_started_ = true;
-
-  // If there's no pending error page information associated with the page load,
-  // or the new page is not an error page, then reset pending error page state.
-  if (!pending_error_page_info_ || !IsErrorPage(url)) {
-    CancelPendingReload();
-  } else {
-    // Halt auto-reload if it's currently scheduled. OnFinishLoad will trigger
-    // auto-reload if appropriate.
-    PauseAutoReloadTimer();
-  }
+void ErrorPageHelper::PrepareErrorPage(const error_page::Error& error) {
+  pending_error_page_info_ = std::make_unique<ErrorPageInfo>(error);
 }
 
 void ErrorPageHelper::DidCommitProvisionalLoad(ui::PageTransition transition) {
-  // If a page is committing, either it's an error page and autoreload will be
-  // started again below, or it's a success page and we need to clear autoreload
-  // state.
-  auto_reload_in_flight_ = false;
-
-  // uncommitted_load_started_ could already be false, since RenderFrameImpl
-  // calls OnCommitLoad once for each in-page navigation (like a fragment
-  // change) with no corresponding OnStartLoad.
-  uncommitted_load_started_ = false;
-
   committed_error_page_info_ = std::move(pending_error_page_info_);
-
   weak_factory_.InvalidateWeakPtrs();
 }
 
 void ErrorPageHelper::DidFinishLoad() {
-  if (!committed_error_page_info_) {
-    auto_reload_count_ = 0;
+  if (!committed_error_page_info_)
     return;
-  }
 
   security_interstitials::SecurityInterstitialPageController::Install(
       render_frame(), weak_factory_.GetWeakPtr());
 
   committed_error_page_info_->is_finished_loading = true;
-  if (IsReloadableError(committed_error_page_info_->error,
-                        committed_error_page_info_->was_failed_post)) {
-    MaybeStartAutoReloadTimer();
-  }
-}
-
-void ErrorPageHelper::OnStop() {
-  CancelPendingReload();
-  uncommitted_load_started_ = false;
-  auto_reload_count_ = 0;
-  auto_reload_in_flight_ = false;
-}
-
-void ErrorPageHelper::WasShown() {
-  if (auto_reload_paused_)
-    MaybeStartAutoReloadTimer();
-}
-
-void ErrorPageHelper::WasHidden() {
-  PauseAutoReloadTimer();
 }
 
 void ErrorPageHelper::OnDestruct() {
@@ -251,14 +128,9 @@ ErrorPageHelper::GetInterface() {
 
 ErrorPageHelper::ErrorPageHelper(content::RenderFrame* render_frame)
     : RenderFrameObserver(render_frame),
-      RenderFrameObserverTracker<ErrorPageHelper>(render_frame),
-      online_(content::RenderThread::Get()->IsOnline()) {
-  content::RenderThread::Get()->AddObserver(this);
-}
+      RenderFrameObserverTracker<ErrorPageHelper>(render_frame) {}
 
-ErrorPageHelper::~ErrorPageHelper() {
-  content::RenderThread::Get()->RemoveObserver(this);
-}
+ErrorPageHelper::~ErrorPageHelper() = default;
 
 void ErrorPageHelper::Reload() {
   if (!committed_error_page_info_)
@@ -266,82 +138,4 @@ void ErrorPageHelper::Reload() {
   render_frame()->GetWebFrame()->StartReload(blink::WebFrameLoadType::kReload);
 }
 
-void ErrorPageHelper::MaybeStartAutoReloadTimer() {
-  // Automation tools expect to be in control of reloads.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableAutomation) ||
-      !base::FeatureList::IsEnabled(features::kEnableAutoReload)) {
-    return;
-  }
-
-  if (!committed_error_page_info_ ||
-      !committed_error_page_info_->is_finished_loading ||
-      pending_error_page_info_ || uncommitted_load_started_) {
-    return;
-  }
-
-  StartAutoReloadTimer();
-}
-
-void ErrorPageHelper::StartAutoReloadTimer() {
-  DCHECK(committed_error_page_info_);
-  DCHECK(IsReloadableError(committed_error_page_info_->error,
-                           committed_error_page_info_->was_failed_post));
-
-  if (!online_ || render_frame()->IsHidden()) {
-    auto_reload_paused_ = true;
-    return;
-  }
-
-  auto_reload_paused_ = false;
-  base::TimeDelta delay = GetAutoReloadTime(auto_reload_count_);
-  auto_reload_timer_.Stop();
-  auto_reload_timer_.Start(
-      FROM_HERE, delay,
-      base::BindOnce(&ErrorPageHelper::AutoReloadTimerFired,
-                     base::Unretained(this)));
-}
-
-void ErrorPageHelper::AutoReloadTimerFired() {
-  // AutoReloadTimerFired only runs if:
-  // 1. StartAutoReloadTimer was previously called, which requires that
-  //    committed_error_page_info_ is populated;
-  // 2. No other page load has started since (1), since DidStartNavigation stops
-  //    the auto-reload timer.
-  DCHECK(committed_error_page_info_);
-
-  auto_reload_count_++;
-  auto_reload_in_flight_ = true;
-  Reload();
-}
-
-void ErrorPageHelper::PauseAutoReloadTimer() {
-  if (!auto_reload_timer_.IsRunning())
-    return;
-  DCHECK(committed_error_page_info_);
-  DCHECK(!auto_reload_paused_);
-  auto_reload_timer_.Stop();
-  auto_reload_paused_ = true;
-}
-
-void ErrorPageHelper::NetworkStateChanged(bool online) {
-  bool was_online = online_;
-  online_ = online;
-  if (!was_online && online) {
-    // Transitioning offline -> online
-    if (auto_reload_paused_)
-      MaybeStartAutoReloadTimer();
-  } else if (was_online && !online) {
-    // Transitioning online -> offline
-    if (auto_reload_timer_.IsRunning())
-      auto_reload_count_ = 0;
-    PauseAutoReloadTimer();
-  }
-}
-
-void ErrorPageHelper::CancelPendingReload() {
-  auto_reload_timer_.Stop();
-  auto_reload_paused_ = false;
-}
-
 }  // namespace weblayer

weblayer/renderer/error_page_helper.h

@@ -10,7 +10,6 @@
 #include "components/security_interstitials/core/common/mojom/interstitial_commands.mojom.h"
 #include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_frame_observer_tracker.h"
-#include "content/public/renderer/render_thread_observer.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 
 namespace error_page {
@@ -27,7 +26,6 @@ namespace weblayer {
 class ErrorPageHelper
     : public content::RenderFrameObserver,
       public content::RenderFrameObserverTracker<ErrorPageHelper>,
-      public content::RenderThreadObserver,
       public security_interstitials::SecurityInterstitialPageController::
           Delegate {
  public:
@@ -40,26 +38,13 @@ class ErrorPageHelper
   static ErrorPageHelper* GetForFrame(content::RenderFrame* render_frame);
 
   // Called when the current navigation results in an error.
-  void PrepareErrorPage(const error_page::Error& error, bool was_failed_post);
-
-  // Returns whether a load for the frame the ErrorPageHelper is attached to
-  // should have its error page suppressed.
-  bool ShouldSuppressErrorPage(int error_code);
+  void PrepareErrorPage(const error_page::Error& error);
 
   // content::RenderFrameObserver:
-  void DidStartNavigation(
-      const GURL& url,
-      base::Optional<blink::WebNavigationType> navigation_type) override;
   void DidCommitProvisionalLoad(ui::PageTransition transition) override;
   void DidFinishLoad() override;
-  void OnStop() override;
-  void WasShown() override;
-  void WasHidden() override;
   void OnDestruct() override;
 
-  // content::RenderThreadObserver:
-  void NetworkStateChanged(bool online) override;
-
   // security_interstitials::SecurityInterstitialPageController::Delegate:
   void SendCommand(
       security_interstitials::SecurityInterstitialCommand command) override;
@@ -74,11 +59,6 @@ class ErrorPageHelper
   ~ErrorPageHelper() override;
 
   void Reload();
-  void MaybeStartAutoReloadTimer();
-  void StartAutoReloadTimer();
-  void AutoReloadTimerFired();
-  void PauseAutoReloadTimer();
-  void CancelPendingReload();
 
   // Information for the provisional / "pre-provisional" error page. Null when
   // there's no page pending, or the pending page is not an error page.
@@ -88,30 +68,6 @@ class ErrorPageHelper
   // not an error page.
   std::unique_ptr<ErrorPageInfo> committed_error_page_info_;
 
-  // Timer used to wait for auto-reload attempts.
-  base::OneShotTimer auto_reload_timer_;
-
-  // True if the auto-reload timer would be running but is waiting for an
-  // offline->online network transition.
-  bool auto_reload_paused_ = false;
-
-  // Whether an auto-reload-initiated Reload() attempt is in flight.
-  bool auto_reload_in_flight_ = false;
-
-  // True if there is an uncommitted-but-started load, error page or not. This
-  // is used to inhibit starting auto-reload when an error page finishes, in
-  // case this happens:
-  //   Error page starts
-  //   Error page commits
-  //   Non-error page starts
-  //   Error page finishes
-  bool uncommitted_load_started_ = false;
-
-  // Is the browser online?
-  bool online_ = false;
-
-  int auto_reload_count_ = 0;
-
   base::WeakPtrFactory<ErrorPageHelper> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ErrorPageHelper);

weblayer/test/BUILD.gn

@@ -95,6 +95,7 @@ test("weblayer_browsertests") {
     "//components/autofill/core/common",
     "//components/blocked_content",
     "//components/content_settings/core/browser",
+    "//components/error_page/content/browser",
     "//components/network_session_configurator/common",
     "//components/network_time",
     "//components/page_load_metrics/browser:browser",