diff options
| -rw-r--r-- | chrome/browser/history/redirect_uitest.cc | 62 | ||||
| -rw-r--r-- | chrome/renderer/render_view.cc | 7 | ||||
| -rw-r--r-- | webkit/glue/webframeloaderclient_impl.cc | 16 | ||||
| -rw-r--r-- | webkit/glue/webview_delegate.h | 2 |
4 files changed, 81 insertions, 6 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); +} diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 519e37d..28a9195 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -1015,6 +1015,7 @@ void RenderView::UpdateURL(WebFrame* frame) { // the page contained a client redirect (meta refresh, document.loc...), // so we set the referrer and transition to match. if (completed_client_redirect_src_.is_valid()) { + DCHECK(completed_client_redirect_src_ == params.redirects[0]); params.referrer = completed_client_redirect_src_; params.transition = static_cast<PageTransition::Type>( params.transition | PageTransition::CLIENT_REDIRECT); @@ -1146,8 +1147,12 @@ void RenderView::DidStartProvisionalLoadForFrame( WebView* webview, WebFrame* frame, NavigationGesture gesture) { - if (webview->GetMainFrame() == frame) + if (webview->GetMainFrame() == frame) { navigation_gesture_ = gesture; + + // Make sure redirect tracking state is clear for the new load. + completed_client_redirect_src_ = GURL(); + } Send(new ViewHostMsg_DidStartProvisionalLoadForFrame( routing_id_, webview->GetMainFrame() == frame, 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) { |