Commit 7f78c3d9 authored by erg@chromium.org's avatar erg@chromium.org

Revert 269361 "Fix WebURLLoaderImpl::Context leak if a pending r..."

This is currently causing a top crasher; tfarina@ rebuilt on a branch
without this patch and have confirmed that reverting it fixes the crash.

Reverting at the request of the TPMs.

> Fix WebURLLoaderImpl::Context leak if a pending request is canceled.
> 
> Now, ResourceDispatcher::CancelPendingRequest will send
> ResourceHostMsg_CancelRequest and trigger OnRequestComplete. However,
> the request is pending and thus OnRequestComplete will be queued.
> 
> There are at least two problems if OnRequestComplete is not called:
> 1. WebURLLoaderImpl::Context will never be released.
> 2. request_info->buffer leaks. The buffer holds shared memory handle
>    (file descriptor). fd leaking may lead to HW video decode failure on
>    ChromeOS (detail in chrome-os-partner:27911#63)
> 
> This issue is similar to issue 328092.
> 
> BUG=chrome-os-partner:27911,chromium:369221,chromium:369128
> R=jam@chromium.org, mmenke@chromium.org
> 
> Review URL: https://codereview.chromium.org/268423002

BUG=372342
TBR=kcwu@chromium.org

Review URL: https://codereview.chromium.org/293823002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271504 0039d316-1c4b-4281-b951-d872f2087c98
parent 6f92f7a8
...@@ -293,18 +293,6 @@ bool ResourceDispatcher::OnMessageReceived(const IPC::Message& message) { ...@@ -293,18 +293,6 @@ bool ResourceDispatcher::OnMessageReceived(const IPC::Message& message) {
return true; return true;
} }
// If the request has been canceled, only dispatch
// ResourceMsg_RequestComplete (otherwise resource leaks) and drop other
// messages.
if (request_info->is_canceled) {
if (message.type() == ResourceMsg_RequestComplete::ID) {
DispatchMessage(message);
} else {
ReleaseResourcesInDataMessage(message);
}
return true;
}
if (request_info->is_deferred) { if (request_info->is_deferred) {
request_info->deferred_message_queue.push_back(new IPC::Message(message)); request_info->deferred_message_queue.push_back(new IPC::Message(message));
return true; return true;
...@@ -605,46 +593,6 @@ void ResourceDispatcher::CancelPendingRequest(int request_id) { ...@@ -605,46 +593,6 @@ void ResourceDispatcher::CancelPendingRequest(int request_id) {
return; return;
} }
PendingRequestInfo& request_info = it->second;
request_info.is_canceled = true;
// Because message handlers could result in request_info being destroyed,
// we need to work with a stack reference to the deferred queue.
MessageQueue queue;
queue.swap(request_info.deferred_message_queue);
// Removes pending requests. If ResourceMsg_RequestComplete was queued,
// dispatch it.
bool first_message = true;
while (!queue.empty()) {
IPC::Message* message = queue.front();
if (message->type() == ResourceMsg_RequestComplete::ID) {
if (first_message) {
// Dispatch as-is.
DispatchMessage(*message);
} else {
// If we skip some ResourceMsg_DataReceived and then dispatched the
// original ResourceMsg_RequestComplete(status=success), chrome will
// crash because it entered an unexpected state. So replace
// ResourceMsg_RequestComplete with failure status.
ResourceMsg_RequestCompleteData request_complete_data;
request_complete_data.error_code = net::ERR_ABORTED;
request_complete_data.was_ignored_by_handler = false;
request_complete_data.exists_in_cache = false;
request_complete_data.completion_time = base::TimeTicks();
request_complete_data.encoded_data_length = 0;
ResourceMsg_RequestComplete error_message(request_id,
request_complete_data);
DispatchMessage(error_message);
}
} else {
ReleaseResourcesInDataMessage(*message);
}
first_message = false;
queue.pop_front();
delete message;
}
// |request_id| will be removed from |pending_requests_| when // |request_id| will be removed from |pending_requests_| when
// OnRequestComplete returns with ERR_ABORTED. // OnRequestComplete returns with ERR_ABORTED.
message_sender()->Send(new ResourceHostMsg_CancelRequest(request_id)); message_sender()->Send(new ResourceHostMsg_CancelRequest(request_id));
...@@ -684,7 +632,6 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo() ...@@ -684,7 +632,6 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo()
: peer(NULL), : peer(NULL),
resource_type(ResourceType::SUB_RESOURCE), resource_type(ResourceType::SUB_RESOURCE),
is_deferred(false), is_deferred(false),
is_canceled(false),
download_to_file(false), download_to_file(false),
blocked_response(false), blocked_response(false),
buffer_size(0) { buffer_size(0) {
...@@ -701,7 +648,6 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo( ...@@ -701,7 +648,6 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo(
resource_type(resource_type), resource_type(resource_type),
origin_pid(origin_pid), origin_pid(origin_pid),
is_deferred(false), is_deferred(false),
is_canceled(false),
url(request_url), url(request_url),
frame_origin(frame_origin), frame_origin(frame_origin),
response_url(request_url), response_url(request_url),
......
...@@ -113,7 +113,6 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener { ...@@ -113,7 +113,6 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener {
int origin_pid; int origin_pid;
MessageQueue deferred_message_queue; MessageQueue deferred_message_queue;
bool is_deferred; bool is_deferred;
bool is_canceled;
// Original requested url. // Original requested url.
GURL url; GURL url;
// The security origin of the frame that initiates this request. // The security origin of the frame that initiates this request.
......
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