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

1 files changed, 61 insertions, 1 deletions

diff --git a/chrome/browser/history/redirect_uitest.cc b/chrome/browser/history/redirect_uitest.cc
index 0ee58f7..dabeddb 100644
--- a/chrome/browser/history/redirect_uitest.cc
+++ b/chrome/browser/history/redirect_uitest.cc
@@ -251,4 +251,64 @@ TEST_F(RedirectTest, ClientFragments) {
 
   EXPECT_EQ(1, redirects.size());
   EXPECT_EQ(first_url.spec() + "#myanchor", redirects[0].spec());
-}
\ No newline at end of file
+}
+
+// TODO(timsteele): This is disabled because our current testserver can't
+// handle multiple requests in parallel, making it hang on the first request to
+// /slow?60. It's unable to serve our second request for files/title2.html until
+// /slow? completes, which doesn't give the desired behavior. We could
+// alternatively load the second page from disk, but we would need to start the
+// browser for this testcase with --process-per-tab, and I don't think we can do
+// this at test-case-level granularity at the moment.
+TEST_F(RedirectTest, DISABLED_ClientCancelledByNewNavigationAfterProvisionalLoad) {
+  // We want to initiate a second navigation after the provisional load for
+  // the client redirect destination has started, but before this load is
+  // committed. To achieve this, we tell the browser to load a slow page,
+  // which causes it to start a provisional load, and while it is waiting
+  // for the response (which means it hasn't committed the load for the client
+  // redirect destination page yet), we issue a new navigation request.
+  TestServer server(kDocRoot);
+  
+  GURL final_url = server.TestServerPageW(std::wstring(L"files/title2.html"));
+  GURL slow = server.TestServerPageW(std::wstring(L"slow?60"));
+  GURL first_url = server.TestServerPageW(
+      std::wstring(L"client-redirect?") + UTF8ToWide(slow.spec()));
+  std::vector<GURL> redirects;
+
+  NavigateToURL(first_url);
+  // We don't sleep here - the first navigation won't have been committed yet
+  // because we told the server to wait a minute. This means the browser has
+  // started it's provisional load for the client redirect destination page but
+  // hasn't completed. Our time is now!
+  NavigateToURL(final_url);
+  
+  std::wstring tab_title;
+  std::wstring final_url_title = L"Title Of Awesomeness";
+  // Wait till the final page has been loaded.
+  for (int i = 0; i < 10; ++i) {
+    Sleep(kWaitForActionMaxMsec / 10);
+    scoped_ptr<TabProxy> tab_proxy(GetActiveTab());
+    ASSERT_TRUE(tab_proxy.get());
+    ASSERT_TRUE(tab_proxy->GetTabTitle(&tab_title));
+    if (tab_title == final_url_title) {
+      ASSERT_TRUE(tab_proxy->GetRedirectsFrom(first_url, &redirects));
+      break;
+    }
+  }
+
+  // Check to make sure the navigation did in fact take place and we are 
+  // at the expected page.
+  EXPECT_EQ(final_url_title, tab_title); 
+
+  bool final_navigation_not_redirect = true;
+  // Check to make sure our request for files/title2.html doesn't get flagged
+  // as a client redirect from the first (/client-redirect?) page.
+  for (std::vector<GURL>::iterator it = redirects.begin();
+       it != redirects.end(); ++it) {
+    if (final_url.spec() == it->spec()) {
+      final_navigation_not_redirect = false;
+      break;
+    }
+  }
+  EXPECT_TRUE(final_navigation_not_redirect);
+}