Send over the HTTP status code in the ViewHostMsg_FrameNavigate message coming in from the renderer.

A failed HTTP navigation like a 404 response to a request is followed by two responses. The first one which is associated with the failed response. This does not send over any information about the failure to the browser and thus appears as a normal navigation.The second response is for the actual 404 page being loaded. 

For network errors the browser does get notified via RenderView::DidFailProvisionalLoadWithError. However due to a prototype mismatch the corresponding function in WebContents is never invoked.

Added a new automation message AutomationMsg_NavigationFailed, which carries information about failed navigations to automation clients.The changes to the navigation controller include sending over the http status code and the URL to observers.

The ExternalTabContainer also subscribes to the FAIL_PROVISIONAL_LOAD_WITH_ERROR notification, so it can inform clients about errors. We also ignore the next NAV_ENTRY_COMMITTED notification after an error due to the reasons mentioned above.


Review URL: http://codereview.chromium.org/21495

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10023 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 83 insertions, 18 deletions

diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index 8491011..59dd51d 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -8,6 +8,7 @@
 #include "base/win_util.h"
 #include "chrome/browser/automation/automation_provider.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/tab_contents/provisional_load_details.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/views/tab_contents_container_view.h"
 #include "chrome/browser/tab_contents/web_contents.h"
@@ -29,7 +30,8 @@ ExternalTabContainer::ExternalTabContainer(
       tab_contents_(NULL),
       external_accel_table_(NULL),
       external_accel_entry_count_(0),
-      tab_contents_container_(NULL) {
+      tab_contents_container_(NULL),
+      ignore_next_load_notification_(false) {
 }
 
 ExternalTabContainer::~ExternalTabContainer() {
@@ -95,6 +97,8 @@ bool ExternalTabContainer::Init(Profile* profile, HWND parent,
   DCHECK(controller);
   registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
                  Source<NavigationController>(controller));
+  registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR,
+                 Source<NavigationController>(controller));
   NotificationService::current()->Notify(
       NotificationType::EXTERNAL_TAB_CREATED,
       Source<NavigationController>(controller),
@@ -251,21 +255,48 @@ void ExternalTabContainer::ForwardMessageToExternalHost(
 void ExternalTabContainer::Observe(NotificationType type,
                                    const NotificationSource& source,
                                    const NotificationDetails& details) {
+  static const int kHttpClientErrorStart = 400;
+  static const int kHttpServerErrorEnd = 510;
+
   switch (type.value) {
     case NotificationType::NAV_ENTRY_COMMITTED:
+      if (ignore_next_load_notification_) {
+        ignore_next_load_notification_ = false;
+        return;
+      }
+
       if (automation_) {
         const NavigationController::LoadCommittedDetails* commit =
             Details<NavigationController::LoadCommittedDetails>(details).ptr();
 
-        // When the previous entry index is invalid, it will be -1, which will
-        // still make the computation come out right (navigating to the 0th
-        // entry will be +1).
-        automation_->Send(new AutomationMsg_DidNavigate(
-            0, commit->type,
-            commit->previous_entry_index -
-                tab_contents_->controller()->GetLastCommittedEntryIndex()));
+        if (commit->http_status_code >= kHttpClientErrorStart && 
+            commit->http_status_code <= kHttpServerErrorEnd) {
+          automation_->Send(new AutomationMsg_NavigationFailed(
+              0, commit->http_status_code, commit->entry->url()));
+
+          ignore_next_load_notification_ = true;
+        } else {
+          // When the previous entry index is invalid, it will be -1, which
+          // will still make the computation come out right (navigating to the
+          // 0th entry will be +1).
+          automation_->Send(new AutomationMsg_DidNavigate(
+              0, commit->type,
+              commit->previous_entry_index -
+                  tab_contents_->controller()->GetLastCommittedEntryIndex()));
+        }
       }
       break;
+    case NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR: {
+      if (automation_) {
+        const ProvisionalLoadDetails* load_details =
+            Details<ProvisionalLoadDetails>(details).ptr();
+        automation_->Send(new AutomationMsg_NavigationFailed(
+            0, load_details->error_code(), load_details->url()));
+
+        ignore_next_load_notification_ = true;
+      }
+      break;
+    }
     default:
       NOTREACHED();
   }
diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h
index 7aaca7f..f82c40f 100644
--- a/chrome/browser/external_tab_container.h
+++ b/chrome/browser/external_tab_container.h
@@ -9,6 +9,7 @@
 #include <atlapp.h>
 #include <atlcrack.h>
 #include <atlmisc.h>
+#include <string>
 
 #include "base/basictypes.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
@@ -80,16 +81,18 @@ class ExternalTabContainer : public TabContentsDelegate,
   virtual void ToolbarSizeChanged(TabContents* source, bool is_animating);
   virtual void ForwardMessageToExternalHost(const std::string& receiver,
                                             const std::string& message);
-  virtual bool IsExternalTabContainer() const { return true; };
+  virtual bool IsExternalTabContainer() const {
+    return true;
+  };
 
   // Notification service callback.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  ////////////////////////////////////////////////////////////////////////////////
+  /////////////////////////////////////////////////////////////////////////////
   // views::Widget
-  ////////////////////////////////////////////////////////////////////////////////
+  /////////////////////////////////////////////////////////////////////////////
   virtual void GetBounds(gfx::Rect* out, bool including_frame) const;
   virtual void MoveToFront(bool should_activate);
   virtual HWND GetHWND() const;
@@ -115,7 +118,7 @@ class ExternalTabContainer : public TabContentsDelegate,
   // message it did not process
   void ProcessUnhandledAccelerator(const MSG& msg);
 
-  // See TabContents::SetInitialFocus	
+  // See TabContents::SetInitialFocus
   void SetInitialFocus(bool reverse);
 
   // A helper method that tests whether the given window is an
@@ -146,6 +149,12 @@ class ExternalTabContainer : public TabContentsDelegate,
   // A view to handle focus cycling
   TabContentsContainerView* tab_contents_container_;
  private:
+  // A failed navigation like a 404 is followed in chrome with a success
+  // navigation for the 404 page. We need to ignore the next navigation
+  // to avoid confusing the clients of the external tab. This member variable
+  // is set when we need to ignore the next load notification.
+  bool ignore_next_load_notification_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalTabContainer);
 };
 
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index fc3f2c5..91d0d84 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -614,6 +614,7 @@ bool NavigationController::RendererDidNavigate(
   details->is_main_frame = PageTransition::IsMainFrame(params.transition);
   details->serialized_security_info = params.security_info;
   details->is_content_filtered = params.is_content_filtered;
+  details->http_status_code = params.http_status_code;
   NotifyNavigationEntryCommitted(details);
 
   // It is now a safe time to schedule collection for any tab contents of a
diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h
index c1bf3fa..a7518c3 100644
--- a/chrome/browser/tab_contents/navigation_controller.h
+++ b/chrome/browser/tab_contents/navigation_controller.h
@@ -107,6 +107,9 @@ class NavigationController {
     bool is_user_initiated_main_frame_load() const {
       return !is_auto && !is_in_page && is_main_frame;
     }
+
+    // The HTTP status code for this entry..
+    int http_status_code;
   };
 
   // Details sent for NOTIFY_NAV_LIST_PRUNED.
diff --git a/chrome/browser/tab_contents/web_contents.cc b/chrome/browser/tab_contents/web_contents.cc
index 575714e..d04f44c 100644
--- a/chrome/browser/tab_contents/web_contents.cc
+++ b/chrome/browser/tab_contents/web_contents.cc
@@ -866,7 +866,8 @@ void WebContents::DidFailProvisionalLoadWithError(
     RenderViewHost* render_view_host,
     bool is_main_frame,
     int error_code,
-    const GURL& url) {
+    const GURL& url,
+    bool showing_repost_interstitial) {
   if (!controller())
     return;
 
diff --git a/chrome/browser/tab_contents/web_contents.h b/chrome/browser/tab_contents/web_contents.h
index c817ce0..c02292c 100644
--- a/chrome/browser/tab_contents/web_contents.h
+++ b/chrome/browser/tab_contents/web_contents.h
@@ -275,10 +275,12 @@ class WebContents : public TabContents,
                                           const GURL& target_url);
   virtual void DidLoadResourceFromMemoryCache(const GURL& url,
                                               const std::string& security_info);
-  virtual void DidFailProvisionalLoadWithError(RenderViewHost* render_view_host,
-                                               bool is_main_frame,
-                                               int error_code,
-                                               const GURL& url);
+  virtual void DidFailProvisionalLoadWithError(
+      RenderViewHost* render_view_host,
+      bool is_main_frame,
+      int error_code,
+      const GURL& url,
+      bool showing_repost_interstitial);
   virtual void UpdateFavIconURL(RenderViewHost* render_view_host,
                                 int32 page_id, const GURL& icon_url);
   virtual void DidDownloadImage(RenderViewHost* render_view_host,
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index c814b84..8c91097 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -123,6 +123,9 @@ struct ViewHostMsg_FrameNavigate_Params {
   // Whether the content of the frame was replaced with some alternate content
   // (this can happen if the resource was insecure).
   bool is_content_filtered;
+
+  // The status code of the HTTP request.
+  int http_status_code;
 };
 
 // Values that may be OR'd together to form the 'flags' parameter of a
@@ -746,6 +749,7 @@ struct ParamTraits<ViewHostMsg_FrameNavigate_Params> {
     WriteParam(m, p.contents_mime_type);
     WriteParam(m, p.is_post);
     WriteParam(m, p.is_content_filtered);
+    WriteParam(m, p.http_status_code);
   }
   static bool Read(const Message* m, void** iter, param_type* p) {
     return
@@ -763,7 +767,8 @@ struct ParamTraits<ViewHostMsg_FrameNavigate_Params> {
       ReadParam(m, iter, &p->gesture) &&
       ReadParam(m, iter, &p->contents_mime_type) &&
       ReadParam(m, iter, &p->is_post) &&
-      ReadParam(m, iter, &p->is_content_filtered);
+      ReadParam(m, iter, &p->is_content_filtered) &&
+      ReadParam(m, iter, &p->http_status_code);
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(L"(");
@@ -796,6 +801,8 @@ struct ParamTraits<ViewHostMsg_FrameNavigate_Params> {
     LogParam(p.is_post, l);
     l->append(L", ");
     LogParam(p.is_content_filtered, l);
+    l->append(L", ");
+    LogParam(p.http_status_code, l);
     l->append(L")");
   }
 };
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 4fa5bdf..10f314d 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1017,6 +1017,7 @@ void RenderView::UpdateURL(WebFrame* frame) {
       static_cast<RenderViewExtraRequestData*>(request.GetExtraData());
 
   ViewHostMsg_FrameNavigate_Params params;
+  params.http_status_code = response.GetHttpStatusCode();
   params.is_post = false;
   params.page_id = page_id_;
   params.is_content_filtered = response.IsContentFiltered();
diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h
index b2745b1..d5a244b 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -575,6 +575,15 @@ IPC_BEGIN_MESSAGES(Automation)
   //   None expected
   IPC_MESSAGE_ROUTED2(AutomationMsg_DidNavigate, int, int)
 
+  // This message is an outgoing message from Chrome to an external host.
+  // It is a notification that a navigation failed
+  // Request:
+  //   -int : The status code.
+  //   -GURL:  The URL we failed to navigate to.
+  // Response:
+  //   None expected
+  IPC_MESSAGE_ROUTED2(AutomationMsg_NavigationFailed, int, GURL)
+
   // This message requests the different security states of the page displayed
   // in the specified tab.
   // Request:
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 359d45c..7ef577a 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -256,6 +256,7 @@ class WebViewDelegate : virtual public WebWidgetDelegate {
   //  @discussion This method is called after the provisional data source has
   //  failed to load.  The frame will continue to display the contents of the
   //  committed data source if there is one.
+  //  This notification is only received for errors like network errors.
   virtual void DidFailProvisionalLoadWithError(WebView* webview,
                                                const WebError& error,
                                                WebFrame* frame) {