summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authorpaulg@google.com <paulg@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2009-04-14 02:03:20 +0000
committerpaulg@google.com <paulg@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2009-04-14 02:03:20 +0000
commit05349986a0c968101b8925be496794b1f2bc40f5 (patch)
tree7cb79c84d92e7e5910664e94979debb9980b3e3d /chrome
parent3401b00e41731a8816d9aec9592b09efb4bf5eac (diff)
downloadchromium_src-05349986a0c968101b8925be496794b1f2bc40f5.zip
chromium_src-05349986a0c968101b8925be496794b1f2bc40f5.tar.gz
chromium_src-05349986a0c968101b8925be496794b1f2bc40f5.tar.bz2
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) Review URL: http://codereview.chromium.org/63036 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13644 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rwxr-xr-xchrome/browser/extensions/extension_message_service.cc26
-rwxr-xr-xchrome/browser/extensions/extension_message_service.h9
-rw-r--r--chrome/browser/renderer_host/resource_dispatcher_host.cc6
-rw-r--r--chrome/browser/renderer_host/resource_message_filter.cc8
-rw-r--r--chrome/browser/renderer_host/safe_browsing_resource_handler.cc35
-rw-r--r--chrome/browser/renderer_host/safe_browsing_resource_handler.h16
-rw-r--r--chrome/browser/worker_host/worker_service.cc34
-rw-r--r--chrome/browser/worker_host/worker_service.h10
-rw-r--r--chrome/common/notification_type.h4
9 files changed, 113 insertions, 35 deletions
diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 8a4ef73..f200c5b 100755
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -11,6 +11,7 @@
#include "chrome/browser/renderer_host/render_view_host.h"
#include "chrome/browser/renderer_host/render_process_host.h"
#include "chrome/browser/renderer_host/resource_message_filter.h"
+#include "chrome/common/notification_service.h"
#include "chrome/common/render_messages.h"
#include "chrome/common/stl_util-inl.h"
@@ -132,14 +133,23 @@ void ExtensionMessageService::RendererReady(ResourceMessageFilter* renderer) {
AutoLock lock(renderers_lock_);
DCHECK(renderers_.find(renderer->GetProcessId()) == renderers_.end());
renderers_[renderer->GetProcessId()] = renderer;
+
+ NotificationService::current()->AddObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(renderer));
}
-void ExtensionMessageService::RendererShutdown(
- ResourceMessageFilter* renderer) {
+void ExtensionMessageService::Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details) {
+ DCHECK(type.value == NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN);
+ ResourceMessageFilter* filter = Source<ResourceMessageFilter>(source).ptr();
+
{
AutoLock lock(renderers_lock_);
- DCHECK(renderers_.find(renderer->GetProcessId()) != renderers_.end());
- renderers_.erase(renderer->GetProcessId());
+ DCHECK(renderers_.find(filter->GetProcessId()) != renderers_.end());
+ renderers_.erase(filter->GetProcessId());
}
// Close any channels that share this filter.
@@ -147,7 +157,13 @@ void ExtensionMessageService::RendererShutdown(
for (MessageChannelMap::iterator it = channels_.begin();
it != channels_.end(); ) {
MessageChannelMap::iterator current = it++;
- if (current->second.port1 == renderer || current->second.port2 == renderer)
+ if (current->second.port1 == filter || current->second.port2 == filter)
channels_.erase(current);
}
+
+ NotificationService::current()->RemoveObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(filter));
}
+
diff --git a/chrome/browser/extensions/extension_message_service.h b/chrome/browser/extensions/extension_message_service.h
index a63837a..1fc3c51 100755
--- a/chrome/browser/extensions/extension_message_service.h
+++ b/chrome/browser/extensions/extension_message_service.h
@@ -9,6 +9,7 @@
#include <string>
#include "base/lock.h"
+#include "chrome/common/notification_observer.h"
class ExtensionView;
class ResourceMessageFilter;
@@ -23,7 +24,7 @@ class URLRequestContext;
// port: an IPC::Message::Sender interface through which we communicate to a
// process. We use MessageFilters for this since that allows us to send our
// messages on the IO thread.
-class ExtensionMessageService {
+class ExtensionMessageService : public NotificationObserver {
public:
// Returns the message service for the given context. Messages can only
// be sent within a single context.
@@ -54,8 +55,10 @@ class ExtensionMessageService {
// Called to let us know that a renderer has been started.
void RendererReady(ResourceMessageFilter* port);
- // Called to let us know that a renderer is going away.
- void RendererShutdown(ResourceMessageFilter* port);
+ // NotificationObserver interface.
+ void Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details);
private:
// A map of extension ID to the render_process_id that the extension lives in.
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc
index 3169786..f73d507 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -382,7 +382,8 @@ void ResourceDispatcherHost::BeginRequest(
request_data.url,
request_data.resource_type,
safe_browsing_,
- this);
+ this,
+ receiver_);
}
// Insert a buffered event handler before the actual one.
@@ -513,7 +514,8 @@ void ResourceDispatcherHost::BeginDownload(const GURL& url,
url,
ResourceType::MAIN_FRAME,
safe_browsing_,
- this);
+ this,
+ receiver_);
}
bool known_proto = URLRequest::IsHandledURL(url);
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 1230f56..06433a1 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -139,9 +139,11 @@ ResourceMessageFilter::ResourceMessageFilter(
}
ResourceMessageFilter::~ResourceMessageFilter() {
- WorkerService::GetInstance()->RendererShutdown(this);
- ExtensionMessageService::GetInstance(request_context_.get())->
- RendererShutdown(this);
+ // Let interested observers know we are being deleted.
+ NotificationService::current()->Notify(
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(this),
+ NotificationService::NoDetails());
if (handle())
base::CloseProcessHandle(handle());
diff --git a/chrome/browser/renderer_host/safe_browsing_resource_handler.cc b/chrome/browser/renderer_host/safe_browsing_resource_handler.cc
index 22c6873..02b946b 100644
--- a/chrome/browser/renderer_host/safe_browsing_resource_handler.cc
+++ b/chrome/browser/renderer_host/safe_browsing_resource_handler.cc
@@ -5,6 +5,8 @@
#include "chrome/browser/renderer_host/safe_browsing_resource_handler.h"
#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/browser/renderer_host/resource_message_filter.h"
+#include "chrome/common/notification_service.h"
// Maximum time to wait for a gethash response from the Safe Browsing servers.
static const int kMaxGetHashMs = 1000;
@@ -16,7 +18,8 @@ SafeBrowsingResourceHandler::SafeBrowsingResourceHandler(
const GURL& url,
ResourceType::Type resource_type,
SafeBrowsingService* safe_browsing,
- ResourceDispatcherHost* resource_dispatcher_host)
+ ResourceDispatcherHost* resource_dispatcher_host,
+ ResourceDispatcherHost::Receiver* receiver)
: next_handler_(handler),
render_process_host_id_(render_process_host_id),
render_view_id_(render_view_id),
@@ -26,7 +29,8 @@ SafeBrowsingResourceHandler::SafeBrowsingResourceHandler(
safe_browsing_(safe_browsing),
queued_error_request_id_(-1),
rdh_(resource_dispatcher_host),
- resource_type_(resource_type) {
+ resource_type_(resource_type),
+ receiver_(receiver) {
if (safe_browsing_->CheckUrl(url, this)) {
safe_browsing_result_ = SafeBrowsingService::URL_SAFE;
safe_browsing_->LogPauseDelay(base::TimeDelta()); // No delay.
@@ -35,13 +39,20 @@ SafeBrowsingResourceHandler::SafeBrowsingResourceHandler(
in_safe_browsing_check_ = true;
// Can't pause now because it's too early, so we'll do it in OnWillRead.
}
+
+ NotificationService::current()->AddObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(
+ static_cast<ResourceMessageFilter*>(receiver_)));
}
SafeBrowsingResourceHandler::~SafeBrowsingResourceHandler() {
- // If we're being deleted before the SafeBrowsing check has completed, cancel
- // the check.
- if (in_safe_browsing_check_)
- safe_browsing_->CancelCheck(this);
+ NotificationService::current()->RemoveObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(
+ static_cast<ResourceMessageFilter*>(receiver_)));
}
bool SafeBrowsingResourceHandler::OnUploadProgress(int request_id,
@@ -190,3 +201,15 @@ void SafeBrowsingResourceHandler::OnBlockingPageComplete(bool proceed) {
Release();
}
+
+void SafeBrowsingResourceHandler::Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details) {
+ DCHECK(type.value == NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN);
+ if (in_safe_browsing_check_) {
+ safe_browsing_->CancelCheck(this);
+ in_safe_browsing_check_ = false;
+ Release();
+ }
+}
+
diff --git a/chrome/browser/renderer_host/safe_browsing_resource_handler.h b/chrome/browser/renderer_host/safe_browsing_resource_handler.h
index 1585e35..19339a7 100644
--- a/chrome/browser/renderer_host/safe_browsing_resource_handler.h
+++ b/chrome/browser/renderer_host/safe_browsing_resource_handler.h
@@ -8,14 +8,15 @@
#include <string>
#include "base/time.h"
+#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
#include "chrome/browser/renderer_host/resource_handler.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
-
-class ResourceDispatcherHost;
+#include "chrome/common/notification_observer.h"
// Checks that a url is safe.
class SafeBrowsingResourceHandler : public ResourceHandler,
- public SafeBrowsingService::Client {
+ public SafeBrowsingService::Client,
+ public NotificationObserver {
public:
SafeBrowsingResourceHandler(ResourceHandler* handler,
int render_process_host_id,
@@ -23,7 +24,8 @@ class SafeBrowsingResourceHandler : public ResourceHandler,
const GURL& url,
ResourceType::Type resource_type,
SafeBrowsingService* safe_browsing,
- ResourceDispatcherHost* resource_dispatcher_host);
+ ResourceDispatcherHost* resource_dispatcher_host,
+ ResourceDispatcherHost::Receiver* receiver);
~SafeBrowsingResourceHandler();
// ResourceHandler implementation:
@@ -47,6 +49,11 @@ class SafeBrowsingResourceHandler : public ResourceHandler,
// the user has decided to proceed with the current request, or go back.
void OnBlockingPageComplete(bool proceed);
+ // NotificationObserver interface.
+ void Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details);
+
private:
scoped_refptr<ResourceHandler> next_handler_;
int render_process_host_id_;
@@ -62,6 +69,7 @@ class SafeBrowsingResourceHandler : public ResourceHandler,
ResourceDispatcherHost* rdh_;
base::Time pause_time_;
ResourceType::Type resource_type_;
+ ResourceDispatcherHost::Receiver* receiver_;
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingResourceHandler);
};
diff --git a/chrome/browser/worker_host/worker_service.cc b/chrome/browser/worker_host/worker_service.cc
index 8151d9b..04d9d8d 100644
--- a/chrome/browser/worker_host/worker_service.cc
+++ b/chrome/browser/worker_host/worker_service.cc
@@ -14,6 +14,7 @@
#include "chrome/browser/renderer_host/render_process_host.h"
#include "chrome/browser/renderer_host/resource_message_filter.h"
#include "chrome/common/chrome_switches.h"
+#include "chrome/common/notification_service.h"
#include "net/base/registry_controlled_domain.h"
namespace {
@@ -57,6 +58,13 @@ bool WorkerService::CreateDedicatedWorker(const GURL &url,
// it to.
worker->CreateWorker(url, render_view_route_id, ++next_worker_route_id_,
renderer_route_id, filter);
+
+ // Receive a notification if the message filter is deleted.
+ NotificationService::current()->AddObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(filter));
+
return true;
}
@@ -71,14 +79,6 @@ void WorkerService::ForwardMessage(const IPC::Message& message) {
// TODO(jabdelmalek): tell sender that callee is gone
}
-void WorkerService::RendererShutdown(ResourceMessageFilter* filter) {
- for (ChildProcessHost::Iterator iter(ChildProcessInfo::WORKER_PROCESS);
- !iter.Done(); ++iter) {
- WorkerProcessHost* worker = static_cast<WorkerProcessHost*>(*iter);
- worker->RendererShutdown(filter);
- }
-}
-
WorkerProcessHost* WorkerService::GetProcessForDomain(const GURL& url) {
int num_processes = 0;
std::string domain =
@@ -126,3 +126,21 @@ WorkerProcessHost* WorkerService::GetLeastLoadedWorker() {
return smallest;
}
+
+void WorkerService::Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details) {
+ DCHECK(type.value == NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN);
+ ResourceMessageFilter* filter = Source<ResourceMessageFilter>(source).ptr();
+
+ for (ChildProcessHost::Iterator iter(ChildProcessInfo::WORKER_PROCESS);
+ !iter.Done(); ++iter) {
+ WorkerProcessHost* worker = static_cast<WorkerProcessHost*>(*iter);
+ worker->RendererShutdown(filter);
+ }
+
+ NotificationService::current()->RemoveObserver(
+ this,
+ NotificationType::RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+ Source<ResourceMessageFilter>(filter));
+}
diff --git a/chrome/browser/worker_host/worker_service.h b/chrome/browser/worker_host/worker_service.h
index 1aea2aa..cf18d58 100644
--- a/chrome/browser/worker_host/worker_service.h
+++ b/chrome/browser/worker_host/worker_service.h
@@ -9,6 +9,7 @@
#include "base/basictypes.h"
#include "base/singleton.h"
+#include "chrome/common/notification_observer.h"
#include "googleurl/src/gurl.h"
namespace IPC {
@@ -19,7 +20,7 @@ class MessageLoop;
class WorkerProcessHost;
class ResourceMessageFilter;
-class WorkerService {
+class WorkerService : public NotificationObserver {
public:
// Returns the WorkerService singleton.
static WorkerService* GetInstance();
@@ -34,9 +35,10 @@ class WorkerService {
// should be forwarded to the worker process.
void ForwardMessage(const IPC::Message& message);
- // Called by ResourceMessageFilter when it's destructed so that all the
- // WorkerProcessHost objects can remove their pointers to it.
- void RendererShutdown(ResourceMessageFilter* filter);
+ // NotificationObserver interface.
+ void Observe(NotificationType type,
+ const NotificationSource& source,
+ const NotificationDetails& details);
private:
friend struct DefaultSingletonTraits<WorkerService>;
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index e2b9da2..cae614d 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -157,6 +157,10 @@ class NotificationType {
// both normal completion or via a cancel operation.
DOWNLOAD_START, DOWNLOAD_STOP,
+ // Lets resource handlers and other interested observers know when the
+ // message filter is being deleted and can no longer be used.
+ RESOURCE_MESSAGE_FILTER_SHUTDOWN,
+
// Views -------------------------------------------------------------------
// Notification that a view was removed from a view hierarchy. The source