Display DNS probe results.

chrome/browser/net/net_error_tab_helper.cc

Index: chrome/browser/net/net_error_tab_helper.cc
diff --git a/chrome/browser/net/net_error_tab_helper.cc b/chrome/browser/net/net_error_tab_helper.cc
index c7189a964fd6a99fc998dd1438949714665faefe..da061f39232083aa739889da6f4e3615fc4ecb2e 100644
--- a/chrome/browser/net/net_error_tab_helper.cc
+++ b/chrome/browser/net/net_error_tab_helper.cc
@@ -18,7 +18,8 @@
 #include "net/base/net_errors.h"
 
 using base::FieldTrialList;
-using chrome_common_net::DnsProbeResult;
+using chrome_common_net::DnsProbeStatus;
+using chrome_common_net::DnsProbeStatusToString;
 using content::BrowserContext;
 using content::BrowserThread;
 using content::PageTransition;
@@ -32,9 +33,6 @@ namespace chrome_browser_net {
 
 namespace {
 
-const char kDnsProbeFieldTrialName[] = "DnsProbe-Enable";
-const char kDnsProbeFieldTrialEnableGroupName[] = "enable";
-
 static NetErrorTabHelper::TestingState testing_state_ =
     NetErrorTabHelper::TESTING_DEFAULT;
 
@@ -45,33 +43,11 @@ bool IsDnsError(int net_error) {
          net_error == net::ERR_NAME_RESOLUTION_FAILED;
 }
 
-bool GetEnabledByTrial() {
-  return (FieldTrialList::FindFullName(kDnsProbeFieldTrialName)
-          == kDnsProbeFieldTrialEnableGroupName);
-}
-
-NetErrorTracker::FrameType GetFrameType(bool is_main_frame) {
-  return is_main_frame ? NetErrorTracker::FRAME_MAIN
-                       : NetErrorTracker::FRAME_SUB;
-}
-
-NetErrorTracker::PageType GetPageType(bool is_error_page) {
-  return is_error_page ? NetErrorTracker::PAGE_ERROR
-                       : NetErrorTracker::PAGE_NORMAL;
-}
-
-NetErrorTracker::ErrorType GetErrorType(int net_error) {
-  return IsDnsError(net_error) ? NetErrorTracker::ERROR_DNS
-                               : NetErrorTracker::ERROR_OTHER;
-}
-
 void OnDnsProbeFinishedOnIOThread(
-    const base::Callback<void(DnsProbeResult)>& callback,
-    DnsProbeResult result) {
+    const base::Callback<void(DnsProbeStatus)>& callback,
+    DnsProbeStatus result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  DVLOG(1) << "DNS probe finished with result " << result;
-
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
@@ -81,12 +57,10 @@ void OnDnsProbeFinishedOnIOThread(
 // We can only access g_browser_process->io_thread() from the browser thread,
 // so we have to pass it in to the callback instead of dereferencing it here.
 void StartDnsProbeOnIOThread(
-    const base::Callback<void(DnsProbeResult)>& callback,
+    const base::Callback<void(DnsProbeStatus)>& callback,
     IOThread* io_thread) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  DVLOG(1) << "Starting DNS probe";
-
   DnsProbeService* probe_service =
       io_thread->globals()->dns_probe_service.get();
 
@@ -113,8 +87,10 @@ void NetErrorTabHelper::DidStartProvisionalLoadForFrame(
     RenderViewHost* render_view_host) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  tracker_.OnStartProvisionalLoad(GetFrameType(is_main_frame),
-                                  GetPageType(is_error_page));
+  if (!is_main_frame)
+    return;
+
+  is_error_page_ = is_error_page;
 }
 
 void NetErrorTabHelper::DidCommitProvisionalLoadForFrame(
@@ -125,7 +101,23 @@ void NetErrorTabHelper::DidCommitProvisionalLoadForFrame(
     RenderViewHost* render_view_host) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  tracker_.OnCommitProvisionalLoad(GetFrameType(is_main_frame));
+  if (!is_main_frame)
+    return;
+
+  // Resend status every time we commit a page; this is somewhat spammy, but
+  // ensures that the status will make it to the real error page, even if the
+  // link doctor loads a blank intermediate page or the tab switches renderer
+  // processes.
+  if (is_error_page_) {
+    if (dns_error_active_) {
+      dns_error_page_committed_ = true;
+      DVLOG(1) << "Committed error page; resending status.";
+      SendInfo();
+    }
+  } else {
+    dns_error_active_ = false;
+    dns_error_page_committed_ = false;
+  }
 }
 
 void NetErrorTabHelper::DidFailProvisionalLoad(
@@ -137,75 +129,69 @@ void NetErrorTabHelper::DidFailProvisionalLoad(
     RenderViewHost* render_view_host) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  tracker_.OnFailProvisionalLoad(GetFrameType(is_main_frame),
-                                 GetErrorType(error_code));
-}
+  if (!is_main_frame)
+    return;
 
-void NetErrorTabHelper::DidFinishLoad(
-    int64 frame_id,
-    const GURL& validated_url,
-    bool is_main_frame,
-    RenderViewHost* render_view_host) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  tracker_.OnFinishLoad(GetFrameType(is_main_frame));
+  if (IsDnsError(error_code)) {
+    dns_error_active_ = true;
+    OnMainFrameDnsError();
+  }
 }
 
 NetErrorTabHelper::NetErrorTabHelper(WebContents* contents)
     : WebContentsObserver(contents),
+      dns_probe_status_(chrome_common_net::DNS_PROBE_POSSIBLE),
       weak_factory_(this),
-      tracker_(base::Bind(&NetErrorTabHelper::TrackerCallback,
-                          weak_factory_.GetWeakPtr())),
-      dns_error_page_state_(NetErrorTracker::DNS_ERROR_PAGE_NONE),
-      dns_probe_state_(DNS_PROBE_NONE),
-      enabled_by_trial_(GetEnabledByTrial()) {
+      is_error_page_(false),
+      dns_error_active_(false),
+      dns_error_page_committed_(false),
+      enabled_by_trial_(chrome_common_net::DnsProbesEnabledByFieldTrial()) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  InitializePref(contents);
+  // If we're being tested, we don't have a WebContents.
+  if (contents)
+    InitializePref(contents);
 }
 
-void NetErrorTabHelper::TrackerCallback(
-    NetErrorTracker::DnsErrorPageState state) {
-  dns_error_page_state_ = state;
-
-  MaybePostStartDnsProbeTask();
-  MaybeSendInfo();
+void NetErrorTabHelper::OnMainFrameDnsError() {
+  if (ProbesAllowed()) {
+    if (dns_probe_status_ != chrome_common_net::DNS_PROBE_STARTED) {

[mmenke, 2013/06/28 20:43:27]

Think this is worth a comment.  Something along the lines of "If multiple error
pages load for that one load failure, as is the case with the link doctor, this
will still only run a single probe.  The result will be sent to both pages, if
the probe completes before the second error page commits.  Otherwise, it will
just be sent to the second error page."

[Deprecated (see juliatuttle), 2013/07/01 17:39:55]

That's not what this check is for; this check is to make sure we don't start a
bunch of probes if we get a bunch of failures (e.g. if the user keeps typing in
new URLs that also fail while we're running probes for earlier ones). 

+      StartDnsProbe();
+      dns_probe_status_ = chrome_common_net::DNS_PROBE_STARTED;
+    }
+  } else {
+    dns_probe_status_ = chrome_common_net::DNS_PROBE_NOT_RUN;
+  }
 }
 
-void NetErrorTabHelper::MaybePostStartDnsProbeTask() {
+void NetErrorTabHelper::StartDnsProbe() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(dns_error_active_);
+  DCHECK_NE(chrome_common_net::DNS_PROBE_STARTED, dns_probe_status_);
 
-  if (dns_error_page_state_ != NetErrorTracker::DNS_ERROR_PAGE_NONE &&
-      dns_probe_state_ != DNS_PROBE_STARTED &&
-      ProbesAllowed()) {
-    BrowserThread::PostTask(
-        BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&StartDnsProbeOnIOThread,
-                   base::Bind(&NetErrorTabHelper::OnDnsProbeFinished,
-                              weak_factory_.GetWeakPtr()),
-                   g_browser_process->io_thread()));
-    dns_probe_state_ = DNS_PROBE_STARTED;
-  }
+  DVLOG(1) << "Starting DNS probe.";
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&StartDnsProbeOnIOThread,
+                 base::Bind(&NetErrorTabHelper::OnDnsProbeFinished,
+                            weak_factory_.GetWeakPtr()),
+                 g_browser_process->io_thread()));
 }
 
-void NetErrorTabHelper::OnDnsProbeFinished(DnsProbeResult result) {
+void NetErrorTabHelper::OnDnsProbeFinished(DnsProbeStatus result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(DNS_PROBE_STARTED, dns_probe_state_);
+  DCHECK_EQ(chrome_common_net::DNS_PROBE_STARTED, dns_probe_status_);
+  DCHECK(chrome_common_net::DnsProbeStatusIsFinished(result));
 
-  dns_probe_result_ = result;
-  dns_probe_state_ = DNS_PROBE_FINISHED;
+  DVLOG(1) << "Finished DNS probe with result "
+           << DnsProbeStatusToString(result) << ".";
 
-  MaybeSendInfo();
-}
+  dns_probe_status_ = result;
 
-void NetErrorTabHelper::MaybeSendInfo() {
-  if (dns_error_page_state_ == NetErrorTracker::DNS_ERROR_PAGE_LOADED &&
-      dns_probe_state_ == DNS_PROBE_FINISHED) {
-    DVLOG(1) << "Sending result " << dns_probe_result_ << " to renderer";
-    Send(new ChromeViewMsg_NetErrorInfo(routing_id(), dns_probe_result_));
-    dns_probe_state_ = DNS_PROBE_NONE;
-  }
+  if (dns_error_page_committed_)
+    SendInfo();
 }
 
 void NetErrorTabHelper::InitializePref(WebContents* contents) {
@@ -226,4 +212,15 @@ bool NetErrorTabHelper::ProbesAllowed() const {
   return enabled_by_trial_ && *resolve_errors_with_web_service_;
 }
 
+void NetErrorTabHelper::SendInfo() {
+  DCHECK_NE(chrome_common_net::DNS_PROBE_POSSIBLE, dns_probe_status_);
+  DCHECK(dns_error_page_committed_);
+
+  DVLOG(1) << "Sending status " << DnsProbeStatusToString(dns_probe_status_);
+  Send(new ChromeViewMsg_NetErrorInfo(routing_id(), dns_probe_status_));
+
+  if (!dns_probe_status_snoop_callback_.is_null())
+    dns_probe_status_snoop_callback_.Run(dns_probe_status_);
+}
+
 }  // namespace chrome_browser_net