Fix duplicate command line switches in GPU process

AppendExtraCommandLineSwitches was called twice during GPU process launch and
it caused some duplicate command line switches in GPU process.
In GpuProcessHost::LaunchGpuProcess, the second AppendExtraCommandLineSwitches
call was from BrowserChildProcessHostImpl::Launch().

The solution is to divide BrowserChildProcessHostImpl::Launch() into two parts
- AppendExtraCommandLineSwitches and LaunchWithoutExtraComandLineSwitches. The
GPU process will now call LaunchWithoutExtraComandLineSwitches to avoid running
AppendExtraCommandLineSwitches twice.

Bug:899361

Change-Id: I4ddfeaaf9624aac68e9ab68aabac6ed4e83f7a03
Reviewed-on: https://chromium-review.googlesource.com/c/1316660
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607043}

content/browser/browser_child_process_host_impl.cc

@@ -130,7 +130,7 @@ base::PortProvider* BrowserChildProcessHost::GetPortProvider() {
 
 // static
 BrowserChildProcessHostImpl::BrowserChildProcessList*
-    BrowserChildProcessHostImpl::GetIterator() {
+BrowserChildProcessHostImpl::GetIterator() {
   return g_child_process_list.Pointer();
 }
 
@@ -240,45 +240,10 @@ void BrowserChildProcessHostImpl::Launch(
     std::unique_ptr<SandboxedProcessLauncherDelegate> delegate,
     std::unique_ptr<base::CommandLine> cmd_line,
     bool terminate_on_shutdown) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   GetContentClient()->browser()->AppendExtraCommandLineSwitches(cmd_line.get(),
                                                                 data_.id);
-
-  const base::CommandLine& browser_command_line =
-      *base::CommandLine::ForCurrentProcess();
-  static const char* const kForwardSwitches[] = {
-      service_manager::switches::kDisableInProcessStackTraces,
-      switches::kDisableBackgroundTasks,
-      switches::kDisableLogging,
-      switches::kEnableLogging,
-      switches::kIPCConnectionTimeout,
-      switches::kLoggingLevel,
-      switches::kTraceToConsole,
-      switches::kV,
-      switches::kVModule,
-  };
-  cmd_line->CopySwitchesFrom(browser_command_line, kForwardSwitches,
-                             arraysize(kForwardSwitches));
-
-  if (child_connection_) {
-    cmd_line->AppendSwitchASCII(
-        service_manager::switches::kServiceRequestChannelToken,
-        child_connection_->service_token());
-  }
-
-  // All processes should have a non-empty metrics name.
-  DCHECK(!data_.metrics_name.empty());
-
-  notify_child_disconnected_ = true;
-  child_process_.reset(new ChildProcessLauncher(
-      std::move(delegate), std::move(cmd_line), data_.id, this,
-      std::move(mojo_invitation_),
-      base::Bind(&BrowserChildProcessHostImpl::OnMojoError,
-                 weak_factory_.GetWeakPtr(),
-                 base::ThreadTaskRunnerHandle::Get()),
-      terminate_on_shutdown));
-  ShareMetricsAllocatorToProcess();
+  LaunchWithoutExtraCommandLineSwitches(
+      std::move(delegate), std::move(cmd_line), terminate_on_shutdown);
 }
 
 const ChildProcessData& BrowserChildProcessHostImpl::GetData() const {
@@ -338,6 +303,47 @@ void BrowserChildProcessHostImpl::AddFilter(BrowserMessageFilter* filter) {
   child_process_host_->AddFilter(filter->GetFilter());
 }
 
+void BrowserChildProcessHostImpl::LaunchWithoutExtraCommandLineSwitches(
+    std::unique_ptr<SandboxedProcessLauncherDelegate> delegate,
+    std::unique_ptr<base::CommandLine> cmd_line,
+    bool terminate_on_shutdown) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  const base::CommandLine& browser_command_line =
+      *base::CommandLine::ForCurrentProcess();
+  static const char* const kForwardSwitches[] = {
+      service_manager::switches::kDisableInProcessStackTraces,
+      switches::kDisableBackgroundTasks,
+      switches::kDisableLogging,
+      switches::kEnableLogging,
+      switches::kIPCConnectionTimeout,
+      switches::kLoggingLevel,
+      switches::kTraceToConsole,
+      switches::kV,
+      switches::kVModule,
+  };
+  cmd_line->CopySwitchesFrom(browser_command_line, kForwardSwitches,
+                             base::size(kForwardSwitches));
+
+  if (child_connection_) {
+    cmd_line->AppendSwitchASCII(
+        service_manager::switches::kServiceRequestChannelToken,
+        child_connection_->service_token());
+  }
+
+  // All processes should have a non-empty metrics name.
+  DCHECK(!data_.metrics_name.empty());
+
+  notify_child_disconnected_ = true;
+  child_process_.reset(new ChildProcessLauncher(
+      std::move(delegate), std::move(cmd_line), data_.id, this,
+      std::move(mojo_invitation_),
+      base::BindRepeating(&BrowserChildProcessHostImpl::OnMojoError,
+                          weak_factory_.GetWeakPtr(),
+                          base::ThreadTaskRunnerHandle::Get()),
+      terminate_on_shutdown));
+  ShareMetricsAllocatorToProcess();
+}
+
 void BrowserChildProcessHostImpl::BindInterface(
     const std::string& interface_name,
     mojo::ScopedMessagePipeHandle interface_pipe) {

content/browser/browser_child_process_host_impl.h

@@ -104,6 +104,15 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl
   // Adds an IPC message filter.
   void AddFilter(BrowserMessageFilter* filter);
 
+  // Unlike Launch(), AppendExtraCommandLineSwitches will not be called
+  // in this function. If AppendExtraCommandLineSwitches has been called before
+  // reaching launch, call this function instead so the command line switches
+  // won't be appended twice
+  void LaunchWithoutExtraCommandLineSwitches(
+      std::unique_ptr<SandboxedProcessLauncherDelegate> delegate,
+      std::unique_ptr<base::CommandLine> cmd_line,
+      bool terminate_on_shutdown);
+
   static void HistogramBadMessageTerminated(ProcessType process_type);
 
   BrowserChildProcessHostDelegate* delegate() const { return delegate_; }

content/browser/gpu/gpu_process_host.cc

@@ -1066,6 +1066,9 @@ bool GpuProcessHost::LaunchGpuProcess() {
   cmd_line->CopySwitchesFrom(browser_command_line, gpu_workarounds.data(),
                              gpu_workarounds.size());
 
+  // Because AppendExtraCommandLineSwitches is called here, we should call
+  // LaunchWithoutExtraCommandLineSwitches() instead of Launch for gpu process
+  // launch below.
   GetContentClient()->browser()->AppendExtraCommandLineSwitches(
       cmd_line.get(), process_->GetData().id);
 
@@ -1089,7 +1092,13 @@ bool GpuProcessHost::LaunchGpuProcess() {
   if (crashed_before_)
     delegate->DisableAppContainer();
 #endif  // defined(OS_WIN)
-  process_->Launch(std::move(delegate), std::move(cmd_line), true);
+
+  // Do not call process_->Launch() here.
+  // AppendExtraCommandLineSwitches will be called again in process_->Launch(),
+  // Call LaunchWithoutExtraCommandLineSwitches() so the command line switches
+  // will not be appended twice.
+  process_->LaunchWithoutExtraCommandLineSwitches(std::move(delegate),
+                                                  std::move(cmd_line), true);
   process_launched_ = true;
 
   if (kind_ == GPU_PROCESS_KIND_SANDBOXED) {