Fix KeyedServiceFactory assertion during shutdown (issue #2083)

This commit is contained in:
Marshall Greenblatt 2017-01-31 17:23:27 -05:00
parent 1758b744b2
commit ebb70f94e8
2 changed files with 55 additions and 6 deletions

View File

@ -23,6 +23,7 @@
#include "chrome/common/features.h"
#include "chrome/common/pref_names.h"
#include "components/content_settings/core/browser/content_settings_utils.h"
#include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/plugin_service.h"
@ -48,6 +49,39 @@ using content::WebPluginInfo;
namespace {
// There's a race condition between deletion of the CefPluginInfoMessageFilter
// object on the UI thread and deletion of the PrefService (owned by Profile)
// on the UI thread. If the PrefService will be deleted first then
// PrefMember::Destroy() must be called from ShutdownOnUIThread() to avoid
// heap-use-after-free on CefPluginInfoMessageFilter destruction (due to
// ~PrefMember trying to access the already-deleted PrefService).
// ShutdownNotifierFactory makes sure that ShutdownOnUIThread() is called in
// this case.
class ShutdownNotifierFactory
: public BrowserContextKeyedServiceShutdownNotifierFactory {
public:
static ShutdownNotifierFactory* GetInstance();
private:
friend struct base::DefaultLazyInstanceTraits<ShutdownNotifierFactory>;
ShutdownNotifierFactory()
: BrowserContextKeyedServiceShutdownNotifierFactory(
"CefPluginInfoMessageFilter") {
}
~ShutdownNotifierFactory() override {}
DISALLOW_COPY_AND_ASSIGN(ShutdownNotifierFactory);
};
base::LazyInstance<ShutdownNotifierFactory>::Leaky
g_shutdown_notifier_factory = LAZY_INSTANCE_INITIALIZER;
// static
ShutdownNotifierFactory* ShutdownNotifierFactory::GetInstance() {
return g_shutdown_notifier_factory.Pointer();
}
// For certain sandboxed Pepper plugins, use the JavaScript Content Settings.
bool ShouldUseJavaScriptSettingForPlugin(const WebPluginInfo& plugin) {
if (plugin.type != WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS &&
@ -161,14 +195,29 @@ CefPluginInfoMessageFilter::Context::Context(
CefPluginInfoMessageFilter::Context::~Context() {
}
void CefPluginInfoMessageFilter::Context::ShutdownOnUIThread() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
always_authorize_plugins_.Destroy();
allow_outdated_plugins_.Destroy();
}
CefPluginInfoMessageFilter::CefPluginInfoMessageFilter(
int render_process_id,
CefBrowserContext* profile)
: BrowserMessageFilter(ExtensionMsgStart),
browser_context_(profile),
context_(render_process_id, profile),
main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
weak_ptr_factory_(this) {
shutdown_notifier_ =
ShutdownNotifierFactory::GetInstance()->Get(profile)->Subscribe(
base::Bind(&CefPluginInfoMessageFilter::ShutdownOnUIThread,
base::Unretained(this)));
}
void CefPluginInfoMessageFilter::ShutdownOnUIThread() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
context_.ShutdownOnUIThread();
shutdown_notifier_.reset();
}
bool CefPluginInfoMessageFilter::OnMessageReceived(

View File

@ -16,6 +16,7 @@
#include "chrome/browser/plugins/plugin_metadata.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/keyed_service/core/keyed_service_shutdown_notifier.h"
#include "components/prefs/pref_member.h"
#include "content/public/browser/browser_message_filter.h"
#include "extensions/features/features.h"
@ -50,6 +51,7 @@ class CefPluginInfoMessageFilter : public content::BrowserMessageFilter {
Context(int render_process_id, CefBrowserContext* profile);
~Context();
void ShutdownOnUIThread();
void DecidePluginStatus(
const GetPluginInfo_Params& params,
@ -95,6 +97,7 @@ class CefPluginInfoMessageFilter : public content::BrowserMessageFilter {
content::BrowserThread::UI>;
friend class base::DeleteHelper<CefPluginInfoMessageFilter>;
void ShutdownOnUIThread();
~CefPluginInfoMessageFilter() override;
void OnGetPluginInfo(int render_frame_id,
@ -125,12 +128,9 @@ class CefPluginInfoMessageFilter : public content::BrowserMessageFilter {
std::vector<base::string16>* additional_param_values);
#endif
scoped_refptr<CefBrowserContext> browser_context_;
// Members will be destroyed in reverse order of declaration. Due to Context
// depending on the PrefService owned by CefBrowserContext the Context object
// must be destroyed before the CefBrowserContext object.
Context context_;
std::unique_ptr<KeyedServiceShutdownNotifier::Subscription>
shutdown_notifier_;
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
base::WeakPtrFactory<CefPluginInfoMessageFilter> weak_ptr_factory_;