Support auto-reload on errors.

chrome/renderer/net/net_error_helper_core.cc

Index: chrome/renderer/net/net_error_helper_core.cc
diff --git a/chrome/renderer/net/net_error_helper_core.cc b/chrome/renderer/net/net_error_helper_core.cc
index 0b947e4ee5c475cf9a957e2ee9ceff990e24a0cd..13efa1bc34b92b41c9e70b838789ea4fed19a41b 100644
--- a/chrome/renderer/net/net_error_helper_core.cc
+++ b/chrome/renderer/net/net_error_helper_core.cc
@@ -6,9 +6,12 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/i18n/rtl.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
+#include "base/location.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string16.h"
 #include "base/values.h"
@@ -45,6 +48,15 @@ const CorrectionTypeToResourceTable kCorrectionResourceTable[] = {
   {IDS_ERRORPAGES_SUGGESTION_CORRECTED_URL, "emphasizedUrlCorrection"},
 };
 
+base::TimeDelta GetAutoReloadTime(size_t reload_count) {
+  static const int kDelaysMs[] = {
+    0, 5000, 30000, 60000, 300000, 600000, 1800000
+  };
+  if (reload_count >= arraysize(kDelaysMs))
+    reload_count = arraysize(kDelaysMs) - 1;
+  return base::TimeDelta::FromMilliseconds(kDelaysMs[reload_count]);
+}
+
 // Returns whether |net_error| is a DNS-related error (and therefore whether
 // the tab helper should start a DNS probe after receiving it.)
 bool IsDnsError(const blink::WebURLError& error) {
@@ -271,19 +283,36 @@ struct NetErrorHelperCore::ErrorPageInfo {
   bool is_finished_loading;
 };
 
+bool NetErrorHelperCore::IsReloadableError(
+    const NetErrorHelperCore::ErrorPageInfo& info) {
+  return info.error.domain.utf8() == net::kErrorDomain &&
+         info.error.reason != net::ERR_ABORTED &&
+         !info.was_failed_post;
+}
+
 NetErrorHelperCore::NetErrorHelperCore(Delegate* delegate)
     : delegate_(delegate),
-      last_probe_status_(chrome_common_net::DNS_PROBE_POSSIBLE) {
+      last_probe_status_(chrome_common_net::DNS_PROBE_POSSIBLE),
+      auto_reload_enabled_(false),
+      auto_reload_timer_(new MockableOneShotTimer()),
+      online_(true),
+      auto_reload_count_(0),
+      can_auto_reload_page_(false) {
 }
 
 NetErrorHelperCore::~NetErrorHelperCore() {
 }
 
-void NetErrorHelperCore::OnStop() {
-  // On stop, cancel loading navigation corrections, and prevent any
-  // pending error page load from starting to load corrections.  Swapping in an
-  // error page once corrections are received could interrupt a navigation,
-  // otherwise.
+void NetErrorHelperCore::CancelPendingFetches() {
+  // Cancel loading the alternate error page, and prevent any pending error page
+  // load from starting a new error page load.  Swapping in the error page when
+  // it's finished loading could abort the navigation, otherwise.
+  if (committed_error_page_info_ && can_auto_reload_page_) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.AutoReload.ErrorAtStop",
+                                     committed_error_page_info_->error.reason,

[mmenke, 2014/03/12 19:02:42]

I think this should be -committed_error_page_info_->error.reason, since net
errors are negative?  Same for the others as well.

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+                                     net::GetAllErrorCodesForUma());
+    UMA_HISTOGRAM_COUNTS("Net.AutoReload.CountAtStop", auto_reload_count_);

[mmenke, 2014/03/12 19:02:42]

Are you going to update histograms.xml in this CL, too?  Think it's kinda nice
to have them in the same CL as they were added., though this is a pretty large
CL already.

[mmenke, 2014/03/12 19:02:42]

If the process or RV is killed by quitting chrome or closing the tab, we
currently don't log anything...  Not sure if logging on destruction would make
it to the browser process, anyways, but thought I'd mention it.

[Elly Fong-Jones, 2014/03/12 20:10:53]

I was going to avoid that - it adds another layer of needed reviews.

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+  }
   if (committed_error_page_info_) {
     committed_error_page_info_->navigation_correction_url = GURL();
     committed_error_page_info_->navigation_correction_request_body.clear();
@@ -293,6 +322,13 @@ void NetErrorHelperCore::OnStop() {
     pending_error_page_info_->navigation_correction_request_body.clear();
   }
   delegate_->CancelFetchNavigationCorrections();
+  auto_reload_timer_->Stop();
+}
+
+void NetErrorHelperCore::OnStop() {
+  CancelPendingFetches();
+  auto_reload_count_ = 0;
+  can_auto_reload_page_ = false;
 }
 
 void NetErrorHelperCore::OnStartLoad(FrameType frame_type, PageType page_type) {
@@ -302,7 +338,13 @@ void NetErrorHelperCore::OnStartLoad(FrameType frame_type, PageType page_type) {
   // 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_ || page_type != ERROR_PAGE) {
-    OnStop();
+    CancelPendingFetches();
+    // If a non-error load is starting, don't try to auto-reload while it is in
+    // flight.
+    can_auto_reload_page_ = false;

[mmenke, 2014/03/12 19:02:42]

May want to do this in CancelPendingFetches(), since we do it in both places
that call it.

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+  } else {
+    // If an error load is starting, the resulting error page is autoreloadable.
+    can_auto_reload_page_ = true;

[mmenke, 2014/03/12 19:02:42]

Suggest:

can_auto_reload_page_ = IsReloadableError(*pending_error_page_info_);

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

   }
 }
 
@@ -310,12 +352,31 @@ void NetErrorHelperCore::OnCommitLoad(FrameType frame_type) {
   if (frame_type != MAIN_FRAME)
     return;
 
+  if (committed_error_page_info_ && !pending_error_page_info_ &&
+      can_auto_reload_page_) {
+    int reason = committed_error_page_info_->error.reason;
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.AutoReload.ErrorAtSuccess",
+                                     reason,
+                                     net::GetAllErrorCodesForUma());
+    UMA_HISTOGRAM_COUNTS("Net.AutoReload.CountAtSuccess", auto_reload_count_);
+    if (auto_reload_count_ == 1) {
+      UMA_HISTOGRAM_CUSTOM_ENUMERATION("Net.AutoReload.ErrorAtFirstSuccess",
+                                       reason,
+                                       net::GetAllErrorCodesForUma());
+    }
+  }
+
   committed_error_page_info_.reset(pending_error_page_info_.release());
 }
 
 void NetErrorHelperCore::OnFinishLoad(FrameType frame_type) {
-  if (frame_type != MAIN_FRAME || !committed_error_page_info_)
+  if (frame_type != MAIN_FRAME)
+    return;
+
+  if (!committed_error_page_info_) {
+    auto_reload_count_ = 0;
     return;
+  }
 
   committed_error_page_info_->is_finished_loading = true;
 
@@ -327,6 +388,10 @@ void NetErrorHelperCore::OnFinishLoad(FrameType frame_type) {
     delegate_->FetchNavigationCorrections(
         committed_error_page_info_->navigation_correction_url,
         committed_error_page_info_->navigation_correction_request_body);
+  } else if (auto_reload_enabled_ &&
+             IsReloadableError(*committed_error_page_info_)) {
+    can_auto_reload_page_ = true;

[mmenke, 2014/03/12 19:02:42]

Get rid of can_auto_reload_page_, and replace
IsReloadableError(*committed_error_page_info_) with can_auto_reload_page_?

If there's no unit test to catch that, we should have one.

"That" being:
* Error page is committed, but not finished loading.
* New load starts
* Error page stops loading (I'm not really sure if we actually stop loading the
first page or not, but we shouldn't depend on it).  The timer should not be
started.
* New load commits.
* New load finishes loading.

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+    MaybeStartAutoReloadTimer();
   }
 
   if (!committed_error_page_info_->needs_dns_updates ||
@@ -498,3 +563,72 @@ blink::WebURLError NetErrorHelperCore::GetUpdatedError(
 
   return updated_error;
 }
+
+void NetErrorHelperCore::Reload() {
+  if (!committed_error_page_info_) {
+    return;
+  }
+  delegate_->ReloadPage();
+}
+
+bool NetErrorHelperCore::MaybeStartAutoReloadTimer() {
+  if (!committed_error_page_info_ || !can_auto_reload_page_)

[mmenke, 2014/03/12 19:02:42]

Can't we DCHECK on these now?

[Elly Fong-Jones, 2014/03/12 20:10:53]

No, because this can be called from NetworkStateChanged, in which they are not
guaranteed to be true.

+    return false;
+

[mmenke, 2014/03/12 19:02:42]

DCHECK(!pending_error_page_info_)?

(Logic elsewhere should make sure this never happens).

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+  DCHECK(IsReloadableError(*committed_error_page_info_));
+
+  if (!online_)
+    return false;
+
+  StartAutoReloadTimer();
+  return true;
+}
+
+void NetErrorHelperCore::StartAutoReloadTimer() {
+  DCHECK(committed_error_page_info_);
+  DCHECK(can_auto_reload_page_);
+  base::TimeDelta delay = GetAutoReloadTime(auto_reload_count_);
+  auto_reload_count_++;
+  auto_reload_timer_->Stop();
+  auto_reload_timer_->Start(FROM_HERE, delay,
+      base::Bind(&NetErrorHelperCore::Reload,
+                 base::Unretained(this)));
+}
+
+void NetErrorHelperCore::NetworkStateChanged(bool online) {
+  online_ = online;
+  if (auto_reload_timer_->IsRunning()) {
+    DCHECK(committed_error_page_info_);
+    // If there's an existing timer running, stop it and reset the retry count.
+    auto_reload_timer_->Stop();
+    auto_reload_count_ = 0;
+  }
+
+  // If the network state changed to online, maybe start auto-reloading again.
+  if (online)
+    MaybeStartAutoReloadTimer();
+}
+
+bool NetErrorHelperCore::ShouldSuppressErrorPage(FrameType frame_type,
+                                                 const GURL& url) {
+  // Don't suppress child frame errors.
+  if (frame_type != MAIN_FRAME)
+    return false;
+
+  // If |auto_reload_timer_| is still running, this error page isn't from an
+  // auto reload.
+  if (auto_reload_timer_->IsRunning())
+    return false;
+
+  // If there's no committed error page, this error page wasn't from an auto
+  // reload.
+  if (!committed_error_page_info_ || !can_auto_reload_page_)
+    return false;
+
+  GURL error_url = committed_error_page_info_->error.unreachableURL;

[mmenke, 2014/03/12 19:02:42]

Maybe add a TODO about checking the new error code against the old one, too?

As-is, this catches manual reloads that result in different errors, I believe.

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+  if (error_url != url)
+    return false;
+
+  MaybeStartAutoReloadTimer();

[mmenke, 2014/03/12 19:02:42]

Think this is worth a comment (Something along the lines of the timer is started
in OnFinishLoad the first time we get the error, but since subsequent errors
don't reload the error page, have to start the timer here for them).

[Elly Fong-Jones, 2014/03/12 20:10:53]

Done.

+  return true;
+}