Fix WebURLLoaderImpl::Context leak if a pending request is canceled.

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269361

[kcwu, 2014-05-07 08:02:14]

The only difference to previous changeset is in
ResourceDispatcher::CancelPendingRequest.

[jam, 2014-05-07 15:44:57]

On 2014/05/07 08:02:14, kcwu wrote:
> The only difference to previous changeset is in
> ResourceDispatcher::CancelPendingRequest.

please upload the original reverted change, and then upload with the fix. that
way the reviewer can diff what got changed.

[kcwu1, 2014-05-08 02:37:50]

On 2014/05/07 15:44:57, jam wrote:
> On 2014/05/07 08:02:14, kcwu wrote:
> > The only difference to previous changeset is in
> > ResourceDispatcher::CancelPendingRequest.
> 
> please upload the original reverted change, and then upload with the fix. that
> way the reviewer can diff what got changed.

done.

[jam, 2014-05-08 16:22:13]

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
content/child/resource_dispatcher.cc:632: DispatchMessage(error_message);
i don't understand this change. why are you dispatching this synthesized IPC
over and over?

[kcwu1, 2014-05-08 17:00:48]

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
content/child/resource_dispatcher.cc:632: DispatchMessage(error_message);
On 2014/05/08 16:22:13, jam wrote:
> i don't understand this change. why are you dispatching this synthesized IPC
> over and over?

What do you mean "over and over"?

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 I replace ResourceMsg_RequestComplete with
failure status.

[jam, 2014-05-08 17:03:52]

lgtm

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
content/child/resource_dispatcher.cc:632: DispatchMessage(error_message);
On 2014/05/08 17:00:49, kcwu1 wrote:
> On 2014/05/08 16:22:13, jam wrote:
> > i don't understand this change. why are you dispatching this synthesized IPC
> > over and over?
> 
> What do you mean "over and over"?
> 
> 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 I replace ResourceMsg_RequestComplete with
> failure status.

ah, I missed that we're only doing this for the RequestComplete which would only
be received once. thanks for the explanation.

can you add the paragraph from the comment into line 622? that's clearer than
the comment there

[kcwu1, 2014-05-08 17:14:53]

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/268423002/diff/60001/content/child/resource_d...
content/child/resource_dispatcher.cc:632: DispatchMessage(error_message);
On 2014/05/08 17:03:52, jam wrote:
> On 2014/05/08 17:00:49, kcwu1 wrote:
> > On 2014/05/08 16:22:13, jam wrote:
> > > i don't understand this change. why are you dispatching this synthesized
IPC
> > > over and over?
> > 
> > What do you mean "over and over"?
> > 
> > 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 I replace ResourceMsg_RequestComplete with
> > failure status.
> 
> ah, I missed that we're only doing this for the RequestComplete which would
only
> be received once. thanks for the explanation.
> 
> can you add the paragraph from the comment into line 622? that's clearer than
> the comment there

Done.
Replaced the comment

[kcwu1, 2014-05-08 17:25:27]

The CQ bit was checked by kcwu@google.com

[commit-bot: I haz the power, 2014-05-08 17:28:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kcwu@chromium.org/268423002/80001

[mmenke, 2014-05-08 17:53:59]

LGTM (Largely deferring to jam on this)

https://codereview.chromium.org/268423002/diff/80001/content/child/resource_d...
File content/child/resource_dispatcher.cc (right):

https://codereview.chromium.org/268423002/diff/80001/content/child/resource_d...
content/child/resource_dispatcher.cc:614: IPC::Message* message = queue.front();
optional:  I think it would be a little cleaner to do:

scoped_ptr<IPC::Message> message = queue.front();
queue.pop_front();

And then get rid of the delete in the end.  It makes it clearer that it's
deleted on all codepaths.

[commit-bot: I haz the power, 2014-05-08 22:22:08]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-09 17:30:16]

Change committed as 269361