Display DNS probe results.

chrome/renderer/net/net_error_helper.cc

Index: chrome/renderer/net/net_error_helper.cc
diff --git a/chrome/renderer/net/net_error_helper.cc b/chrome/renderer/net/net_error_helper.cc
index c51a7c2ddc269fb0ac69e34699cf46f48e128ece..03cd3d2e05e49eb1d00a0b09d70e5bbc921563d6 100644
--- a/chrome/renderer/net/net_error_helper.cc
+++ b/chrome/renderer/net/net_error_helper.cc
@@ -4,16 +4,24 @@
 
 #include "chrome/renderer/net/net_error_helper.h"
 
+#include <string>

[mmenke, 2013/04/01 17:44:12]

This should be in the header

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

+
+#include "base/json/json_writer.h"
+#include "base/stringprintf.h"

[mmenke, 2013/04/01 17:44:12]

This doesn't seem to be used.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/common/localized_error.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/net/net_error_info.h"
+#include "chrome/common/net/net_error_tracker.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "googleurl/src/gurl.h"
+#include "grit/chromium_strings.h"
+#include "grit/generated_resources.h"

[mmenke, 2013/04/01 17:44:12]

Don't think these are used.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
 #include "net/base/net_errors.h"
@@ -22,9 +30,12 @@
 #include "third_party/WebKit/Source/Platform/chromium/public/WebURLRequest.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDataSource.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
+#include "ui/base/l10n/l10n_util.h"

[mmenke, 2013/04/01 17:44:12]

Not used

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

 
 using base::DictionaryValue;
+using base::JSONWriter;
 using chrome_common_net::DnsProbeResult;
+using chrome_common_net::DnsProbeStatus;
 using content::GetContentClient;
 using content::RenderThread;
 using content::RenderView;
@@ -41,11 +52,10 @@ bool IsErrorPage(const GURL& url) {
   return (url.spec() == kUnreachableWebDataURL);
 }
 
-// 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 IsDnsError(const WebKit::WebURLError& error) {
+  return std::string(error.domain.utf8()) == net::kErrorDomain &&
+         (error.reason == net::ERR_NAME_NOT_RESOLVED ||
+          error.reason == net::ERR_NAME_RESOLUTION_FAILED);
 }
 
 NetErrorTracker::FrameType GetFrameType(WebKit::WebFrame* frame) {
@@ -60,54 +70,25 @@ NetErrorTracker::PageType GetPageType(WebKit::WebFrame* frame) {
 }
 
 NetErrorTracker::ErrorType GetErrorType(const WebKit::WebURLError& error) {
-  return IsDnsError(error.reason) ? NetErrorTracker::ERROR_DNS
-                                  : NetErrorTracker::ERROR_OTHER;
-}
-
-// Converts a DNS probe result into a net error.  Returns OK if the error page
-// should not be changed from the original DNS error.
-int DnsProbeResultToNetError(DnsProbeResult result) {
-  switch (result) {
-  case chrome_common_net::DNS_PROBE_UNKNOWN:
-    return net::OK;
-  case chrome_common_net::DNS_PROBE_NO_INTERNET:
-    // TODO(ttuttle): This is not the same error as when NCN returns this;
-    // ideally we should have two separate error codes for "no network" and
-    // "network with no internet".
-    return net::ERR_INTERNET_DISCONNECTED;
-  case chrome_common_net::DNS_PROBE_BAD_CONFIG:
-    // This is unspecific enough that we should still show the full DNS error
-    // page.
-    return net::OK;
-  case chrome_common_net::DNS_PROBE_NXDOMAIN:
-    return net::ERR_NAME_NOT_RESOLVED;
-  default:
-    NOTREACHED();
-    return net::OK;
-  }
-}
-
-WebKit::WebURLError NetErrorToWebURLError(int net_error) {
-  WebKit::WebURLError error;
-  error.domain = WebKit::WebString::fromUTF8(net::kErrorDomain);
-  error.reason = net_error;
-  return error;
+  return IsDnsError(error) ? NetErrorTracker::ERROR_DNS
+                           : NetErrorTracker::ERROR_OTHER;
 }
 
 }  // namespace
 
 NetErrorHelper::NetErrorHelper(RenderView* render_view)
     : RenderViewObserver(render_view),
-      ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(base::Bind(
-          &NetErrorHelper::TrackerCallback,
-          base::Unretained(this)))),
-      dns_error_page_state_(NetErrorTracker::DNS_ERROR_PAGE_NONE),
-      updated_error_page_(false) {
+      state_(IDLE),
+      ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(

[mmenke, 2013/04/01 17:44:12]

It looks like we're now completely ignoring the results returned by the tracker.
 Should we go back to using it?  If not, we should get rid of it.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Went back to using it.

+          base::Bind(&NetErrorHelper::TrackerCallback,
+                     base::Unretained(this)))) {
 }
 
 NetErrorHelper::~NetErrorHelper() {
 }
 
+// Returns whether |net_error| is a DNS-related error (and therefore whether
+// the tab helper should start a DNS probe after receiving it.)

[mmenke, 2013/04/01 17:44:12]

Function level documentation should be with the function declaration, not its
implementation.

Also...This doesn't seem to return anything.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

It belonged with IsDnsError, which is declared above.  I've put it in the right
place.

 void NetErrorHelper::DidStartProvisionalLoad(WebKit::WebFrame* frame) {
   tracker_.OnStartProvisionalLoad(GetFrameType(frame), GetPageType(frame));

[mmenke, 2013/04/01 17:44:12]

state_ should be reset if this isn't an error page load, right?  Isn't that what
tracker_'s for?

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

 }
@@ -115,6 +96,19 @@ void NetErrorHelper::DidStartProvisionalLoad(WebKit::WebFrame* frame) {
 void NetErrorHelper::DidFailProvisionalLoad(WebKit::WebFrame* frame,
                                             const WebKit::WebURLError& error) {
   tracker_.OnFailProvisionalLoad(GetFrameType(frame), GetErrorType(error));
+
+  if (frame->parent() || !IsDnsError(error))
+    return;
+
+  if (state_ != IDLE) {
+    LOG(WARNING) << "DNS error in unexpected state " << state_;

[mmenke, 2013/04/01 17:44:12]

DVLOG?

Also...Why isn't this expected?  Reload an error page before the probe finishes,
and it seems like you'd end up here.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

+    return;
+  }
+
+  DVLOG(1) << "Failed DNS page load; now awaiting error page load.";
+
+  state_ = AWAITING_LOAD;
+  last_error_ = error;
 }
 
 void NetErrorHelper::DidCommitProvisionalLoad(WebKit::WebFrame* frame,
@@ -124,6 +118,25 @@ void NetErrorHelper::DidCommitProvisionalLoad(WebKit::WebFrame* frame,
 
 void NetErrorHelper::DidFinishLoad(WebKit::WebFrame* frame) {
   tracker_.OnFinishLoad(GetFrameType(frame));
+
+  if (frame->parent())
+    return;
+
+  if (state_ == IDLE)
+    return;
+
+  if (state_ != AWAITING_LOAD) {
+    LOG(WARNING) << "Finish load in unexpected state " << state_;

[mmenke, 2013/04/01 17:44:12]

DVLOG?

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

Done.

+    return;
+  }
+
+  DVLOG(1) << "Finished error page load; now awaiting IPC.";
+
+  state_ = AWAITING_INITIAL_IPC;
+}
+
+void NetErrorHelper::TrackerCallback(
+    NetErrorTracker::DnsErrorPageState state) {
 }
 
 bool NetErrorHelper::OnMessageReceived(const IPC::Message& message) {
@@ -137,52 +150,114 @@ bool NetErrorHelper::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void NetErrorHelper::OnNetErrorInfo(int dns_probe_result) {
-  DVLOG(1) << "Received DNS probe result " << dns_probe_result;
+bool NetErrorHelper::GetErrorStringsForDnsProbe(
+    const WebKit::WebURLError& error,
+    base::DictionaryValue* strings,
+    const std::string& locale) {
+  if (!IsDnsError(error))
+    return false;
+
+  // Get the strings for a fake "DNS probe possible" error
+  WebKit::WebURLError fake_error;
+  fake_error.domain = WebKit::WebString::fromUTF8(
+      chrome_common_net::kDnsProbeErrorDomain);
+  fake_error.reason = chrome_common_net::DNS_PROBE_ERR_POSSIBLE;
+  fake_error.unreachableURL = error.unreachableURL;
+  LocalizedError::GetStrings(fake_error, strings, locale);
+  return true;
+}
+
+void NetErrorHelper::OnNetErrorInfo(int status_num, int result_num) {
+  DVLOG(1) << "Got IPC: status=" << status_num << " result=" << result_num;
 
-  if (dns_probe_result < 0 ||
-      dns_probe_result >= chrome_common_net::DNS_PROBE_MAX) {
-    DLOG(WARNING) << "Ignoring DNS probe result: invalid result "
-                  << dns_probe_result;
+  if (status_num < 0 ||
+      status_num >= chrome_common_net::DNS_PROBE_STATUS_MAX) {
+    DLOG(WARNING) << "Ignoring NetErrorInfo: invalid status " << status_num;
     NOTREACHED();
     return;
   }
 
-  if (dns_error_page_state_ != NetErrorTracker::DNS_ERROR_PAGE_LOADED) {
-    DVLOG(1) << "Ignoring DNS probe result: not on DNS error page.";
+  if (result_num < 0 ||
+      result_num >= chrome_common_net::DNS_PROBE_MAX) {
+    DLOG(WARNING) << "Ignoring NetErrorInfo: invalid result " << result_num;
+    NOTREACHED();
     return;
   }
 
-  if (updated_error_page_) {
-    DVLOG(1) << "Ignoring DNS probe result: already updated error page.";
+  DnsProbeStatus status = static_cast<DnsProbeStatus>(status_num);
+  DnsProbeResult result = static_cast<DnsProbeResult>(result_num);
+
+  if (state_ != AWAITING_INITIAL_IPC &&
+      (state_ != AWAITING_PROBE_RESULT ||
+       status != chrome_common_net::DNS_PROBE_FINISHED)) {
+    DLOG(WARNING) << "Ignoring NetErrorInfo: invalid status " << status_num
+                  << " for state " << state_;
+    NOTREACHED();

[mmenke, 2013/04/01 17:44:12]

This seems reachable:  If the error page is reloaded when we're in
AWAITING_PROBE_RESULT, then we can get one of the other messages.

[Deprecated (see juliatuttle), 2013/04/09 20:35:49]

We'd see the DidStart and DidCommit before we got another message.

     return;
   }
 
-  UpdateErrorPage(static_cast<DnsProbeResult>(dns_probe_result));
-  updated_error_page_ = true;
-}
+  if (status == chrome_common_net::DNS_PROBE_STARTED)
+    state_ = AWAITING_PROBE_RESULT;
+  else
+    state_ = IDLE;
 
-void NetErrorHelper::TrackerCallback(
-    NetErrorTracker::DnsErrorPageState state) {
-  dns_error_page_state_ = state;
+  last_status_ = status;
+  last_result_ = result;
 
-  if (state == NetErrorTracker::DNS_ERROR_PAGE_LOADED)
-    updated_error_page_ = false;
+  UpdateErrorPage();
 }
 
-void NetErrorHelper::UpdateErrorPage(DnsProbeResult dns_probe_result) {
-  DVLOG(1) << "Updating error page with result " << dns_probe_result;
+WebKit::WebURLError NetErrorHelper::MakeUpdatedError() {
+  WebKit::WebURLError error;
 
-  int net_error = DnsProbeResultToNetError(dns_probe_result);
-  if (net_error == net::OK)
-    return;
+  switch (last_status_) {
+  case chrome_common_net::DNS_PROBE_STARTED:
+    error.domain = WebKit::WebString::fromUTF8(
+        chrome_common_net::kDnsProbeErrorDomain);
+    error.reason = chrome_common_net::DNS_PROBE_ERR_RUNNING;
+    break;
+
+  case chrome_common_net::DNS_PROBE_FINISHED:
+    error.domain = WebKit::WebString::fromUTF8(
+        chrome_common_net::kDnsProbeErrorDomain);
+    error.reason = chrome_common_net::DNS_PROBE_ERR_FINISHED +
+                   last_result_;
+    break;
+
+  case chrome_common_net::DNS_PROBE_NOT_RUN:
+    error.domain = last_error_.domain;
+    error.reason = last_error_.reason;
+    break;
+
+  case chrome_common_net::DNS_PROBE_STATUS_MAX:
+    NOTREACHED();
+    break;
+  }
 
-  DVLOG(1) << "net error code is " << net_error;
+  error.unreachableURL = last_error_.unreachableURL;
+
+  return error;
+}
 
+void NetErrorHelper::UpdateErrorPage() {
   DictionaryValue error_strings;
-  LocalizedError::GetStrings(NetErrorToWebURLError(net_error),
+  LocalizedError::GetStrings(MakeUpdatedError(),
                              &error_strings,
                              RenderThread::Get()->GetLocale());
 
-  // TODO(ttuttle): Update error page with error_strings.
+  std::string json;
+  JSONWriter::Write(&error_strings, &json);
+
+  std::string js = "updateForDnsProbe(" + json + ");";
+  string16 js16;
+  if (!UTF8ToUTF16(js.c_str(), js.length(), &js16)) {
+    NOTREACHED();
+    return;
+  }
+
+  DVLOG(1) << "Updating error page.";
+  DVLOG(2) << "New strings: " << js;
+
+  string16 frame_xpath;
+  render_view()->EvaluateScript(frame_xpath, js16, 0, false);
 }