Fix a memory leak in ResourceDispatcher...

Fix a memory leak in ResourceDispatcher

When we delete a ResourceLoaderBridge before OnCompletedRequest is received, bad things happen. There's a lot of leaks at the following points:
1. OnMessageReceived ignores the message.
2. RemovePendingRequest removes it's internal deferred_message_queue.

But ViewHostMsg_Resource_DataReceived is not POD. We should also close the shared memory handle inside it.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=16297

[Alpha Left Google, 2009-05-15 09:07:18]

I'll also find the one who touched ResourceDispatcher the most to review this.

[Alpha Left Google, 2009-05-15 17:13:54]

May I should add a static method to SharedMemory to close a SharedMemoryHandle?

[scherkus (not reviewing), 2009-05-15 21:12:11]

brett you should really look over this too

http://codereview.chromium.org/115396/diff/11/1009
File base/shared_memory.h (right):

http://codereview.chromium.org/115396/diff/11/1009#newcode65
Line 65: static void CloseHandle(const SharedMemoryHandle& handle);
linux/mac implementation?

http://codereview.chromium.org/115396/diff/11/1008
File base/shared_memory_win.cc (right):

http://codereview.chromium.org/115396/diff/11/1008#newcode45
Line 45: CloseHandle(lock_);
I would change the destructor to use CloseHandle() as well

http://codereview.chromium.org/115396/diff/11/1007
File chrome/common/resource_dispatcher.cc (right):

http://codereview.chromium.org/115396/diff/11/1007#newcode281
Line 281: // handle or there will be a leak of memory.
nit: "leak of memory" -> "memory leak"

[brettw, 2009-05-15 21:20:16]

http://codereview.chromium.org/115396/diff/13/15
File base/shared_memory_win.cc (right):

http://codereview.chromium.org/115396/diff/13/15#newcode60
Line 60: if (IsHandleValid(handle)) {
Why do we need this check? Do we expect people calling it with NULL handles?
What do you think about a DCHECK that it is valid?

[Alpha Left Google, 2009-05-15 22:04:39]

http://codereview.chromium.org/115396/diff/13/15
File base/shared_memory_win.cc (right):

http://codereview.chromium.org/115396/diff/13/15#newcode60
Line 60: if (IsHandleValid(handle)) {
On 2009/05/15 21:20:16, brettw wrote:
> Why do we need this check? Do we expect people calling it with NULL handles?
> What do you think about a DCHECK that it is valid?

A DCHECK is more sensible.

[Alpha Left Google, 2009-05-15 23:28:47]

Added SharedMemory::CloseHandle() to posix. Also fixed one more leak in
ResourceDispatcher::RemovePendingRequest()

[brettw, 2009-05-15 23:33:14]

LGTM