[content] Make ContentMainParams and MainFunctionParams move-only

This is taking care of a long-standing TODO to move these OnceClosure
holders rather than copy them around with their OnceClosure* members.

This is a precursor to
https://chromium-review.googlesource.com/c/chromium/src/+/3187153/35#message-fcc92e9f85e73f0e5ba6c03610a95cda8736f1f9
which highlighted a problem where some tests see a non-null
MainFunctionParams::ui_task but running the closure results in a UAF.
Logs show that the test hitting the UAF is not the one setting this
field. This CL makes that impossible and fixes the issue in the
follow-up CL.

This CL is intended to be a logical no-op.
This CL touches a lot of files and must happen all at once.

The core change is that ContentMainParams and MainFunctionParams's
moveable fields (ui_task, created_main_parts_closure, and startup_data)
are now held by moveable types rather than raw pointers.

This trickles in the following chain:
main() (in various *_main.cc)
(or SetUp() in !OS_ANDROID browser_test_base.cc)
  -> ContentMain()
    -> ContentMainRunnerImpl::Initialize()
      (forwards arg into MainFunctionParams)
      -> RunBrowser()
        -> BrowserMain()
          -> BrowserMainRunnerImpl::Initialize()
            -> BrowserMainLoop (stores MainFunctionParams)
            -> BrowserMainLoop::Init
              -> ContentBrowserClient::CreateBrowserMainParts()
                -> (Embedder)ContentBrowserClient::CreateBrowserMainParts()
                  -> (Embedder)BrowserMainParts(Platform)
                    -> (Embedder)BrowserMainParts
      -> RunOtherNamedProcessTypeMain()
        -> (Embedder)ContentMainDelegate::RunProcess()
        (or)
        -> FooMain() (kMainFunctions)
        (or)
        -> RunZygote()
          (creates its own MainFunctionParams)
          -> (Embedder)ContentMainDelegate::RunProcess()
(on OS_ANDROID, browser_test_base.cc calls directly into
 ContentMainDelegate::RunProcess())

Few of these needed the params after passing them down so a move-only
model was simple to adapt (even if invasive). The few exceptions like
BrowserMainRunnerImpl::Initialize consuming |created_main_parts_closure|
are better off in the new model (where they take the OnceClosure before
passing down the params) because that prevents others down the chain
from having access to a OnceClosure they shouldn't invoke anyways.

Noteworthy:
 - ContentMainDelegate::RunProcess():
   Returned an exit_code >= 0 to indicate the embedder elected to handle
   the run request given these params. With move-only semantics it is
   necessary to return the params back when the embedder declines
   handling this run request. An absl::variant return value is used
   to satisfy this requirement.

- content/public/test/test_launcher.h : GetContentMainParams():
  Becomes CopyContentMainParams() and only exposes a copy of copyable
  params. Uses new ContentMainParams::ShallowCopyForTesting() which
  verifies that moveable fields are still null by that time as should be
  the case in the order browser tests are initialized.

- MainFunctionParams::command_line being const& violated the style-guide
  rule to "avoid defining functions that require a const reference
  parameter to outlive the call". This also prevented moving. The type
  was hence switched to a const CommandLine*.

- BUILD.gn changes for nacl_helper_win_64 which requires static linking
  of its minimal //content deps (was previously missing a dep but was
  getting away with it because MainFunctionParams was .h only; required
  now with .cc). This was already done for static_switches and this CL
  adds static_main_function_params, reusing a similar static_features
  target that already existed but was no longer required in
  /c/nacl/broker, cleaning that up by replacing rather than copying that
  target's definition in this CL.

- ContentMainParams::minimal_browser_mode was weirdly passed as a
  parameter to ContentMainRunner::Run(bool start_minimal_browser) but
  that method also has access to the ContentMainParams originally passed
  via ContentMainRunner::Init(). Passing the param again from Run()
  would be a use-after-move in content_main.cc, instead
  content_main_runner_impl.cc was updated to use the param it already
  has in store.

Bug: 1175074
Change-Id: I3af90505525e426383c59107a3903d645d455682
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3244976
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Owners-Override: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940478}

android_webview/browser/aw_content_browser_client.cc

@@ -301,7 +301,7 @@ AwBrowserContext* AwContentBrowserClient::InitBrowserContext() {
 
 std::unique_ptr<content::BrowserMainParts>
 AwContentBrowserClient::CreateBrowserMainParts(
-    const content::MainFunctionParams& parameters) {
+    content::MainFunctionParams /*parameters*/) {
   return std::make_unique<AwBrowserMainParts>(this);
 }
 

android_webview/browser/aw_content_browser_client.h

@@ -76,7 +76,7 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
       cert_verifier::mojom::CertVerifierCreationParams*
           cert_verifier_creation_params) override;
   std::unique_ptr<content::BrowserMainParts> CreateBrowserMainParts(
-      const content::MainFunctionParams& parameters) override;
+      content::MainFunctionParams parameters) override;
   content::WebContentsViewDelegate* GetWebContentsViewDelegate(
       content::WebContents* web_contents) override;
   void RenderProcessWillLaunch(content::RenderProcessHost* host) override;

android_webview/lib/aw_main_delegate.cc

@@ -53,6 +53,7 @@
 #include "content/public/common/content_descriptor_keys.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/main_function_params.h"
 #include "device/base/features.h"
 #include "gin/public/isolate_holder.h"
 #include "gin/v8_initializer.h"
@@ -330,20 +331,18 @@ void AwMainDelegate::PreSandboxStartup() {
   sdk_int_key.Set(base::NumberToString(android_build_info->sdk_int()));
 }
 
-int AwMainDelegate::RunProcess(
+absl::variant<int, content::MainFunctionParams> AwMainDelegate::RunProcess(
     const std::string& process_type,
-    const content::MainFunctionParams& main_function_params) {
+    content::MainFunctionParams main_function_params) {
   // Defer to the default main method outside the browser process.
   if (!process_type.empty())
-    return -1;
+    return std::move(main_function_params);
 
   browser_runner_ = content::BrowserMainRunner::Create();
-  int exit_code = browser_runner_->Initialize(main_function_params);
+  int exit_code = browser_runner_->Initialize(std::move(main_function_params));
   // We do not expect Initialize() to ever fail in AndroidWebView. On success
   // it returns a negative value but we do not want to use that on Android.
   DCHECK_LT(exit_code, 0);
-  // Return 0 so that we do NOT trigger the default behavior. On Android, the
-  // UI message loop is managed by the Java application.
   return 0;
 }
 

android_webview/lib/aw_main_delegate.h

@@ -44,9 +44,9 @@ class AwMainDelegate : public content::ContentMainDelegate {
   // content::ContentMainDelegate implementation:
   bool BasicStartupComplete(int* exit_code) override;
   void PreSandboxStartup() override;
-  int RunProcess(
+  absl::variant<int, content::MainFunctionParams> RunProcess(
       const std::string& process_type,
-      const content::MainFunctionParams& main_function_params) override;
+      content::MainFunctionParams main_function_params) override;
   void ProcessExiting(const std::string& process_type) override;
   bool ShouldCreateFeatureList() override;
   void PostEarlyInitialization(bool is_running_tests) override;

base/memory/shared_memory_hooks.h

@@ -14,7 +14,7 @@
 namespace content {
 struct MainFunctionParams;
 }  // namespace content
-int CloudPrintServiceProcessMain(const content::MainFunctionParams& parameters);
+int CloudPrintServiceProcessMain(content::MainFunctionParams parameters);
 
 namespace mojo {
 
@@ -31,7 +31,7 @@ class SharedMemoryHooks {
  private:
   friend class SharedMemoryHooksTest;
   friend int ::CloudPrintServiceProcessMain(
-      const content::MainFunctionParams& parameters);
+      content::MainFunctionParams parameters);
   friend mojo::SharedMemoryUtils;
 
   // Allows shared memory region creation to be hooked. Useful for sandboxed

chrome/app/android/chrome_main_delegate_android.cc

@@ -77,13 +77,14 @@ void ChromeMainDelegateAndroid::SecureDataDirectory() {
   }
 }
 
-int ChromeMainDelegateAndroid::RunProcess(
+absl::variant<int, content::MainFunctionParams>
+ChromeMainDelegateAndroid::RunProcess(
     const std::string& process_type,
-    const content::MainFunctionParams& main_function_params) {
+    content::MainFunctionParams main_function_params) {
   TRACE_EVENT0("startup", "ChromeMainDelegateAndroid::RunProcess");
   // Defer to the default main method outside the browser process.
   if (!process_type.empty())
-    return -1;
+    return std::move(main_function_params);
 
   SecureDataDirectory();
 
@@ -110,10 +111,9 @@ int ChromeMainDelegateAndroid::RunProcess(
     browser_runner_ = content::BrowserMainRunner::Create();
   }
 
-  int exit_code = browser_runner_->Initialize(main_function_params);
+  int exit_code = browser_runner_->Initialize(std::move(main_function_params));
   // On Android we do not run BrowserMain(), so the above initialization of a
-  // BrowserMainRunner is all we want to occur. Return >= 0 to avoid running
-  // BrowserMain, while preserving any error codes > 0.
+  // BrowserMainRunner is all we want to occur. Preserve any error codes > 0.
   if (exit_code > 0)
     return exit_code;
   return 0;

chrome/app/android/chrome_main_delegate_android.h

@@ -33,9 +33,9 @@ class ChromeMainDelegateAndroid : public ChromeMainDelegate {
 
   bool BasicStartupComplete(int* exit_code) override;
   void PreSandboxStartup() override;
-  int RunProcess(
+  absl::variant<int, content::MainFunctionParams> RunProcess(
       const std::string& process_type,
-      const content::MainFunctionParams& main_function_params) override;
+      content::MainFunctionParams main_function_params) override;
   void ProcessExiting(const std::string& process_type) override;
 
  private:

chrome/app/chrome_main.cc

@@ -148,7 +148,7 @@ int ChromeMain(int argc, const char** argv) {
 #if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_MAC) || \
     defined(OS_WIN)
     if (command_line->HasSwitch(switches::kHeadless))
-      return headless::HeadlessShellMain(params);
+      return headless::HeadlessShellMain(std::move(params));
 #endif  // defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_MAC) ||
         // defined(OS_WIN)
   }
@@ -169,7 +169,7 @@ int ChromeMain(int argc, const char** argv) {
   }
 #endif
 
-  int rv = content::ContentMain(params);
+  int rv = content::ContentMain(std::move(params));
 
   return rv;
 }

chrome/app/chrome_main_delegate.cc

@@ -64,6 +64,7 @@
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_paths.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/main_function_params.h"
 #include "content/public/common/profiling.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/constants.h"
@@ -197,10 +198,10 @@ base::LazyInstance<ChromeContentRendererClient>::DestructorAtExit
 base::LazyInstance<ChromeContentUtilityClient>::DestructorAtExit
     g_chrome_content_utility_client = LAZY_INSTANCE_INITIALIZER;
 
-extern int NaClMain(const content::MainFunctionParams&);
+extern int NaClMain(content::MainFunctionParams);
 
 #if !defined(OS_CHROMEOS)
-extern int CloudPrintServiceProcessMain(const content::MainFunctionParams&);
+extern int CloudPrintServiceProcessMain(content::MainFunctionParams);
 #endif
 
 const char* const ChromeMainDelegate::kNonWildcardDomainNonPortSchemes[] = {
@@ -383,7 +384,7 @@ void SetUpProfilingShutdownHandler() {
 
 struct MainFunction {
   const char* name;
-  int (*function)(const content::MainFunctionParams&);
+  int (*function)(content::MainFunctionParams);
 };
 
 // Initializes the user data dir. Must be called before InitializeLocalState().
@@ -1211,9 +1212,9 @@ void ChromeMainDelegate::SandboxInitialized(const std::string& process_type) {
 #endif
 }
 
-int ChromeMainDelegate::RunProcess(
+absl::variant<int, content::MainFunctionParams> ChromeMainDelegate::RunProcess(
     const std::string& process_type,
-    const content::MainFunctionParams& main_function_params) {
+    content::MainFunctionParams main_function_params) {
 // ANDROID doesn't support "service", so no CloudPrintServiceProcessMain, and
 // arraysize doesn't support empty array. So we comment out the block for
 // Android.
@@ -1241,11 +1242,11 @@ int ChromeMainDelegate::RunProcess(
 
   for (size_t i = 0; i < base::size(kMainFunctions); ++i) {
     if (process_type == kMainFunctions[i].name)
-      return kMainFunctions[i].function(main_function_params);
+      return kMainFunctions[i].function(std::move(main_function_params));
   }
 #endif  // !defined(OS_ANDROID)
 
-  return -1;
+  return std::move(main_function_params);
 }
 
 void ChromeMainDelegate::ProcessExiting(const std::string& process_type) {

chrome/app/chrome_main_delegate.h

@@ -53,9 +53,9 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
   bool BasicStartupComplete(int* exit_code) override;
   void PreSandboxStartup() override;
   void SandboxInitialized(const std::string& process_type) override;
-  int RunProcess(
+  absl::variant<int, content::MainFunctionParams> RunProcess(
       const std::string& process_type,
-      const content::MainFunctionParams& main_function_params) override;
+      content::MainFunctionParams main_function_params) override;
   void ProcessExiting(const std::string& process_type) override;
 #if defined(OS_LINUX) || defined(OS_CHROMEOS)
   void ZygoteStarting(std::vector<std::unique_ptr<content::ZygoteForkDelegate>>*

chrome/browser/ash/chrome_browser_main_parts_ash.cc

@@ -532,9 +532,9 @@ class DBusServices {
 // ChromeBrowserMainPartsAsh ---------------------------------------------------
 
 ChromeBrowserMainPartsAsh::ChromeBrowserMainPartsAsh(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : ChromeBrowserMainPartsLinux(parameters, startup_data) {}
+    : ChromeBrowserMainPartsLinux(std::move(parameters), startup_data) {}
 
 ChromeBrowserMainPartsAsh::~ChromeBrowserMainPartsAsh() {
   // To be precise, logout (browser shutdown) is not yet done, but the
@@ -1077,7 +1077,7 @@ void ChromeBrowserMainPartsAsh::PostProfileInit() {
 
   manager->SetState(session_manager->GetDefaultIMEState(profile()));
 
-  bool is_running_test = parameters().ui_task != nullptr;
+  bool is_running_test = !!parameters().ui_task;
   g_browser_process->platform_part()->session_manager()->Initialize(
       parsed_command_line(), profile(), is_running_test);
 

chrome/browser/ash/chrome_browser_main_parts_ash.h

@@ -120,7 +120,7 @@ class DarkResumeController;
 // src/ash or chrome/browser/ui/ash.
 class ChromeBrowserMainPartsAsh : public ChromeBrowserMainPartsLinux {
  public:
-  ChromeBrowserMainPartsAsh(const content::MainFunctionParams& parameters,
+  ChromeBrowserMainPartsAsh(content::MainFunctionParams parameters,
                             StartupData* startup_data);
 
   ChromeBrowserMainPartsAsh(const ChromeBrowserMainPartsAsh&) = delete;

chrome/browser/chrome_browser_main.cc

@@ -500,10 +500,10 @@ bool ProcessSingletonNotificationCallback(
 // BrowserMainParts ------------------------------------------------------------
 
 ChromeBrowserMainParts::ChromeBrowserMainParts(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : parameters_(parameters),
-      parsed_command_line_(parameters.command_line),
+    : parameters_(std::move(parameters)),
+      parsed_command_line_(*parameters.command_line),
       should_call_pre_main_loop_start_startup_on_variations_service_(
           !parameters.ui_task),
       startup_data_(startup_data) {
@@ -1097,7 +1097,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRun() {
 //   PostProfileInit()
 //   ... additional setup
 //   PreBrowserStart()
-//   ... browser_creator_->Start (OR parameters().ui_task->Run())
+//   ... browser_creator_->Start (OR parameters_.ui_task->Run())
 //   PostBrowserStart()
 
 void ChromeBrowserMainParts::PreProfileInit() {
@@ -1433,7 +1433,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   // This step is costly and is already measured in Startup.CreateFirstProfile
   // and more directly Profile.CreateAndInitializeProfile.
   StartupProfileInfo profile_info = CreatePrimaryProfile(
-      parameters(), /*cur_dir=*/base::FilePath(), parsed_command_line());
+      parameters_, /*cur_dir=*/base::FilePath(), parsed_command_line());
 
   profile_ = profile_info.profile;
   if (profile_info.mode == StartupProfileMode::kError)
@@ -1726,8 +1726,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
 #if defined(OS_MAC)
     // Call Recycle() here as late as possible, before going into the loop
     // because Start() will add things to it while creating the main window.
-    if (parameters().autorelease_pool)
-      parameters().autorelease_pool->Recycle();
+    if (parameters_.autorelease_pool)
+      parameters_.autorelease_pool->Recycle();
 #endif  // defined(OS_MAC)
 
     // Transfer ownership of the browser's lifetime to the BrowserProcess.
@@ -1744,9 +1744,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   // The ui_task can be injected by tests to replace the main message loop.
   // In that case we Run() it here, and set a flag to avoid running the main
   // message loop later, as the test will do so as needed from the |ui_task|.
-  if (parameters().ui_task) {
-    std::move(*parameters().ui_task).Run();
-    delete parameters().ui_task;
+  if (parameters_.ui_task) {
+    std::move(parameters_.ui_task).Run();
     run_message_loop_ = false;
   }
 
@@ -1840,7 +1839,7 @@ void ChromeBrowserMainParts::PostMainMessageLoopRun() {
   // Some tests don't set parameters.ui_task, so they started translate
   // language fetch that was never completed so we need to cleanup here
   // otherwise it will be done by the destructor in a wrong thread.
-  TranslateService::Shutdown(!parameters().ui_task);
+  TranslateService::Shutdown(!parameters_.ui_task);
 
   if (notify_result_ == ProcessSingleton::PROCESS_NONE)
     process_singleton_->Cleanup();

chrome/browser/chrome_browser_main.h

@@ -60,7 +60,7 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
 #endif
 
  protected:
-  ChromeBrowserMainParts(const content::MainFunctionParams& parameters,
+  ChromeBrowserMainParts(content::MainFunctionParams parameters,
                          StartupData* startup_data);
 
   // content::BrowserMainParts overrides.
@@ -139,7 +139,7 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
 
   // Members initialized on construction ---------------------------------------
 
-  const content::MainFunctionParams parameters_;
+  content::MainFunctionParams parameters_;
   // TODO(sky): remove this. This class (and related calls), may mutate the
   // CommandLine, so it is misleading keeping a const ref here.
   const base::CommandLine& parsed_command_line_;

chrome/browser/chrome_browser_main_android.cc

@@ -31,9 +31,9 @@
 #include "ui/base/ui_base_paths.h"
 
 ChromeBrowserMainPartsAndroid::ChromeBrowserMainPartsAndroid(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : ChromeBrowserMainParts(parameters, startup_data) {}
+    : ChromeBrowserMainParts(std::move(parameters), startup_data) {}
 
 ChromeBrowserMainPartsAndroid::~ChromeBrowserMainPartsAndroid() {
 }

chrome/browser/chrome_browser_main_android.h

@@ -12,7 +12,7 @@
 
 class ChromeBrowserMainPartsAndroid : public ChromeBrowserMainParts {
  public:
-  ChromeBrowserMainPartsAndroid(const content::MainFunctionParams& parameters,
+  ChromeBrowserMainPartsAndroid(content::MainFunctionParams parameters,
                                 StartupData* startup_data);
 
   ChromeBrowserMainPartsAndroid(const ChromeBrowserMainPartsAndroid&) = delete;

chrome/browser/chrome_browser_main_linux.cc

@@ -45,9 +45,9 @@
 #endif
 
 ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : ChromeBrowserMainPartsPosix(parameters, startup_data) {}
+    : ChromeBrowserMainPartsPosix(std::move(parameters), startup_data) {}
 
 ChromeBrowserMainPartsLinux::~ChromeBrowserMainPartsLinux() {
 }

chrome/browser/chrome_browser_main_linux.h

@@ -13,7 +13,7 @@
 
 class ChromeBrowserMainPartsLinux : public ChromeBrowserMainPartsPosix {
  public:
-  ChromeBrowserMainPartsLinux(const content::MainFunctionParams& parameters,
+  ChromeBrowserMainPartsLinux(content::MainFunctionParams parameters,
                               StartupData* startup_data);
 
   ChromeBrowserMainPartsLinux(const ChromeBrowserMainPartsLinux&) = delete;

chrome/browser/chrome_browser_main_mac.h

@@ -10,7 +10,7 @@
 
 class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {
  public:
-  ChromeBrowserMainPartsMac(const content::MainFunctionParams& parameters,
+  ChromeBrowserMainPartsMac(content::MainFunctionParams parameters,
                             StartupData* startup_data);
 
   ChromeBrowserMainPartsMac(const ChromeBrowserMainPartsMac&) = delete;

chrome/browser/chrome_browser_main_mac.mm

@@ -52,9 +52,9 @@
 // ChromeBrowserMainPartsMac ---------------------------------------------------
 
 ChromeBrowserMainPartsMac::ChromeBrowserMainPartsMac(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : ChromeBrowserMainPartsPosix(parameters, startup_data) {}
+    : ChromeBrowserMainPartsPosix(std::move(parameters), startup_data) {}
 
 ChromeBrowserMainPartsMac::~ChromeBrowserMainPartsMac() {
 }

chrome/browser/chrome_browser_main_parts_fuchsia.cc

@@ -198,9 +198,9 @@ class ViewProviderScenic : public fuchsia::ui::app::ViewProvider {
 }  // namespace
 
 ChromeBrowserMainPartsFuchsia::ChromeBrowserMainPartsFuchsia(
-    const content::MainFunctionParams& parameters,
+    content::MainFunctionParams parameters,
     StartupData* startup_data)
-    : ChromeBrowserMainParts(parameters, startup_data) {}
+    : ChromeBrowserMainParts(std::move(parameters), startup_data) {}
 
 ChromeBrowserMainPartsFuchsia::~ChromeBrowserMainPartsFuchsia() = default;