Add error description to the DidFailProvisionalLoad callback.

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 39a108a0ac70e38f0e06fbf5350f61ccc8d6f467..cfbe96a3826019dc57132018a588ee704afecfe1 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -533,7 +533,7 @@ void ChromeContentRendererClient::ShowErrorPage(RenderView* render_view,
 std::string ChromeContentRendererClient::GetNavigationErrorHtml(
     const WebURLRequest& failed_request,
     const WebURLError& error) {
-  GURL failed_url = error.unreachableURL;
+  const GURL failed_url(error.unreachableURL);

[jam, 2011/10/05 17:12:06]

nit: the compiler is smart enough to call the copy constructor here, no need to
change this

[mkosiba (inactive), 2011/10/05 17:53:18]

Ok, I'm going to keep the const in there though.

   std::string html;
   const Extension* extension = NULL;
 
@@ -573,6 +573,25 @@ std::string ChromeContentRendererClient::GetNavigationErrorHtml(
   return html;
 }
 
+string16 ChromeContentRendererClient::GetNavigationErrorDescription(
+    const WebKit::WebURLRequest& failed_request,
+    const WebKit::WebURLError& error) {
+  const GURL failed_url = error.unreachableURL;

[mmenke, 2011/10/05 16:05:18]

nit:  const GURL failed_url(error.unreachableURL);

[mmenke, 2011/10/05 17:40:11]

This comment was made along the line of "if you're going to change one, change
both".  I defer to John's comment above.

[mkosiba (inactive), 2011/10/05 17:53:18]

Done.

+  const Extension* extension = NULL;
+  const bool is_repost =
+      error.reason == net::ERR_CACHE_MISS &&
+      error.domain == WebString::fromUTF8(net::kErrorDomain) &&
+      EqualsASCII(failed_request.httpMethod(), "POST");

[mmenke, 2011/10/05 16:05:18]

We now have this logic in 3 places:  Here, just above, and in RenderView, where
we call this function.  I think we should either add a function here to merge
both copies in this file, or maybe update the interface to take it as an
argument.

[mkosiba (inactive), 2011/10/05 17:53:18]

I don't think making that information a parameter is a good solution, since the
information can already be derived from the parameter itself.
Adding a field to WebURLError seems like overkill so I was thinking of adding a
static bool IsRepostInterstitial(const WebURLError&, const WebURLRequest&);  to
LocalizedError, though LocalizedError probably isn't the best place to put it.

Any ideas?

[mmenke, 2011/10/05 18:07:28]

If you end up merging the two functions, as per John's suggestion, I'm happy
with that.  You're right about it not really belonging in LocalizedError. 
content can't depend on LocalizedError, either, so we'd still need it in two
places in that case.  If you keep the functions separate, could just add a
function in an anonymous namespace in this file.  Not a great solution, but
think it'd be better than the duplicate logic.

+
+  if (failed_url.is_valid() && !failed_url.SchemeIs(chrome::kExtensionScheme))
+    extension = extension_dispatcher_->extensions()->GetByURL(failed_url);
+
+  if (!extension && !is_repost)
+    return LocalizedError::GetErrorDetails(error);
+  else
+    return string16();
+}
+
 bool ChromeContentRendererClient::RunIdleHandlerWhenWidgetsHidden() {
   return !extension_dispatcher_->is_extension_process();
 }