Add DiscardableSharedMemoryManager::Get()

There's always a single instance of this class in practice, shared by the
whole browser process. Previously it was exposed via a method on
BrowserMainLoop, but BrowserMainLoop has other usage constraints which make
it an inconvenient way to expose the DiscardableSharedMemoryManager in some
environments.

This adds a static Get() method to DiscardableSharedMemoryManager and removes
the discardable_shared_memory_manager() accessor from BrowserMainLoop. This
is a precursor to lifting the instance itself out of BrowserMainLoop and into
ContentMainRunnerImpl so that the the system Service Manager Connector (which
also exposes the DiscardableSharedMemoryMananager) can be brought up earlier
and without BrowserMainLoop.

Finally, this also deletes an unused and unnecessary public content API
to access the DiscardableSharedMemoryManager, given that any dependents
(in, e.g., other non-chromium content embedders, assuming some dependents
exist) can just call the static Get() now.

Bug: 904240, 968147
Change-Id: Ifb0f9936a2f852f9ce3ffe5447f778c7fad3b7d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651496
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668597}

components/discardable_memory/service/discardable_shared_memory_manager.cc

@@ -206,6 +206,8 @@ const int kEnforceMemoryPolicyDelayMs = 1000;
 // Global atomic to generate unique discardable shared memory IDs.
 base::AtomicSequenceNumber g_next_discardable_shared_memory_id;
 
+DiscardableSharedMemoryManager* g_instance = nullptr;
+
 }  // namespace
 
 DiscardableSharedMemoryManager::MemorySegment::MemorySegment(
@@ -228,6 +230,9 @@ DiscardableSharedMemoryManager::DiscardableSharedMemoryManager()
       mojo_thread_message_loop_(base::MessageLoopCurrent::GetNull()),
       weak_ptr_factory_(this),
       mojo_thread_weak_ptr_factory_(this) {
+  DCHECK(!g_instance)
+      << "A DiscardableSharedMemoryManager already exists in this process.";
+  g_instance = this;
   DCHECK_NE(memory_limit_, 0u);
   enforce_memory_policy_callback_ =
       base::Bind(&DiscardableSharedMemoryManager::EnforceMemoryPolicy,
@@ -265,6 +270,14 @@ DiscardableSharedMemoryManager::~DiscardableSharedMemoryManager() {
         event.Wait();
     }
   }
+
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
+
+// static
+DiscardableSharedMemoryManager* DiscardableSharedMemoryManager::Get() {
+  return g_instance;
 }
 
 void DiscardableSharedMemoryManager::Bind(

components/discardable_memory/service/discardable_shared_memory_manager.h

@@ -53,6 +53,11 @@ class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryManager
   DiscardableSharedMemoryManager();
   ~DiscardableSharedMemoryManager() override;
 
+  // Returns the global instance of DiscardableSharedMemoryManager, usable from
+  // any thread. May return null if no DiscardableSharedMemoryManager has been
+  // created in the current process.
+  static DiscardableSharedMemoryManager* Get();
+
   // Bind the manager to a mojo interface request.
   void Bind(mojom::DiscardableSharedMemoryManagerRequest request,
             const service_manager::BindSourceInfo& source_info);

content/app/DEPS

@@ -1,4 +1,5 @@
 include_rules = [
+  "+components/discardable_memory",
   "+components/download",
   "+content",
   "+device/bluetooth",

content/app/content_main_runner_impl.cc

@@ -44,6 +44,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
+#include "components/discardable_memory/service/discardable_shared_memory_manager.h"
 #include "components/download/public/common/download_task_runner.h"
 #include "content/app/mojo/mojo_init.h"
 #include "content/browser/browser_process_sub_thread.h"
@@ -951,6 +952,9 @@ int ContentMainRunnerImpl::RunServiceManager(MainFunctionParams& main_params,
       }
     }
 
+    discardable_shared_memory_manager_ =
+        std::make_unique<discardable_memory::DiscardableSharedMemoryManager>();
+
     // PowerMonitor is needed in reduced mode but is eventually passed on to
     // BrowserMainLoop.
     power_monitor_ = std::make_unique<base::PowerMonitor>(

content/app/content_main_runner_impl.h

@@ -29,6 +29,10 @@ namespace base {
 class AtExitManager;
 }  // namespace base
 
+namespace discardable_memory {
+class DiscardableSharedMemoryManager;
+}
+
 namespace content {
 class ContentMainDelegate;
 struct ContentMainParams;
@@ -54,6 +58,8 @@ class ContentMainRunnerImpl : public ContentMainRunner {
 
   bool is_browser_main_loop_started_ = false;
 
+  std::unique_ptr<discardable_memory::DiscardableSharedMemoryManager>
+      discardable_shared_memory_manager_;
   std::unique_ptr<StartupDataImpl> startup_data_;
   std::unique_ptr<base::FieldTrialList> field_trial_list_;
   std::unique_ptr<BrowserProcessSubThread> service_manager_thread_;

content/browser/BUILD.gn

@@ -786,7 +786,6 @@ jumbo_source_set("browser") {
     "devtools/shared_worker_devtools_manager.h",
     "devtools/worker_devtools_agent_host.cc",
     "devtools/worker_devtools_agent_host.h",
-    "discardable_shared_memory_manager.cc",
     "display_cutout/display_cutout_constants.h",
     "display_cutout/display_cutout_host_impl.cc",
     "display_cutout/display_cutout_host_impl.h",

content/browser/browser_main_loop.cc

@@ -735,13 +735,11 @@ void BrowserMainLoop::PostMainMessageLoopStart() {
         base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI}));
   }
 
-  discardable_shared_memory_manager_ =
-      std::make_unique<discardable_memory::DiscardableSharedMemoryManager>();
   // TODO(boliu): kSingleProcess check is a temporary workaround for
   // in-process Android WebView. crbug.com/503724 tracks proper fix.
   if (!parsed_command_line_.HasSwitch(switches::kSingleProcess)) {
     base::DiscardableMemoryAllocator::SetInstance(
-        discardable_shared_memory_manager_.get());
+        discardable_memory::DiscardableSharedMemoryManager::Get());
   }
 
   if (parts_)

content/browser/browser_main_loop.h

@@ -39,10 +39,6 @@ class SingleThreadTaskRunner;
 class SystemMonitor;
 }  // namespace base
 
-namespace discardable_memory {
-class DiscardableSharedMemoryManager;
-}
-
 namespace gpu {
 class GpuChannelEstablishFactory;
 }
@@ -190,10 +186,6 @@ class CONTENT_EXPORT BrowserMainLoop {
   }
 #endif
 
-  discardable_memory::DiscardableSharedMemoryManager*
-  discardable_shared_memory_manager() const {
-    return discardable_shared_memory_manager_.get();
-  }
   midi::MidiService* midi_service() const { return midi_service_.get(); }
 
   // Returns the task runner for tasks that that are critical to producing a new
@@ -396,8 +388,6 @@ class CONTENT_EXPORT BrowserMainLoop {
   std::unique_ptr<LoaderDelegateImpl> loader_delegate_;
   std::unique_ptr<ResourceDispatcherHostImpl> resource_dispatcher_host_;
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
-  std::unique_ptr<discardable_memory::DiscardableSharedMemoryManager>
-      discardable_shared_memory_manager_;
   scoped_refptr<SaveFileManager> save_file_manager_;
   std::unique_ptr<content::TracingControllerImpl> tracing_controller_;
   scoped_refptr<responsiveness::Watcher> responsiveness_watcher_;

content/browser/gpu/gpu_process_host.cc

@@ -35,7 +35,6 @@
 #include "components/viz/common/features.h"
 #include "components/viz/common/switches.h"
 #include "content/browser/browser_child_process_host_impl.h"
-#include "content/browser/browser_main_loop.h"
 #include "content/browser/compositor/image_transport_factory.h"
 #include "content/browser/field_trial_recorder.h"
 #include "content/browser/gpu/compositor_util.h"
@@ -48,6 +47,7 @@
 #include "content/common/in_process_child_thread_params.h"
 #include "content/common/service_manager/child_connection.h"
 #include "content/common/view_messages.h"
+#include "content/public/browser/browser_main_runner.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -454,7 +454,7 @@ void BindDiscardableMemoryRequestOnUI(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(
           &BindDiscardableMemoryRequestOnIO, std::move(request),
-          BrowserMainLoop::GetInstance()->discardable_shared_memory_manager()));
+          discardable_memory::DiscardableSharedMemoryManager::Get()));
 }
 
 }  // anonymous namespace

content/browser/service_manager/common_browser_interfaces.cc

@@ -17,7 +17,6 @@
 #include "build/build_config.h"
 #include "components/discardable_memory/service/discardable_shared_memory_manager.h"
 #include "components/viz/host/gpu_client.h"
-#include "content/browser/browser_main_loop.h"
 #include "content/browser/gpu/browser_gpu_client_delegate.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -54,19 +53,16 @@ class ConnectionFilterImpl : public ConnectionFilter {
         base::BindRepeating(&SandboxSupportMacImpl::BindRequest,
                             base::Owned(new SandboxSupportMacImpl)));
 #endif
-    // For mus, the mojom::discardable_memory::DiscardableSharedMemoryManager
-    // is exposed from ui::Service. So we don't need bind the interface here.
-    auto* browser_main_loop = BrowserMainLoop::GetInstance();
-    if (browser_main_loop) {
-      auto* manager = browser_main_loop->discardable_shared_memory_manager();
-      if (manager) {
-        registry_.AddInterface(base::BindRepeating(
-            &discardable_memory::DiscardableSharedMemoryManager::Bind,
-            base::Unretained(manager)));
-      }
-    }
     registry_.AddInterface(base::BindRepeating(
         &ConnectionFilterImpl::BindGpuRequest, base::Unretained(this)));
+
+    auto* discardable_shared_memory_manager =
+        discardable_memory::DiscardableSharedMemoryManager::Get();
+    if (discardable_shared_memory_manager) {
+      registry_.AddInterface(base::BindRepeating(
+          &discardable_memory::DiscardableSharedMemoryManager::Bind,
+          base::Unretained(discardable_shared_memory_manager)));
+    }
   }
 
   ~ConnectionFilterImpl() override { DCHECK_CURRENTLY_ON(BrowserThread::IO); }

content/public/browser/BUILD.gn

@@ -133,7 +133,6 @@ jumbo_source_set("browser_sources") {
     "devtools_manager_delegate.h",
     "devtools_network_transaction_factory.h",
     "devtools_socket_factory.h",
-    "discardable_shared_memory_manager.h",
     "dom_storage_context.h",
     "download_item_utils.h",
     "download_manager.cc",

content/public/test/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "-content",
   "+content/common/service_worker/service_worker_types.h",
   "+content/public",
+  "+components/discardable_memory/service",
   "+components/download/public/common",
   "+components/viz/common",
   "+components/viz/host",

content/public/test/browser_test_base.cc

@@ -59,6 +59,7 @@
 #include "ui/gl/gl_switches.h"
 
 #if defined(OS_ANDROID)
+#include "components/discardable_memory/service/discardable_shared_memory_manager.h"  // nogncheck
 #include "content/app/mojo/mojo_init.h"
 #include "content/common/url_schemes.h"
 #include "content/public/app/content_main_delegate.h"
@@ -183,6 +184,7 @@ BrowserTestBase::~BrowserTestBase() {
   // RemoteTestServer can cause wait on the UI thread.
   base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait;
   spawned_test_server_.reset();
+  discardable_shared_memory_manager_.reset();
 #endif
 
   CHECK(set_up_called_ || IsSkipped())
@@ -393,6 +395,9 @@ void BrowserTestBase::SetUp() {
     delegate->PostTaskSchedulerStart();
   }
 
+  discardable_shared_memory_manager_ =
+      std::make_unique<discardable_memory::DiscardableSharedMemoryManager>();
+
   // ContentMain would normally call RunProcess() on the delegate and fallback
   // to BrowserMain() if it did not run it (or equivalent) itself. On Android,
   // RunProcess() will return 0 so we don't have to fallback to BrowserMain().

content/public/test/browser_test_base.h

@@ -5,6 +5,8 @@
 #ifndef CONTENT_PUBLIC_TEST_BROWSER_TEST_BASE_H_
 #define CONTENT_PUBLIC_TEST_BROWSER_TEST_BASE_H_
 
+#include <memory>
+
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/field_trial.h"
@@ -20,6 +22,12 @@ class CommandLine;
 class FilePath;
 }
 
+#if defined(OS_ANDROID)
+namespace discardable_memory {
+class DiscardableSharedMemoryManager;
+}
+#endif  // defined(OS_ANDROID)
+
 namespace content {
 
 class BrowserMainParts;
@@ -158,6 +166,13 @@ class BrowserTestBase : public testing::Test {
   // added in SetUpOnMainThread.
   void InitializeNetworkProcess();
 
+#if defined(OS_ANDROID)
+  // On Android we don't use ContentMainRunner for browser tests, so we bring
+  // our own DiscardableSharedMemoryManager instance.
+  std::unique_ptr<discardable_memory::DiscardableSharedMemoryManager>
+      discardable_shared_memory_manager_;
+#endif
+
   // Testing server, started on demand.
   std::unique_ptr<net::SpawnedTestServer> spawned_test_server_;
 

content/renderer/render_thread_impl_discardable_memory_browsertest.cc

@@ -19,7 +19,6 @@
 #include "build/build_config.h"
 #include "components/discardable_memory/client/client_discardable_shared_memory_manager.h"
 #include "components/discardable_memory/service/discardable_shared_memory_manager.h"
-#include "content/browser/browser_main_loop.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
@@ -79,9 +78,7 @@ IN_PROC_BROWSER_TEST_F(RenderThreadImplDiscardableMemoryBrowserTest,
   memory->Unlock();
 
   // Purge all unlocked memory.
-  BrowserMainLoop::GetInstance()
-      ->discardable_shared_memory_manager()
-      ->SetMemoryLimit(0);
+  discardable_memory::DiscardableSharedMemoryManager::Get()->SetMemoryLimit(0);
 
   // Should fail as memory should have been purged.
   EXPECT_FALSE(memory->Lock());
@@ -120,8 +117,7 @@ IN_PROC_BROWSER_TEST_F(RenderThreadImplDiscardableMemoryBrowserTest,
   EXPECT_TRUE(memory);
   memory.reset();
 
-  EXPECT_GE(BrowserMainLoop::GetInstance()
-                ->discardable_shared_memory_manager()
+  EXPECT_GE(discardable_memory::DiscardableSharedMemoryManager::Get()
                 ->GetBytesAllocated(),
             kSize);
 
@@ -131,8 +127,7 @@ IN_PROC_BROWSER_TEST_F(RenderThreadImplDiscardableMemoryBrowserTest,
   base::TimeTicks end =
       base::TimeTicks::Now() + base::TimeDelta::FromSeconds(5);
   while (base::TimeTicks::Now() < end) {
-    if (!BrowserMainLoop::GetInstance()
-             ->discardable_shared_memory_manager()
+    if (!discardable_memory::DiscardableSharedMemoryManager::Get()
              ->GetBytesAllocated())
       break;
   }
@@ -151,8 +146,7 @@ IN_PROC_BROWSER_TEST_F(RenderThreadImplDiscardableMemoryBrowserTest,
   EXPECT_TRUE(memory);
   memory.reset();
 
-  EXPECT_GE(BrowserMainLoop::GetInstance()
-                ->discardable_shared_memory_manager()
+  EXPECT_GE(discardable_memory::DiscardableSharedMemoryManager::Get()
                 ->GetBytesAllocated(),
             kSize);
 
@@ -163,8 +157,7 @@ IN_PROC_BROWSER_TEST_F(RenderThreadImplDiscardableMemoryBrowserTest,
   base::RunLoop().RunUntilIdle();
   RunAllTasksUntilIdle();
 
-  EXPECT_EQ(0U, BrowserMainLoop::GetInstance()
-                    ->discardable_shared_memory_manager()
+  EXPECT_EQ(0U, discardable_memory::DiscardableSharedMemoryManager::Get()
                     ->GetBytesAllocated());
 }
 

content/test/BUILD.gn

@@ -526,6 +526,7 @@ jumbo_static_library("test_support") {
     ]
 
     deps += [
+      "//components/discardable_memory/service",
       "//media/capture/video/android:android",
       "//ui/android",
       "//ui/android:test_support",