Commit 8b3bca18 authored by pmeenan's avatar pmeenan Committed by Commit bot

Eliminate spurious request cancelations

This fixes the situation where every request is canceled even after it
has completed successfully.  The cancels were not harming anything and
were dropped on the floor but the additional messages were useless.

BUG=642172

Review-Url: https://codereview.chromium.org/2303733004
Cr-Commit-Position: refs/heads/master@{#417565}
parent 802aefed
...@@ -1762,13 +1762,13 @@ void ResourceDispatcherHostImpl::OnCancelRequest(int request_id) { ...@@ -1762,13 +1762,13 @@ void ResourceDispatcherHostImpl::OnCancelRequest(int request_id) {
return; return;
ResourceLoader* loader = GetLoader(child_id, request_id); ResourceLoader* loader = GetLoader(child_id, request_id);
if (!loader) {
// We probably want to remove this warning eventually, but I wanted to be // It is possible that the request has been completed and removed from the
// able to notice when this happens during initial development since it // loader queue but the client has not processed the request completed message
// should be rare and may indicate a bug. // before issuing a cancel. This happens frequently for beacons which are
DVLOG(1) << "Canceling a request that wasn't found"; // canceled in the response received handler.
if (!loader)
return; return;
}
loader->CancelRequest(true); loader->CancelRequest(true);
} }
......
...@@ -488,9 +488,10 @@ void ResourceDispatcher::Cancel(int request_id) { ...@@ -488,9 +488,10 @@ void ResourceDispatcher::Cancel(int request_id) {
should_dump = false; should_dump = false;
} }
} }
// Cancel the request, and clean it up so the bridge will receive no more // Cancel the request if it didn't complete, and clean it up so the bridge
// messages. // will receive no more messages.
message_sender_->Send(new ResourceHostMsg_CancelRequest(request_id)); if (info.completion_time.is_null())
message_sender_->Send(new ResourceHostMsg_CancelRequest(request_id));
RemovePendingRequest(request_id); RemovePendingRequest(request_id);
} }
......
...@@ -637,8 +637,8 @@ TEST_F(ResourceDispatcherTest, CancelDuringCallbackWithWrapperPeer) { ...@@ -637,8 +637,8 @@ TEST_F(ResourceDispatcherTest, CancelDuringCallbackWithWrapperPeer) {
NotifyRequestComplete(id, strlen(kTestPageContents)); NotifyRequestComplete(id, strlen(kTestPageContents));
EXPECT_TRUE(peer_context.received_response); EXPECT_TRUE(peer_context.received_response);
// Request should have been cancelled. // Request should have been cancelled with no additional messages.
ConsumeCancelRequest(id); EXPECT_EQ(0u, queued_messages());
EXPECT_TRUE(peer_context.cancelled); EXPECT_TRUE(peer_context.cancelled);
// Any future messages related to the request should be ignored. // Any future messages related to the request should be ignored.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment