Add error description to the DidFailProvisionalLoad callback.

chrome/renderer/chrome_content_renderer_client.cc

 // Copyright (c) 2011 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/renderer/chrome_content_renderer_client.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/message_loop.h"
@@ skipping 515 matching lines @@
     error.reason = http_status_code;
 
     render_view->LoadNavigationErrorPage(
         frame, frame->dataSource()->request(), error, std::string(), true);
   }
 }
 
 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;
 
   // Use a local error page.
   int resource_id;
   DictionaryValue error_strings;
   if (failed_url.is_valid() && !failed_url.SchemeIs(chrome::kExtensionScheme))
     extension = extension_dispatcher_->extensions()->GetByURL(failed_url);
   if (extension) {
     LocalizedError::GetAppErrorStrings(error, failed_url, extension,
@@ skipping 19 matching lines @@
     NOTREACHED() << "unable to load template. ID: " << resource_id;
   } else {
     // "t" is the id of the templates root node.
     html = jstemplate_builder::GetTemplatesHtml(
         template_html, &error_strings, "t");
   }
 
   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();
 }
 
 bool ChromeContentRendererClient::AllowPopup(const GURL& creator) {
   // Extensions and apps always allowed to create unrequested popups. The second
   // check is necessary to include content scripts.
   return extension_dispatcher_->extensions()->GetByURL(creator) ||
       ExtensionBindingsContext::GetCurrent();
 }
@@ skipping 184 matching lines @@
   if (spellcheck_.get())
     thread->RemoveObserver(spellcheck_.get());
   SpellCheck* new_spellcheck = new SpellCheck();
   if (spellcheck_provider_)
     spellcheck_provider_->SetSpellCheck(new_spellcheck);
   spellcheck_.reset(new_spellcheck);
   thread->AddObserver(new_spellcheck);
 }
 
 }  // namespace chrome