Fix DCHECK in history_backend by ensuring we clear redirect tracking state whenever the RenderView is told that a provisional load has started for the main frame.

The DCHECK(params->referrer == params->redirects[0]) was firing because:
a) page A was loaded, triggered WillPerformClientRedirect
b) after the provisional load started for the destination page of A's client redirect, but before this load was committed, the browser makes a Navigation request for page B.
c) When page B's load is committed, the RenderView's completed_client_redirect_src_ was still set, resulting in a CLIENT_REDIRECT transition type and forwarding the src value through params->referrer -- but params->redirects was now completely unrelated. Kaboom.

This fix should be general enough to handle cases (that are relatively likely in the wild) where WebKit legitimately cancels the redirect, instead of just the browser doing so. Note we can't depend on dispatchDidCancelClientRedirect because we get that callback on both completion and cancellation of a client redirect.

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

2 files changed, 14 insertions, 4 deletions

diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index 232230f..ec1ff4b 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -671,6 +671,7 @@ void WebFrameLoaderClient::dispatchDidStartProvisionalLoad() {
   // If this load is what we expected from a client redirect, treat it as a
   // redirect from that original page. The expected redirect urls will be
   // cleared by DidCancelClientRedirect.
+  bool completing_client_redirect = false;
   if (expected_client_redirect_src_.is_valid()) {
     // expected_client_redirect_dest_ could be something like 
     // "javascript:history.go(-1)" thus we need to exclude url starts with
@@ -678,15 +679,22 @@ void WebFrameLoaderClient::dispatchDidStartProvisionalLoad() {
     DCHECK(expected_client_redirect_dest_.SchemeIs("javascript") || 
            expected_client_redirect_dest_ == url);
     ds->AppendRedirect(expected_client_redirect_src_);
-    if (d)
-      d->DidCompleteClientRedirect(webview, webframe_, 
-                                   expected_client_redirect_src_);
+    completing_client_redirect = true;
   }
   ds->AppendRedirect(url);
 
-  if (d)
+  if (d) {
+    // As the comment for DidCompleteClientRedirect in webview_delegate.h
+    // points out, whatever information its invocation contains should only
+    // be considered relevant until the next provisional load has started.
+    // So we first tell the delegate that the load started, and then tell it
+    // about the client redirect the load is responsible for completing.
     d->DidStartProvisionalLoadForFrame(webview, webframe_,
                                        NavigationGestureForLastLoad());
+    if (completing_client_redirect)
+      d->DidCompleteClientRedirect(webview, webframe_, 
+                                   expected_client_redirect_src_);
+  }
 
   // Cancel any pending loads.
   if (alt_404_page_fetcher_.get())
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index f7aedd5..cf18ca5 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -404,6 +404,8 @@ class WebViewDelegate : virtual public WebWidgetDelegate {
 
   // Notifies the delegate that the load about to be committed for the specified
   // webview and frame was due to a client redirect originating from source URL.
+  // The information/notification obtained from this method is relevant until
+  // the next provisional load is started, at which point it becomes obsolete.
   virtual void DidCompleteClientRedirect(WebView* webview,
                                          WebFrame* frame,
                                          const GURL& source) {