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 /chrome/common/resource_dispatcher.cc | |
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 'chrome/common/resource_dispatcher.cc')
-rw-r--r-- | chrome/common/resource_dispatcher.cc | 50 |
1 files changed, 37 insertions, 13 deletions
diff --git a/chrome/common/resource_dispatcher.cc b/chrome/common/resource_dispatcher.cc index d598b39..8167901 100644 --- a/chrome/common/resource_dispatcher.cc +++ b/chrome/common/resource_dispatcher.cc @@ -56,7 +56,7 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge { ResourceType::Type resource_type, uint32 request_context, int app_cache_context_id, - int route_id); + int routing_id); virtual ~IPCResourceLoaderBridge(); // ResourceLoaderBridge @@ -88,7 +88,7 @@ class IPCResourceLoaderBridge : public ResourceLoaderBridge { int request_id_; // The routing id used when sending IPC messages. - int route_id_; + int routing_id_; #ifdef LOG_RESOURCE_REQUESTS // indicates the URL of this resource request for help debugging @@ -110,11 +110,11 @@ IPCResourceLoaderBridge::IPCResourceLoaderBridge( ResourceType::Type resource_type, uint32 request_context, int app_cache_context_id, - int route_id) + int routing_id) : peer_(NULL), dispatcher_(dispatcher), request_id_(-1), - route_id_(route_id) { + routing_id_(routing_id) { DCHECK(dispatcher_) << "no resource dispatcher"; request_.method = method; request_.url = url; @@ -189,7 +189,7 @@ bool IPCResourceLoaderBridge::Start(Peer* peer) { request_id_ = dispatcher_->AddPendingRequest(peer_, request_.resource_type); return dispatcher_->message_sender()->Send( - new ViewHostMsg_RequestResource(route_id_, request_id_, request_)); + new ViewHostMsg_RequestResource(routing_id_, request_id_, request_)); } void IPCResourceLoaderBridge::Cancel() { @@ -200,8 +200,7 @@ void IPCResourceLoaderBridge::Cancel() { RESOURCE_LOG("Canceling request for " << url_); - dispatcher_->message_sender()->Send( - new ViewHostMsg_CancelRequest(route_id_, request_id_)); + dispatcher_->CancelPendingRequest(routing_id_, request_id_); // We can't remove the request ID from the resource dispatcher because more // data might be pending. Sending the cancel message may cause more data @@ -229,7 +228,7 @@ void IPCResourceLoaderBridge::SyncLoad(SyncLoadResponse* response) { request_id_ = MakeRequestID(); SyncLoadResult result; - IPC::Message* msg = new ViewHostMsg_SyncLoad(route_id_, request_id_, + IPC::Message* msg = new ViewHostMsg_SyncLoad(routing_id_, request_id_, request_, &result); if (!dispatcher_->message_sender()->Send(msg)) { response->status.set_status(URLRequestStatus::FAILED); @@ -351,7 +350,7 @@ void ResourceDispatcher::OnReceivedData(const IPC::Message& message, int request_id, base::SharedMemoryHandle shm_handle, int data_len) { - // Acknowlegde the reception of this data. + // Acknowledge the reception of this data. message_sender()->Send( new ViewHostMsg_DataReceived_ACK(message.routing_id(), request_id)); @@ -377,8 +376,11 @@ void ResourceDispatcher::OnReceivedData(const IPC::Message& message, } } -void ResourceDispatcher::OnReceivedRedirect(int request_id, - const GURL& new_url) { +void ResourceDispatcher::OnReceivedRedirect( + const IPC::Message& message, + int request_id, + const GURL& new_url, + const webkit_glue::ResourceLoaderBridge::ResponseInfo& info) { PendingRequestList::iterator it = pending_requests_.find(request_id); if (it == pending_requests_.end()) { // this might happen for kill()ed requests on the webkit end, so perhaps @@ -391,7 +393,13 @@ void ResourceDispatcher::OnReceivedRedirect(int request_id, RESOURCE_LOG("Dispatching redirect for " << request_info.peer->GetURLForDebugging()); - request_info.peer->OnReceivedRedirect(new_url); + + if (request_info.peer->OnReceivedRedirect(new_url, info)) { + message_sender()->Send( + new ViewHostMsg_FollowRedirect(message.routing_id(), request_id)); + } else { + CancelPendingRequest(message.routing_id(), request_id); + } } void ResourceDispatcher::OnRequestComplete(int request_id, @@ -461,10 +469,26 @@ bool ResourceDispatcher::RemovePendingRequest(int request_id) { return true; } +void ResourceDispatcher::CancelPendingRequest(int routing_id, + int request_id) { + PendingRequestList::iterator it = pending_requests_.find(request_id); + if (it == pending_requests_.end()) { + DLOG(ERROR) << "unknown request"; + return; + } + PendingRequestInfo& request_info = it->second; + // Avoid spamming the host with cancel messages. + if (request_info.is_cancelled) + return; + request_info.is_cancelled = true; + message_sender()->Send( + new ViewHostMsg_CancelRequest(routing_id, request_id)); +} + void ResourceDispatcher::SetDefersLoading(int request_id, bool value) { PendingRequestList::iterator it = pending_requests_.find(request_id); if (it == pending_requests_.end()) { - NOTREACHED() << "unknown request"; + DLOG(ERROR) << "unknown request"; return; } PendingRequestInfo& request_info = it->second; |