Fix a crash where the ResourceMessageFilter is deleted

Fix a crash where the ResourceMessageFilter is deleted before a
SafeBrowsing check has completed. The problem occurs since the
SafeBrowsingResourceHandler is not deleted when its associated
URLRequest is cleaned up *and* a SafeBrowsing check is in progress.
When the check completes, the next resource handler in the chain
(the AsyncResourceHandler which caches a pointer the now deleted
ResourceMessageFilter) will crash.

This CL adds a notification for objects to know when the
ResourceMessageFilter is destroyed.

BUG=8544
(http://crbug.com)


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

[jam, 2009-04-13 21:00:25]

http://codereview.chromium.org/63036/diff/5003/5012
File chrome/browser/extensions/extension_message_service.cc (right):

http://codereview.chromium.org/63036/diff/5003/5012#newcode165
Line 165: // NotificationObserver interface.
nit: I think this is a little redundant.

http://codereview.chromium.org/63036/diff/5003/5012#newcode171
Line 171: RendererShutdown(filter);
since no one else is calling RendererShutdown() anymore, can you take it out of
the header, and you can just move that code here..

http://codereview.chromium.org/63036/diff/5003/5004
File chrome/browser/renderer_host/safe_browsing_resource_handler.cc (right):

http://codereview.chromium.org/63036/diff/5003/5004#newcode47
Line 47: static_cast<ResourceMessageFilter*>(receiver_)));
why is the ResourceDispatcherPointer casted to a ResourceMessageFilter??

http://codereview.chromium.org/63036/diff/5003/5006
File chrome/browser/renderer_host/safe_browsing_resource_handler.h (right):

http://codereview.chromium.org/63036/diff/5003/5006#newcode16
Line 16: class ResourceMessageFilter;
why is this needed?

http://codereview.chromium.org/63036/diff/5003/5010
File chrome/browser/worker_host/worker_service.cc (right):

http://codereview.chromium.org/63036/diff/5003/5010#newcode148
Line 148: RendererShutdown(filter);
same, since no one is calling this, can the function be removed from the header
and the code moved here?

[Paul Godavari, 2009-04-13 22:04:15]

http://codereview.chromium.org/63036/diff/5003/5012
File chrome/browser/extensions/extension_message_service.cc (right):

http://codereview.chromium.org/63036/diff/5003/5012#newcode165
Line 165: // NotificationObserver interface.
On 2009/04/13 21:00:25, John Abd-El-Malek wrote:
> nit: I think this is a little redundant.

Removed.

http://codereview.chromium.org/63036/diff/5003/5012#newcode171
Line 171: RendererShutdown(filter);
On 2009/04/13 21:00:25, John Abd-El-Malek wrote:
> since no one else is calling RendererShutdown() anymore, can you take it out
of
> the header, and you can just move that code here..

Done.

http://codereview.chromium.org/63036/diff/5003/5004
File chrome/browser/renderer_host/safe_browsing_resource_handler.cc (right):

http://codereview.chromium.org/63036/diff/5003/5004#newcode47
Line 47: static_cast<ResourceMessageFilter*>(receiver_)));
On 2009/04/13 21:00:25, John Abd-El-Malek wrote:
> why is the ResourceDispatcherPointer casted to a ResourceMessageFilter??

'receiver_' is a ResourceDispatcherHost::Receiver*, which is really a
ResourceMessageFilter in the context of the resource handlers. I want to be
consistent with the notification source type.

http://codereview.chromium.org/63036/diff/5003/5006
File chrome/browser/renderer_host/safe_browsing_resource_handler.h (right):

http://codereview.chromium.org/63036/diff/5003/5006#newcode16
Line 16: class ResourceMessageFilter;
On 2009/04/13 21:00:25, John Abd-El-Malek wrote:
> why is this needed?

I forgot to remove this in the last update.

http://codereview.chromium.org/63036/diff/5003/5010
File chrome/browser/worker_host/worker_service.cc (right):

http://codereview.chromium.org/63036/diff/5003/5010#newcode148
Line 148: RendererShutdown(filter);
On 2009/04/13 21:00:25, John Abd-El-Malek wrote:
> same, since no one is calling this, can the function be removed from the
header
> and the code moved here?

Done.

[jam, 2009-04-13 22:48:43]

lgtm

[Matt Perry, 2009-04-17 23:31:02]

http://codereview.chromium.org/63036/diff/3036/3045
File chrome/browser/extensions/extension_message_service.cc (right):

http://codereview.chromium.org/63036/diff/3036/3045#newcode137
Line 137: NotificationService::current()->AddObserver(
This doesn't work.  RendererReady is called on the UI thread, but the
notification is sent on the IO thread.