Mac: Fix incorrect creation of NSAutoreleasePool (fixes issue #2160)

This commit is contained in:
Marshall Greenblatt 2021-02-11 17:12:15 -05:00
parent d6dc4b667b
commit 7fe6a18f03
1 changed files with 39 additions and 44 deletions

View File

@ -1,5 +1,5 @@
diff --git content/app/content_main.cc content/app/content_main.cc diff --git content/app/content_main.cc content/app/content_main.cc
index 2aba28d210db..5413f08af93d 100644 index 2aba28d210db..00edc202e2c4 100644
--- content/app/content_main.cc --- content/app/content_main.cc
+++ content/app/content_main.cc +++ content/app/content_main.cc
@@ -205,15 +205,10 @@ void InitializeMojo(mojo::core::Configuration* config) { @@ -205,15 +205,10 @@ void InitializeMojo(mojo::core::Configuration* config) {
@ -20,17 +20,20 @@ index 2aba28d210db..5413f08af93d 100644
// A flag to indicate whether Main() has been called before. On Android, we // A flag to indicate whether Main() has been called before. On Android, we
// may re-run Main() without restarting the browser process. This flag // may re-run Main() without restarting the browser process. This flag
@@ -299,8 +294,7 @@ int RunContentProcess(const ContentMainParams& params, @@ -295,12 +290,6 @@ int RunContentProcess(const ContentMainParams& params,
// loop, but we don't want to leave them hanging around until the app quits. #endif
// Each "main" needs to flush this pool right before it goes into its main
// event loop to get rid of the cruft. #if defined(OS_MAC)
- // We need this pool for all the objects created before we get to the event
- // loop, but we don't want to leave them hanging around until the app quits.
- // Each "main" needs to flush this pool right before it goes into its main
- // event loop to get rid of the cruft.
- autorelease_pool = std::make_unique<base::mac::ScopedNSAutoreleasePool>(); - autorelease_pool = std::make_unique<base::mac::ScopedNSAutoreleasePool>();
- content_main_params.autorelease_pool = autorelease_pool.get(); - content_main_params.autorelease_pool = autorelease_pool.get();
+ params.autorelease_pool = std::make_unique<base::mac::ScopedNSAutoreleasePool>();
InitializeMac(); InitializeMac();
#endif #endif
@@ -310,7 +304,7 @@ int RunContentProcess(const ContentMainParams& params, @@ -310,7 +299,7 @@ int RunContentProcess(const ContentMainParams& params,
ui::RegisterPathProvider(); ui::RegisterPathProvider();
tracker = base::debug::GlobalActivityTracker::Get(); tracker = base::debug::GlobalActivityTracker::Get();
@ -39,7 +42,7 @@ index 2aba28d210db..5413f08af93d 100644
if (exit_code >= 0) { if (exit_code >= 0) {
if (tracker) { if (tracker) {
@@ -369,8 +363,16 @@ int RunContentProcess(const ContentMainParams& params, @@ -369,8 +358,16 @@ int RunContentProcess(const ContentMainParams& params,
if (IsSubprocess()) if (IsSubprocess())
CommonSubprocessInit(); CommonSubprocessInit();
@ -57,7 +60,7 @@ index 2aba28d210db..5413f08af93d 100644
if (tracker) { if (tracker) {
if (exit_code == 0) { if (exit_code == 0) {
tracker->SetProcessPhaseIfEnabled( tracker->SetProcessPhaseIfEnabled(
@@ -381,19 +383,33 @@ int RunContentProcess(const ContentMainParams& params, @@ -381,19 +378,45 @@ int RunContentProcess(const ContentMainParams& params,
tracker->process_data().SetInt("exit-code", exit_code); tracker->process_data().SetInt("exit-code", exit_code);
} }
} }
@ -67,24 +70,38 @@ index 2aba28d210db..5413f08af93d 100644
+void ContentMainShutdown(ContentMainParams& params, +void ContentMainShutdown(ContentMainParams& params,
+ ContentMainRunner* content_main_runner) { + ContentMainRunner* content_main_runner) {
#if defined(OS_MAC) +#if !defined(OS_ANDROID)
- autorelease_pool.reset(); + content_main_runner->Shutdown();
+ params.autorelease_pool.reset(); +#endif
#endif
#if !defined(OS_ANDROID)
content_main_runner->Shutdown();
#endif
+} +}
+ +
+int RunContentProcess(ContentMainParams& params, +int RunContentProcess(ContentMainParams& params,
+ ContentMainRunner* content_main_runner) { + ContentMainRunner* content_main_runner) {
#if defined(OS_MAC)
- autorelease_pool.reset();
+ // We need this pool for all the objects created before we get to the event
+ // loop, but we don't want to leave them hanging around until the app quits.
+ // Each "main" needs to flush this pool right before it goes into its main
+ // event loop to get rid of the cruft.
+ std::unique_ptr<base::mac::ScopedNSAutoreleasePool> autorelease_pool =
+ std::make_unique<base::mac::ScopedNSAutoreleasePool>();
+ params.autorelease_pool = autorelease_pool.get();
#endif
-#if !defined(OS_ANDROID)
- content_main_runner->Shutdown();
+ int exit_code = ContentMainInitialize(params, content_main_runner); + int exit_code = ContentMainInitialize(params, content_main_runner);
+ if (exit_code >= 0) + if (exit_code >= 0)
+ return exit_code; + return exit_code;
+ exit_code = ContentMainRun(params, content_main_runner); + exit_code = ContentMainRun(params, content_main_runner);
+ ContentMainShutdown(params, content_main_runner); +
+#if defined(OS_MAC)
+ params.autorelease_pool = nullptr;
+ autorelease_pool.reset();
#endif
+ ContentMainShutdown(params, content_main_runner);
+
return exit_code; return exit_code;
} }
@ -94,7 +111,7 @@ index 2aba28d210db..5413f08af93d 100644
return RunContentProcess(params, runner.get()); return RunContentProcess(params, runner.get());
} }
diff --git content/app/content_main_runner_impl.cc content/app/content_main_runner_impl.cc diff --git content/app/content_main_runner_impl.cc content/app/content_main_runner_impl.cc
index 811017f5d455..00e4020dea17 100644 index 811017f5d455..57055727e83d 100644
--- content/app/content_main_runner_impl.cc --- content/app/content_main_runner_impl.cc
+++ content/app/content_main_runner_impl.cc +++ content/app/content_main_runner_impl.cc
@@ -48,6 +48,7 @@ @@ -48,6 +48,7 @@
@ -105,15 +122,6 @@ index 811017f5d455..00e4020dea17 100644
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "components/discardable_memory/service/discardable_shared_memory_manager.h" #include "components/discardable_memory/service/discardable_shared_memory_manager.h"
#include "components/download/public/common/download_task_runner.h" #include "components/download/public/common/download_task_runner.h"
@@ -629,7 +630,7 @@ int ContentMainRunnerImpl::Initialize(const ContentMainParams& params) {
#else // !OS_WIN
#if defined(OS_MAC)
- autorelease_pool_ = params.autorelease_pool;
+ autorelease_pool_ = params.autorelease_pool.get();
#endif // defined(OS_MAC)
#if defined(OS_ANDROID)
@@ -1089,6 +1090,11 @@ void ContentMainRunnerImpl::Shutdown() { @@ -1089,6 +1090,11 @@ void ContentMainRunnerImpl::Shutdown() {
is_shutdown_ = true; is_shutdown_ = true;
} }
@ -162,23 +170,10 @@ index 8b829a488773..a69a08869728 100644
if (main_argv) if (main_argv)
setproctitle_init(main_argv); setproctitle_init(main_argv);
diff --git content/public/app/content_main.h content/public/app/content_main.h diff --git content/public/app/content_main.h content/public/app/content_main.h
index 97aac3d0c758..4e7baa790252 100644 index 97aac3d0c758..fc795ae0287f 100644
--- content/public/app/content_main.h --- content/public/app/content_main.h
+++ content/public/app/content_main.h +++ content/public/app/content_main.h
@@ -5,6 +5,7 @@ @@ -68,7 +68,16 @@ struct ContentMainParams {
#ifndef CONTENT_PUBLIC_APP_CONTENT_MAIN_H_
#define CONTENT_PUBLIC_APP_CONTENT_MAIN_H_
+#include <memory>
#include <stddef.h>
#include "base/callback_forward.h"
@@ -64,11 +65,20 @@ struct ContentMainParams {
#if defined(OS_MAC)
// The outermost autorelease pool to pass to main entry points.
- base::mac::ScopedNSAutoreleasePool* autorelease_pool = nullptr;
+ std::unique_ptr<base::mac::ScopedNSAutoreleasePool> autorelease_pool;
#endif #endif
}; };
@ -196,7 +191,7 @@ index 97aac3d0c758..4e7baa790252 100644
ContentMainRunner* content_main_runner); ContentMainRunner* content_main_runner);
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
@@ -91,7 +101,7 @@ ContentMainDelegate* GetContentMainDelegate(); @@ -91,7 +100,7 @@ ContentMainDelegate* GetContentMainDelegate();
// initial setup for every process. The embedder has a chance to customize // initial setup for every process. The embedder has a chance to customize
// startup using the ContentMainDelegate interface. The embedder can also pass // startup using the ContentMainDelegate interface. The embedder can also pass
// in null for |delegate| if they don't want to override default startup. // in null for |delegate| if they don't want to override default startup.