Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1661)

Unified Diff: chrome/browser/net/net_error_tab_helper.cc

Issue 13270005: Display DNS probe results. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..c7d2346f5f963a77039d61cc4ce5032812dd94e6 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,48 +43,24 @@ 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,
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.
+// Can only access g_browser_process->io_thread() from the browser thread,
+// so 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,21 @@ 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 an error page commits; this is somewhat spammy,
+ // but // ensures that the status will make it to the real error page, even
mmenke 2013/07/15 16:33:22 nit: Remove extra "//"
Deprecated (see juliatuttle) 2013/07/15 19:33:38 Done.
+ // if the link doctor loads a blank intermediate page or the tab switches
+ // renderer processes.
+ if (is_error_page_ && 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 +127,70 @@ 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),
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 this helper is under test, it won'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()) {
+ // 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::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 +211,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

Powered by Google App Engine
This is Rietveld 408576698