diff options
author | darin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-30 18:01:39 +0000 |
---|---|---|
committer | darin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-30 18:01:39 +0000 |
commit | 6568a9e384e0f92422c68d4f31fb401df4acbaed (patch) | |
tree | 477e1e2de4dd34d79e47a8a1e3b922ff3cecd83a /webkit/glue | |
parent | ae5ca890de8d9f1a91f5198741c76f375146693b (diff) | |
download | chromium_src-6568a9e384e0f92422c68d4f31fb401df4acbaed.zip chromium_src-6568a9e384e0f92422c68d4f31fb401df4acbaed.tar.gz chromium_src-6568a9e384e0f92422c68d4f31fb401df4acbaed.tar.bz2 |
Add plumbing for allowing the renderer to intercept and cancel redirects before
they are sent.
A good portion of this CL is to support the new UI test.
The IPC to notify the renderer of a redirect now includes a ResponseInfo struct
allowing WebURLLoaderImpl to provide detailed response info (including response
headers) to WebKit. This isn't strictly necessary, but I thought I'd include
this to make the code more future proof.
A cross origin restriction is added to SyncResourceHandler::OnRequestRedirected
that mimics the code in WebCore/platform/network/cf/ResourceHandleCFNet.cpp.
This is most unfortunate, and I filed a bug at bugs.webkit.org about the
similar duplication of logic in WebCore.
There seemed to be enough code paths leading to request cancellation at the
ResourceDispatcher level that I couldn't easily ensure that a request only gets
cancelled once. So, I added an is_cancelled flag to record if it is not
necessary to send a ViewHostMsg_CancelRequest IPC. This avoids some warnings
in the ResourceDispatcherHost.
To support my UI test, I needed to change URLRequestMockHttpJob to know how to
serve redirects. I moved URLRequestHttpJob::IsRedirectResponse to its base
class, URLRequestJob so that the implementation could be shared. This revealed
a minor bug in URLRequest. We were never resetting response_info_ upon
following a redirect. I added this code consolidated similar code from
URLRequest::Redirect and URLRequest::RestartWithJob into a new PrepareToRestart
method.
To support my UI test, I added a "hit count" field to URLRequestFilter, and I
added an associated automation IPC to query the value. The test was a bit
challenging to write because there is no way to tell the difference from JS.
Before and after, it appears to JS as though the cross-origin redirect failed.
However, the server can see the extra redirect request. So, I simply record
the number of hits against URLs of the form http://mock.http/foo, and use that
to observe if any extra requests were made. I implemented the new IPC message
by extending the AutomationResourceMessageFilter. This allowed me to trap the
IPC message on the IO thread where it is safe to probe the URLRequestFilter. I
then changed the implementation of AutomationMsg_SetFilteredInet to work
similarly.
I revised URLRequestMockHTTPJob::GetOnDiskPath to support ports. This actually
allowed me to reuse URLRequestMockHTTPJob to service URLs in different security
origins. My test redirects from http://mock.http/ to http://mock.http:4000/.
Please see the comments in cross-origin-redirect-blocked.html for details about
how the test functions.
R=brettw,wtc
BUG=16413
TEST=covered by resource_dispatcher_host_uitest.cc
Review URL: http://codereview.chromium.org/159370
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22067 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'webkit/glue')
-rw-r--r-- | webkit/glue/media/buffered_data_source.cc | 6 | ||||
-rw-r--r-- | webkit/glue/media/buffered_data_source.h | 4 | ||||
-rw-r--r-- | webkit/glue/media/simple_data_source.cc | 5 | ||||
-rw-r--r-- | webkit/glue/media/simple_data_source.h | 4 | ||||
-rw-r--r-- | webkit/glue/resource_loader_bridge.h | 8 | ||||
-rw-r--r-- | webkit/glue/weburlloader_impl.cc | 37 |
6 files changed, 41 insertions, 23 deletions
diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc index 4fc3549..738f30d 100644 --- a/webkit/glue/media/buffered_data_source.cc +++ b/webkit/glue/media/buffered_data_source.cc @@ -194,7 +194,9 @@ void BufferedResourceLoader::Read(int64 position, ///////////////////////////////////////////////////////////////////////////// // BufferedResourceLoader, // webkit_glue::ResourceLoaderBridge::Peer implementations -void BufferedResourceLoader::OnReceivedRedirect(const GURL& new_url) { +bool BufferedResourceLoader::OnReceivedRedirect( + const GURL& new_url, + const webkit_glue::ResourceLoaderBridge::ResponseInfo& info) { DCHECK(bridge_.get()); DCHECK(start_callback_.get()); @@ -206,6 +208,8 @@ void BufferedResourceLoader::OnReceivedRedirect(const GURL& new_url) { DoneStart(net::ERR_ADDRESS_INVALID); Stop(); } + + return true; } void BufferedResourceLoader::OnReceivedResponse( diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h index 6dd51a7..ef46477 100644 --- a/webkit/glue/media/buffered_data_source.h +++ b/webkit/glue/media/buffered_data_source.h @@ -86,7 +86,9 @@ class BufferedResourceLoader : public webkit_glue::ResourceLoaderBridge::Peer { ///////////////////////////////////////////////////////////////////////////// // webkit_glue::ResourceLoaderBridge::Peer implementations. virtual void OnUploadProgress(uint64 position, uint64 size) {} - virtual void OnReceivedRedirect(const GURL& new_url); + virtual bool OnReceivedRedirect( + const GURL& new_url, + const webkit_glue::ResourceLoaderBridge::ResponseInfo& info); virtual void OnReceivedResponse( const webkit_glue::ResourceLoaderBridge::ResponseInfo& info, bool content_filtered); diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc index 48c7138..639aa5f 100644 --- a/webkit/glue/media/simple_data_source.cc +++ b/webkit/glue/media/simple_data_source.cc @@ -109,8 +109,11 @@ void SimpleDataSource::OnDownloadProgress(uint64 position, uint64 size) {} void SimpleDataSource::OnUploadProgress(uint64 position, uint64 size) {} -void SimpleDataSource::OnReceivedRedirect(const GURL& new_url) { +bool SimpleDataSource::OnReceivedRedirect( + const GURL& new_url, + const webkit_glue::ResourceLoaderBridge::ResponseInfo& info) { SetURL(new_url); + return true; } void SimpleDataSource::OnReceivedResponse( diff --git a/webkit/glue/media/simple_data_source.h b/webkit/glue/media/simple_data_source.h index acfc503..e458c57 100644 --- a/webkit/glue/media/simple_data_source.h +++ b/webkit/glue/media/simple_data_source.h @@ -49,7 +49,9 @@ class SimpleDataSource : public media::DataSource, // webkit_glue::ResourceLoaderBridge::Peer implementation. virtual void OnDownloadProgress(uint64 position, uint64 size); virtual void OnUploadProgress(uint64 position, uint64 size); - virtual void OnReceivedRedirect(const GURL& new_url); + virtual bool OnReceivedRedirect( + const GURL& new_url, + const webkit_glue::ResourceLoaderBridge::ResponseInfo& info); virtual void OnReceivedResponse( const webkit_glue::ResourceLoaderBridge::ResponseInfo& info, bool content_filtered); diff --git a/webkit/glue/resource_loader_bridge.h b/webkit/glue/resource_loader_bridge.h index 2d7ba77..b76fd56 100644 --- a/webkit/glue/resource_loader_bridge.h +++ b/webkit/glue/resource_loader_bridge.h @@ -105,8 +105,12 @@ class ResourceLoaderBridge { // note: only for requests with LOAD_ENABLE_UPLOAD_PROGRESS set virtual void OnUploadProgress(uint64 position, uint64 size) = 0; - // Called when a redirect occurs. - virtual void OnReceivedRedirect(const GURL& new_url) = 0; + // Called when a redirect occurs. The implementation may return false to + // suppress the redirect. The given ResponseInfo provides complete + // information about the redirect, and new_url is the URL that will be + // loaded if this method returns true. + virtual bool OnReceivedRedirect(const GURL& new_url, + const ResponseInfo& info) = 0; // Called when response headers are available (after all redirects have // been followed). |content_filtered| is set to true if the contents is diff --git a/webkit/glue/weburlloader_impl.cc b/webkit/glue/weburlloader_impl.cc index d0158f6..3041049 100644 --- a/webkit/glue/weburlloader_impl.cc +++ b/webkit/glue/weburlloader_impl.cc @@ -205,7 +205,8 @@ class WebURLLoaderImpl::Context : public base::RefCounted<Context>, // ResourceLoaderBridge::Peer methods: virtual void OnUploadProgress(uint64 position, uint64 size); - virtual void OnReceivedRedirect(const GURL& new_url); + virtual bool OnReceivedRedirect( + const GURL& new_url, const ResourceLoaderBridge::ResponseInfo& info); virtual void OnReceivedResponse( const ResourceLoaderBridge::ResponseInfo& info, bool content_filtered); virtual void OnReceivedData(const char* data, int len); @@ -371,30 +372,32 @@ void WebURLLoaderImpl::Context::OnUploadProgress(uint64 position, uint64 size) { client_->didSendData(loader_, position, size); } -void WebURLLoaderImpl::Context::OnReceivedRedirect(const GURL& new_url) { +bool WebURLLoaderImpl::Context::OnReceivedRedirect( + const GURL& new_url, + const ResourceLoaderBridge::ResponseInfo& info) { if (!client_) - return; + return false; - // TODO(darin): We lack sufficient information to construct the - // actual redirect response, so we just simulate it here. - WebURLResponse response(url_); + WebURLResponse response; + response.initialize(); + PopulateURLResponse(url_, info, &response); - // TODO(darin): We lack sufficient information to construct the - // actual request that resulted from the redirect, so we just - // report a GET navigation to the new location. + // TODO(darin): We lack sufficient information to construct the actual + // request that resulted from the redirect, so we just report a GET + // navigation to the new location. WebURLRequest new_request(new_url); url_ = new_url; client_->willSendRequest(loader_, new_request, response); - // TODO(darin): since new_request is sent as a mutable reference, it is - // possible that willSendRequest may have modified it. - // - // andresca on #webkit confirms that that is intentional, so we'll need - // to rework the ResourceLoaderBridge to give us control over what URL - // is really loaded (and with what headers) when a redirect is encountered. - // TODO(darin): we fail this assertion in some layout tests! - //DCHECK(GURL(new_request.url()) == new_url); + // Only follow the redirect if WebKit left the URL unmodified. + if (url_ == new_request.url()) + return true; + + // We assume that WebKit only changes the URL to suppress a redirect, and we + // assume that it does so by setting it to be invalid. + DCHECK(!new_request.url().isValid()); + return false; } void WebURLLoaderImpl::Context::OnReceivedResponse( |