Remove user agent from HeadlessBrowserContextOptions.

headless/lib/browser/headless_content_browser_client.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "headless/lib/browser/headless_content_browser_client.h"
 
 #include <memory>
 #include <unordered_set>
 
 #include "base/base_switches.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_names.mojom.h"
+#include "headless/app/headless_shell_switches.h"
 #include "headless/grit/headless_lib_resources.h"
 #include "headless/lib/browser/headless_browser_context_impl.h"
 #include "headless/lib/browser/headless_browser_impl.h"
 #include "headless/lib/browser/headless_browser_main_parts.h"
 #include "headless/lib/browser/headless_devtools_manager_delegate.h"
 #include "headless/lib/headless_macros.h"
 #include "storage/browser/quota/quota_settings.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/switches.h"
 
@@ skipping 37 matching lines @@
     return crash_handler;
   }
 }
 
 int GetCrashSignalFD(const base::CommandLine& command_line,
                      const HeadlessBrowser::Options& options) {
   if (!breakpad::IsCrashReporterEnabled())
     return -1;
 
   std::string process_type =
-      command_line.GetSwitchValueASCII(switches::kProcessType);
+      command_line.GetSwitchValueASCII(::switches::kProcessType);
 
-  if (process_type == switches::kRendererProcess) {
+  if (process_type == ::switches::kRendererProcess) {
     static breakpad::CrashHandlerHostLinux* crash_handler =
         CreateCrashHandlerHost(process_type, options);
     return crash_handler->GetDeathSignalSocket();
   }
 
-  if (process_type == switches::kPpapiPluginProcess) {
+  if (process_type == ::switches::kPpapiPluginProcess) {
     static breakpad::CrashHandlerHostLinux* crash_handler =
         CreateCrashHandlerHost(process_type, options);
     return crash_handler->GetDeathSignalSocket();
   }
 
-  if (process_type == switches::kGpuProcess) {
+  if (process_type == ::switches::kGpuProcess) {
     static breakpad::CrashHandlerHostLinux* crash_handler =
         CreateCrashHandlerHost(process_type, options);
     return crash_handler->GetDeathSignalSocket();
   }
 
   return -1;
 }
 #endif  // defined(HEADLESS_USE_BREAKPAD)
 
 }  // namespace
@@ skipping 75 matching lines @@
   int crash_signal_fd = GetCrashSignalFD(command_line, *browser_->options());
   if (crash_signal_fd >= 0)
     mappings->Share(kCrashDumpSignal, crash_signal_fd);
 #endif  // defined(HEADLESS_USE_BREAKPAD)
 }
 #endif  // defined(OS_POSIX) && !defined(OS_MACOSX)
 
 void HeadlessContentBrowserClient::AppendExtraCommandLineSwitches(
     base::CommandLine* command_line,
     int child_process_id) {
-  command_line->AppendSwitch(switches::kHeadless);
+  command_line->AppendSwitch(::switches::kHeadless);
+  command_line->AppendSwitchNative(switches::kUserAgent,

[Sami, 2017/03/27 15:52:08]

I'm wondering how things worked before when we didn't tell the renderer about
the user agent? Would you mind adding a simple test for this?

In any case something isn't quite wired up right because the user agent is an
option on the BrowserContext[1], which means you can have multiple user agents
in one headless instance, whereas this code uses a single global user agent.
Does it look like we can support multiple user agents in the first place?

[1]
https://cs.chromium.org/chromium/src/headless/lib/browser/headless_browser_co...

[irisu, 2017/03/29 05:51:33]

It looks as though the user agent header is set using
HeadlessContentClient::GetUserAgent(), which takes it from the
HeadlessBrowser::Options rather than the BrowserContextOptions.
https://cs.chromium.org/chromium/src/headless/lib/headless_content_client.cc?...

I checked this by modifying that method to return a hardcoded string and
running: cr run headless_shell --remote-debugging-port=9222 --disable-gpu
http://www.whatsmyua.com/

To my understanding, there is only one instance of the HeadlessContentClient for
each HeadlessBrowser, is that correct?

[Sami, 2017/03/29 10:46:07]

That's right, that's the default user agent for the entire browser. Maybe we
should be using this instead to override the user agent:
https://chromedevtools.github.io/debugger-protocol-viewer/tot/Network/#method...

Also, given this we should probably remove the per-context user agent option.

[irisu, 2017/03/30 07:06:07]

Removed user agent from BrowserContextOptions.

I've created another patch which uses devtools instead of the command line
switch to set the user agent. Is that what you had in mind? If so, I can remove
the flag from this patch and this patch would only remove the user agent from
BrowserContextOptions.
https://codereview.chromium.org/2785083002

+                                   browser_->options()->user_agent);
 #if defined(HEADLESS_USE_BREAKPAD)
   // This flag tells child processes to also turn on crash reporting.
   if (breakpad::IsCrashReporterEnabled())
-    command_line->AppendSwitch(switches::kEnableCrashReporter);
+    command_line->AppendSwitch(::switches::kEnableCrashReporter);
 #endif  // defined(HEADLESS_USE_BREAKPAD)
 }
 
 }  // namespace headless