Fix KeyedServiceFactory assertion during shutdown (issue #2083)

libcef/browser/plugins/plugin_info_message_filter.cc

@@ -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(

libcef/browser/plugins/plugin_info_message_filter.h

@@ -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_;