diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-14 15:25:13 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-14 15:25:13 +0000 |
commit | 528211a4c55cec0d9ef0bc22d7cd7fc9e3138f57 (patch) | |
tree | e0252e8cf0d69b56c07d5b54ed88475fce8cfa59 /chrome/browser | |
parent | cf2f9ebf31d505a701b62dea166bea7bce86b76c (diff) | |
download | chromium_src-528211a4c55cec0d9ef0bc22d7cd7fc9e3138f57.zip chromium_src-528211a4c55cec0d9ef0bc22d7cd7fc9e3138f57.tar.gz chromium_src-528211a4c55cec0d9ef0bc22d7cd7fc9e3138f57.tar.bz2 |
Fix a Chrome crash which occurs when the chrome instance launched via ChromeFrame is shutting down.
I was not able to reproduce this locally. Based on the crash dump, it looks like the crash occurs
when the sync channel used by the automation provider is shutting down and it is attempting a PostTask
of OnChannelClosed. The crash occurs while posting the task to the message loop, because the message loop
is invalid.
This could occur if the message loop has been destroyed. The message loop is destroyed when the module
refcount drops to zero. The automation provider releases the module ref count in OnChannelError. In theory
there is a race condition here, i.e. the message loop could be destroyed before the ExternalTabContainer,
etc are destroyed.
Suggested fix is to call AddRefModule which grabs a reference on the UI message loop in the automation provider
constructor and release it in the destructor. We also close the channel before invoking ReleaseModule in the
destructor.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=31621
Bug=31621
Review URL: http://codereview.chromium.org/542036
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36240 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/automation/automation_provider.cc | 6 | ||||
-rw-r--r-- | chrome/browser/automation/automation_provider_list.cc | 3 |
2 files changed, 6 insertions, 3 deletions
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index f905220..df086ff 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -140,6 +140,7 @@ AutomationProvider::AutomationProvider(Profile* profile) new_tab_ui_load_observer_.reset(new NewTabUILoadObserver(this)); dom_operation_observer_.reset(new DomOperationNotificationObserver(this)); metric_event_duration_observer_.reset(new MetricEventDurationObserver()); + g_browser_process->AddRefModule(); } AutomationProvider::~AutomationProvider() { @@ -152,6 +153,11 @@ AutomationProvider::~AutomationProvider() { NotificationObserver* observer; while ((observer = it.GetNext()) != NULL) delete observer; + + if (channel_.get()) { + channel_->Close(); + } + g_browser_process->ReleaseModule(); } void AutomationProvider::ConnectToChannel(const std::string& channel_id) { diff --git a/chrome/browser/automation/automation_provider_list.cc b/chrome/browser/automation/automation_provider_list.cc index b803be1..6676626 100644 --- a/chrome/browser/automation/automation_provider_list.cc +++ b/chrome/browser/automation/automation_provider_list.cc @@ -20,7 +20,6 @@ AutomationProviderList::~AutomationProviderList() { while (iter != automation_providers_.end()) { (*iter)->Release(); iter = automation_providers_.erase(iter); - g_browser_process->ReleaseModule(); } instance_ = NULL; } @@ -28,7 +27,6 @@ AutomationProviderList::~AutomationProviderList() { bool AutomationProviderList::AddProvider(AutomationProvider* provider) { provider->AddRef(); automation_providers_.push_back(provider); - g_browser_process->AddRefModule(); return true; } @@ -38,7 +36,6 @@ bool AutomationProviderList::RemoveProvider(AutomationProvider* provider) { if (remove_provider != automation_providers_.end()) { (*remove_provider)->Release(); automation_providers_.erase(remove_provider); - g_browser_process->ReleaseModule(); if (automation_providers_.empty()) OnLastProviderRemoved(); return true; |