[PM] Extend the lifetime of PerformanceManagerLockObserver.

PerformanceManagerLockObserver must stay alive until after
base::ThreadPool shutdown because it can be accessed by IndexedDB
which runs on a base::ThreadPool sequence.

Bug: 980533
Change-Id: I6d454cffa68cb776b9c4abf3180506096c4e91c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856891
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705370}

chrome/browser/chrome_content_browser_client.cc

@@ -223,7 +223,6 @@
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/payments/content/payment_request_display_manager.h"
-#include "components/performance_manager/public/performance_manager.h"
 #include "components/policy/content/policy_blacklist_navigation_throttle.h"
 #include "components/policy/content/policy_blacklist_service.h"
 #include "components/policy/core/common/policy_service.h"
@@ -2844,7 +2843,8 @@ content::MediaObserver* ChromeContentBrowserClient::GetMediaObserver() {
 }
 
 content::LockObserver* ChromeContentBrowserClient::GetLockObserver() {
-  return performance_manager::PerformanceManager::GetLockObserver();
+  return ChromeBrowserMainExtraPartsPerformanceManager::GetInstance()
+      ->GetLockObserver();
 }
 
 content::PlatformNotificationService*

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "components/performance_manager/graph/graph_impl.h"
 #include "components/performance_manager/performance_manager_impl.h"
+#include "components/performance_manager/performance_manager_lock_observer.h"
 #include "components/performance_manager/performance_manager_tab_helper.h"
 #include "components/performance_manager/render_process_user_data.h"
 #include "components/performance_manager/shared_worker_watcher.h"
@@ -38,10 +39,29 @@
 #endif  // BUILDFLAG(USE_TCMALLOC)
 #endif  // defined(OS_LINUX)
 
+namespace {
+ChromeBrowserMainExtraPartsPerformanceManager* g_instance = nullptr;
+}
+
 ChromeBrowserMainExtraPartsPerformanceManager::
-    ChromeBrowserMainExtraPartsPerformanceManager() = default;
+    ChromeBrowserMainExtraPartsPerformanceManager()
+    : lock_observer_(std::make_unique<
+                     performance_manager::PerformanceManagerLockObserver>()) {
+  DCHECK(!g_instance);
+  g_instance = this;
+}
+
 ChromeBrowserMainExtraPartsPerformanceManager::
-    ~ChromeBrowserMainExtraPartsPerformanceManager() = default;
+    ~ChromeBrowserMainExtraPartsPerformanceManager() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
+
+// static
+ChromeBrowserMainExtraPartsPerformanceManager*
+ChromeBrowserMainExtraPartsPerformanceManager::GetInstance() {
+  return g_instance;
+}
 
 // static
 void ChromeBrowserMainExtraPartsPerformanceManager::
@@ -74,6 +94,11 @@ void ChromeBrowserMainExtraPartsPerformanceManager::
 #endif  // defined(OS_LINUX)
 }
 
+content::LockObserver*
+ChromeBrowserMainExtraPartsPerformanceManager::GetLockObserver() {
+  return lock_observer_.get();
+}
+
 void ChromeBrowserMainExtraPartsPerformanceManager::PostCreateThreads() {
   performance_manager_ = performance_manager::PerformanceManagerImpl::Create(
       base::BindOnce(&ChromeBrowserMainExtraPartsPerformanceManager::

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.h

@@ -18,6 +18,10 @@
 
 class Profile;
 
+namespace content {
+class LockObserver;
+}
+
 namespace performance_manager {
 class BrowserChildProcessWatcher;
 class GraphImpl;
@@ -34,9 +38,18 @@ class ChromeBrowserMainExtraPartsPerformanceManager
   ChromeBrowserMainExtraPartsPerformanceManager();
   ~ChromeBrowserMainExtraPartsPerformanceManager() override;
 
+  // Returns the only instance of this class.
+  static ChromeBrowserMainExtraPartsPerformanceManager* GetInstance();
+
   static void CreateDefaultPoliciesAndDecorators(
       performance_manager::GraphImpl* graph);
 
+  // Returns the LockObserver that should be exposed to //content to allow the
+  // performance manager to track usage of locks in frames. Valid to call from
+  // any thread, but external synchronization is needed to make sure that the
+  // performance manager is available.
+  content::LockObserver* GetLockObserver();
+
  private:
   // ChromeBrowserMainExtraParts overrides.
   void PostCreateThreads() override;
@@ -54,6 +67,11 @@ class ChromeBrowserMainExtraPartsPerformanceManager
   std::unique_ptr<performance_manager::PerformanceManagerImpl>
       performance_manager_;
 
+  // This must be alive at least until the end of base::ThreadPool shutdown,
+  // because it can be accessed by IndexedDB which runs on a base::ThreadPool
+  // sequence.
+  const std::unique_ptr<content::LockObserver> lock_observer_;
+
   std::unique_ptr<performance_manager::BrowserChildProcessWatcher>
       browser_child_process_watcher_;
 

components/performance_manager/performance_manager.cc

@@ -45,9 +45,4 @@ void PerformanceManager::PassToGraph(const base::Location& from_here,
           std::move(graph_owned)));
 }
 
-// static
-content::LockObserver* PerformanceManager::GetLockObserver() {
-  return PerformanceManagerImpl::GetInstance()->lock_observer();
-}
-
 }  // namespace performance_manager

components/performance_manager/performance_manager_impl.h

@@ -17,7 +17,6 @@
 #include "base/sequenced_task_runner.h"
 #include "base/task_runner_util.h"
 #include "components/performance_manager/graph/graph_impl.h"
-#include "components/performance_manager/performance_manager_lock_observer.h"
 #include "components/performance_manager/public/graph/worker_node.h"
 #include "components/performance_manager/public/performance_manager.h"
 #include "components/performance_manager/public/render_process_host_proxy.h"
@@ -117,8 +116,6 @@ class PerformanceManagerImpl : public PerformanceManager {
   // TODO(chrisha): Hide this after the last consumer stops using it!
   static scoped_refptr<base::SequencedTaskRunner> GetTaskRunner();
 
-  content::LockObserver* lock_observer() { return &lock_observer_; }
-
   // Indicates whether or not the caller is currently running on the PM task
   // runner.
   bool OnPMTaskRunnerForTesting() const {
@@ -149,9 +146,6 @@ class PerformanceManagerImpl : public PerformanceManager {
 
   GraphImpl graph_;
 
-  // The LockObserver registered with //content to track lock acquisitions.
-  PerformanceManagerLockObserver lock_observer_;
-
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(PerformanceManagerImpl);

components/performance_manager/performance_manager_lock_observer.cc

@@ -25,7 +25,11 @@ void SetIsHoldingWebLock(int render_process_id,
 }  // namespace
 
 PerformanceManagerLockObserver::PerformanceManagerLockObserver() = default;
-PerformanceManagerLockObserver::~PerformanceManagerLockObserver() = default;
+
+PerformanceManagerLockObserver::~PerformanceManagerLockObserver() {
+  // TODO(https://crbug.com/1013760): DCHECK that this happens after ThreadPool
+  // shutdown.
+}
 
 void PerformanceManagerLockObserver::OnFrameStartsHoldingWebLocks(
     int render_process_id,

components/performance_manager/public/performance_manager.h

@@ -8,10 +8,6 @@
 #include "base/callback.h"
 #include "base/location.h"
 
-namespace content {
-class LockObserver;
-}
-
 namespace performance_manager {
 
 class Graph;
@@ -37,12 +33,6 @@ class PerformanceManager {
   static void PassToGraph(const base::Location& from_here,
                           std::unique_ptr<GraphOwned> graph_owned);
 
-  // Returns the LockObserver that should be exposed to //content to allow the
-  // performance manager to track usage of locks in frames. Valid to call from
-  // any thread, but external synchronization is needed to make sure that the
-  // performance manager is available.
-  static content::LockObserver* GetLockObserver();
-
  protected:
   PerformanceManager();
   virtual ~PerformanceManager();