Handle the Web Page Replay related Chrome flags in one place.

BUG=404771

Review URL: https://codereview.chromium.org/595213003

Cr-Commit-Position: refs/heads/master@{#297272}

tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py

@@ -328,9 +328,6 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
 
   def GetBrowserStartupArgs(self):
     args = super(AndroidBrowserBackend, self).GetBrowserStartupArgs()
-    if self.forwarder_factory.does_forwarder_override_dns:
-      args = [arg for arg in args
-              if not arg.startswith('--host-resolver-rules')]
     args.append('--enable-remote-debugging')
     args.append('--disable-fre')
     args.append('--disable-external-intent-requests')

tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py

@@ -19,7 +19,6 @@ from telemetry.core import user_agent
 from telemetry.core import util
 from telemetry.core import web_contents
 from telemetry.core import wpr_modes
-from telemetry.core import wpr_server
 from telemetry.core.backends import browser_backend
 from telemetry.core.backends.chrome import extension_backend
 from telemetry.core.backends.chrome import system_info_backend
@@ -102,12 +101,7 @@ class ChromeBrowserBackend(browser_backend.BrowserBackend):
 
     if self.browser_options.disable_background_networking:
       args.append('--disable-background-networking')
-
-    if self.browser_options.netsim:
-      args.append('--ignore-certificate-errors')
-    elif self.browser_options.wpr_mode != wpr_modes.WPR_OFF:
-      args.extend(wpr_server.GetChromeFlags(self.forwarder_factory.host_ip,
-                                            self.wpr_port_pairs))
+    args.extend(self.GetReplayBrowserStartupArgs())
     args.extend(user_agent.GetChromeUserAgentArgumentFromType(
         self.browser_options.browser_user_agent_type))
 
@@ -133,6 +127,34 @@ class ChromeBrowserBackend(browser_backend.BrowserBackend):
 
     return args
 
+  def GetReplayBrowserStartupArgs(self):
+    if self.browser_options.wpr_mode == wpr_modes.WPR_OFF:
+      return []
+    # Use Chrome's --host-resolver-rules flag if the forwarder does not send
+    # the HTTP requests directly to Replay. Also use --host-resolver-rules
+    # without netsim. With netsim, DNS requests should be sent (to get the
+    # simulated latency), however, the flag causes DNS requests to be skipped.
+    use_host_resolver = (
+        not self.forwarder_factory.does_forwarder_override_dns and
+        not self.browser_options.netsim)
+    http_remote_port = self.wpr_port_pairs.http.remote_port
+    https_remote_port = self.wpr_port_pairs.https.remote_port
+    replay_args = ['--ignore-certificate-errors']
+    if use_host_resolver:
+      replay_args.append('--host-resolver-rules=MAP * %s,EXCLUDE localhost' %
+                         self.forwarder_factory.host_ip)  # replay's host_ip
+    # Force the browser to send HTTP/HTTPS requests to fixed ports if they
+    # are not the standard defaults.
+    # Backstory:
+    #     That allows Telemetry to pick ports that do not require sudo
+    #     and leaves room for multiple instances running simultaneously.
+    #     The default ports are required for network simulation.
+    if http_remote_port != 80:
+      replay_args.append('--testing-fixed-http-port=%s' % http_remote_port)
+    if https_remote_port != 443:  # check if using the default HTTPS port
+      replay_args.append('--testing-fixed-https-port=%s' % https_remote_port)
+    return replay_args
+
   def HasBrowserFinishedLaunching(self):
     try:
       self.Request('', timeout=.1)

tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend_unittest.py

+# Copyright 2013 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.
+
+import unittest
+
+from telemetry.core import forwarders
+from telemetry.core import wpr_modes
+from telemetry.core.backends.chrome import chrome_browser_backend
+
+
+class FakeBrowserOptions(object):
+  def __init__(self, netsim=False, wpr_mode=wpr_modes.WPR_OFF):
+    self.netsim = netsim
+    self.wpr_mode = wpr_mode
+    self.browser_type = 'chrome'
+    self.dont_override_profile = False
+
+
+class FakeForwarderFactory(object):
+  host_ip = '127.0.0.1'
+
+  def __init__(self, does_forwarder_override_dns):
+    self.does_forwarder_override_dns = does_forwarder_override_dns
+
+
+class TestChromeBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
+  # The test does not need to define the abstract methods. pylint: disable=W0223
+
+  def __init__(self, browser_options, does_forwarder_override_dns=False):
+    super(TestChromeBrowserBackend, self).__init__(
+        supports_tab_control=False,
+        supports_extensions=False,
+        browser_options=browser_options,
+        output_profile_path=None,
+        extensions_to_load=None)
+    self._forwarder_factory = FakeForwarderFactory(does_forwarder_override_dns)
+
+
+class ReplayStartupArgsTest(unittest.TestCase):
+  """Test expected inputs for GetReplayBrowserStartupArgs."""
+
+  def testReplayOffGivesEmptyArgs(self):
+    browser_options = FakeBrowserOptions()
+    browser_backend = TestChromeBrowserBackend(browser_options)
+    self.assertEqual([], browser_backend.GetReplayBrowserStartupArgs())
+
+  def testReplayOnGivesBasicArgs(self):
+    browser_options = FakeBrowserOptions(
+        wpr_mode=wpr_modes.WPR_REPLAY,
+        netsim=False)
+    browser_backend = TestChromeBrowserBackend(browser_options)
+    self.assertEqual((0, 0), tuple(browser_backend.wpr_port_pairs.http))
+    self.assertEqual((0, 0), tuple(browser_backend.wpr_port_pairs.https))
+    self.assertEqual(None, browser_backend.wpr_port_pairs.dns)
+
+    # When Replay is started, it fills in the actual port values.
+    # Use different values here to show that the args get the
+    # remote port values.
+    browser_backend.wpr_port_pairs = forwarders.PortPairs(
+        http=forwarders.PortPair(123, 456),
+        https=forwarders.PortPair(234, 567),
+        dns=None)
+    expected_args = [
+        '--host-resolver-rules=MAP * 127.0.0.1,EXCLUDE localhost',
+        '--ignore-certificate-errors',
+        '--testing-fixed-http-port=456',
+        '--testing-fixed-https-port=567'
+        ]
+    self.assertEqual(
+        expected_args,
+        sorted(browser_backend.GetReplayBrowserStartupArgs()))
+
+  def testNetsimGivesNoHostResolver(self):
+    browser_options = FakeBrowserOptions(
+        wpr_mode=wpr_modes.WPR_REPLAY,
+        netsim=True)
+    browser_backend = TestChromeBrowserBackend(browser_options)
+    self.assertEqual((80, 80), tuple(browser_backend.wpr_port_pairs.http))
+    self.assertEqual((443, 443), tuple(browser_backend.wpr_port_pairs.https))
+    self.assertEqual((53, 53), tuple(browser_backend.wpr_port_pairs.dns))
+    expected_args = ['--ignore-certificate-errors']
+    self.assertEqual(
+        expected_args,
+        sorted(browser_backend.GetReplayBrowserStartupArgs()))
+
+  def testForwaderOverridesDnsGivesNoHostResolver(self):
+    # Android with --use-rndis uses standard remote ports and
+    # relies on the forwarder to override DNS resolution.
+    browser_options = FakeBrowserOptions(
+        wpr_mode=wpr_modes.WPR_REPLAY,
+        netsim=False)
+    browser_backend = TestChromeBrowserBackend(
+        browser_options, does_forwarder_override_dns=True)
+    browser_backend.wpr_port_pairs = forwarders.PortPairs(
+        http=forwarders.PortPair(123, 80),
+        https=forwarders.PortPair(234, 443),
+        dns=forwarders.PortPair(345, 53))
+    expected_args = ['--ignore-certificate-errors']
+    self.assertEqual(
+        expected_args,
+        sorted(browser_backend.GetReplayBrowserStartupArgs()))

tools/telemetry/telemetry/core/webpagereplay.py

@@ -26,17 +26,6 @@ LOG_PATH = os.path.join(
     _CHROME_SRC_DIR, 'webpagereplay_logs', 'logs.txt')
 
 
-# Chrome options to make it work with Web Page Replay.
-def GetChromeFlags(replay_host, http_port, https_port):
-  assert replay_host and http_port and https_port, 'All arguments required'
-  return [
-      '--host-resolver-rules=MAP * %s,EXCLUDE localhost' % replay_host,
-      '--testing-fixed-http-port=%s' % http_port,
-      '--testing-fixed-https-port=%s' % https_port,
-      '--ignore-certificate-errors',
-      ]
-
-
 # Signal masks on Linux are inherited from parent processes.  If anything
 # invoking us accidentally masks SIGINT (e.g. by putting a process in the
 # background from a shell script), sending a SIGINT to the child will fail

tools/telemetry/telemetry/core/wpr_server.py

@@ -10,11 +10,6 @@ from telemetry.core import webpagereplay
 # make ReplayServer subclass LocalServer.
 
 
-def GetChromeFlags(replay_host, port_pairs):
-  return webpagereplay.GetChromeFlags(replay_host, port_pairs.http.remote_port,
-                                      port_pairs.https.remote_port)
-
-
 class ReplayServer(object):
   def __init__(self, browser_backend, path, is_record_mode, is_append_mode,
                make_javascript_deterministic):