Display DNS probe results.

chrome/browser/net/net_error_tab_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/net_error_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/metrics/field_trial.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/dns_probe_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/net/net_error_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "content/public/browser/browser_thread.h"
 #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;
 using content::RenderViewHost;
 using content::WebContents;
 using content::WebContentsObserver;
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(chrome_browser_net::NetErrorTabHelper);
 
 namespace chrome_browser_net {
 
 namespace {
 
-const char kDnsProbeFieldTrialName[] = "DnsProbe-Enable";
-const char kDnsProbeFieldTrialEnableGroupName[] = "enable";
-
 static NetErrorTabHelper::TestingState testing_state_ =
     NetErrorTabHelper::TESTING_DEFAULT;
 
 // 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(int net_error) {
   return net_error == net::ERR_NAME_NOT_RESOLVED ||
          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,
       base::Bind(callback, result));
 }
 
 // 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();
 
   probe_service->ProbeDns(base::Bind(&OnDnsProbeFinishedOnIOThread, callback));
 }
 
 }  // namespace
 
 NetErrorTabHelper::~NetErrorTabHelper() {
 }
 
 // static
 void NetErrorTabHelper::set_state_for_testing(TestingState state) {
   testing_state_ = state;
 }
 
 void NetErrorTabHelper::DidStartProvisionalLoadForFrame(
     int64 frame_id,
     int64 parent_frame_id,
     bool is_main_frame,
     const GURL& validated_url,
     bool is_error_page,
     bool is_iframe_srcdoc,
     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(
     int64 frame_id,
     bool is_main_frame,
     const GURL& url,
     PageTransition transition_type,
     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();
+    }

[mmenke, 2013/07/12 19:51:22]

Shouldn't we clear dns_error_page_committed_ if dns_error_active is false?  If
we have a DNS error and then a non-DNS error, looks like we'd still be sending
info.  This would be benign, since the renderer would ignore the extra messages,
and we wouldn't trigger any extra probes.  Regardless, seems like a bad idea. 
Think we can easily unit test it.

[Deprecated (see juliatuttle), 2013/07/12 20:55:55]

Done.

+  } else {
+    dns_error_active_ = false;

[mmenke, 2013/07/12 19:51:22]

optional:  Could we just clear this in DidStartProvisionalLoadForFrame, if
is_error_page_ is false?  This works and is fine, just wondering if it would be
more consistent to do so.  Sorry if I asked this before.

[Deprecated (see juliatuttle), 2013/07/12 20:55:55]

What would it be more consistent with?  Right now all we do in Start is remember
whether it's an error page or not.

[mmenke, 2013/07/12 21:07:49]

It would be more consistent with the current state of the RenderView.

+    dns_error_page_committed_ = false;
+  }
 }
 
 void NetErrorTabHelper::DidFailProvisionalLoad(
     int64 frame_id,
     bool is_main_frame,
     const GURL& validated_url,
     int error_code,
     const string16& error_description,
     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),
       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),
+      dns_probe_status_(chrome_common_net::DNS_PROBE_POSSIBLE),
+      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.

[mmenke, 2013/07/12 19:51:22]

nit:  Some people feel strongly about not using "we" in comments.  Suggest just
using "this" instead.

[Deprecated (see juliatuttle), 2013/07/12 20:55:55]

Fixed, here and elsewhere.

+  if (contents)
+    InitializePref(contents);
 }
 
-void NetErrorTabHelper::TrackerCallback(
-    NetErrorTracker::DnsErrorPageState state) {
-  dns_error_page_state_ = state;
-
-  MaybePostStartDnsProbeTask();
-  MaybeSendInfo();
-}
-
-void NetErrorTabHelper::MaybePostStartDnsProbeTask() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  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;
+void NetErrorTabHelper::OnMainFrameDnsError() {
+  if (ProbesAllowed()) {
+    // Don't start more than one probe at a time.
+    if (dns_probe_status_ != chrome_common_net::DNS_PROBE_STARTED) {
+      StartDnsProbe();
+      dns_probe_status_ = chrome_common_net::DNS_PROBE_STARTED;
+    }
+  } else {
+    dns_probe_status_ = chrome_common_net::DNS_PROBE_NOT_RUN;
   }
 }
 
-void NetErrorTabHelper::OnDnsProbeFinished(DnsProbeResult result) {
+void NetErrorTabHelper::StartDnsProbe() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(DNS_PROBE_STARTED, dns_probe_state_);
+  DCHECK(dns_error_active_);
+  DCHECK_NE(chrome_common_net::DNS_PROBE_STARTED, dns_probe_status_);
 
-  dns_probe_result_ = result;
-  dns_probe_state_ = DNS_PROBE_FINISHED;
+  DVLOG(1) << "Starting DNS probe.";
 
-  MaybeSendInfo();
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&StartDnsProbeOnIOThread,
+                 base::Bind(&NetErrorTabHelper::OnDnsProbeFinished,
+                            weak_factory_.GetWeakPtr()),
+                 g_browser_process->io_thread()));
 }
 
-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;
-  }
+void NetErrorTabHelper::OnDnsProbeFinished(DnsProbeStatus result) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(chrome_common_net::DNS_PROBE_STARTED, dns_probe_status_);
+  DCHECK(chrome_common_net::DnsProbeStatusIsFinished(result));
+
+  DVLOG(1) << "Finished DNS probe with result "
+           << DnsProbeStatusToString(result) << ".";
+
+  dns_probe_status_ = result;
+
+  if (dns_error_page_committed_)
+    SendInfo();
 }
 
 void NetErrorTabHelper::InitializePref(WebContents* contents) {
   DCHECK(contents);
 
   BrowserContext* browser_context = contents->GetBrowserContext();
   Profile* profile = Profile::FromBrowserContext(browser_context);
   resolve_errors_with_web_service_.Init(
       prefs::kAlternateErrorPagesEnabled,
       profile->GetPrefs());
 }
 
 bool NetErrorTabHelper::ProbesAllowed() const {
   if (testing_state_ != TESTING_DEFAULT)
     return testing_state_ == TESTING_FORCE_ENABLED;
 
   // TODO(ttuttle): Disable on mobile?
   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