Consolidate logic for --host-resolver-rules so it can be moved to platform backend.

BUG=423962

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

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

5 files changed, 71 insertions, 25 deletions

diff --git a/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py b/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py
index a1fa1ff..20d29cec 100644
--- a/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py
@@ -123,16 +123,16 @@ class ChromeBrowserBackend(browser_backend.BrowserBackend):
 
     return args
 
-  @property
-  def _use_host_resolver_rules(self):
-    """Returns True if need --host-resolver-rules to send requests to replay."""
-    if self.browser_options.netsim:
-      # Avoid --host-resolver-rules with netsim because it causes Chrome to
-      # skip DNS requests. With netsim, we want to exercise DNS requests.
-      return False
+  def _UseHostResolverRules(self):
+    """Returns True to add --host-resolver-rules to send requests to replay."""
     if self.forwarder_factory.does_forwarder_override_dns:
-      # Avoid --host-resolver-rules when the forwarder can map DNS requests
-      # from devices to the replay DNS port on the host running Telemetry.
+      # Avoid --host-resolver-rules when the forwarder will map DNS requests
+      # from the target platform to replay (on the host platform).
+      # This allows the browser to exercise DNS requests.
+      return False
+    if self.browser_options.netsim and self._platform_backend.is_host_platform:
+      # Avoid --host-resolver-rules when replay will configure the platform to
+      # resolve hosts to replay.
       # This allows the browser to exercise DNS requests.
       return False
     return True
@@ -151,7 +151,7 @@ class ChromeBrowserBackend(browser_backend.BrowserBackend):
       # Ignore certificate errors if the platform backend has not created
       # and installed a root certificate.
       replay_args.append('--ignore-certificate-errors')
-    if self._use_host_resolver_rules:
+    if self._UseHostResolverRules():
       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
diff --git a/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend_unittest.py b/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend_unittest.py
index 172a241..d66982e 100644
--- a/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend_unittest.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend_unittest.py
@@ -9,6 +9,11 @@ from telemetry.core import wpr_modes
 from telemetry.core.backends.chrome import chrome_browser_backend
 
 
+class FakePlatformBackend(object):
+  def __init__(self, is_host_platform):
+    self.is_host_platform = is_host_platform
+
+
 class FakeBrowserOptions(object):
   def __init__(self, netsim=False, wpr_mode=wpr_modes.WPR_OFF):
     self.netsim = netsim
@@ -27,9 +32,10 @@ class FakeForwarderFactory(object):
 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):
+  def __init__(self, browser_options, does_forwarder_override_dns=False,
+               is_running_locally=False):
     super(TestChromeBrowserBackend, self).__init__(
-        platform_backend=None,
+        platform_backend=FakePlatformBackend(is_running_locally),
         supports_tab_control=False,
         supports_extensions=False,
         browser_options=browser_options,
@@ -46,14 +52,18 @@ class ReplayStartupArgsTest(unittest.TestCase):
     browser_backend = TestChromeBrowserBackend(browser_options)
     self.assertEqual([], browser_backend.GetReplayBrowserStartupArgs())
 
-  def testReplayOnGivesBasicArgs(self):
+  def BasicArgsHelper(self, is_running_locally):
+    # Covers Android without RNDIS and CrOS.
     browser_options = FakeBrowserOptions(
         wpr_mode=wpr_modes.WPR_REPLAY,
         netsim=False)
-    browser_backend = TestChromeBrowserBackend(browser_options)
+    browser_backend = TestChromeBrowserBackend(
+        browser_options,
+        does_forwarder_override_dns=False,
+        is_running_locally=is_running_locally)
     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)
+    self.assertIsNone(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
@@ -72,11 +82,19 @@ class ReplayStartupArgsTest(unittest.TestCase):
         expected_args,
         sorted(browser_backend.GetReplayBrowserStartupArgs()))
 
-  def testNetsimGivesNoHostResolver(self):
+  def testBasicArgs(self):
+    # The result is the same regardless of whether running locally.
+    self.BasicArgsHelper(is_running_locally=True)
+    self.BasicArgsHelper(is_running_locally=False)
+
+  def testDesktopNetsimGivesNoFixedPortsNorHostResolverRules(self):
     browser_options = FakeBrowserOptions(
         wpr_mode=wpr_modes.WPR_REPLAY,
         netsim=True)
-    browser_backend = TestChromeBrowserBackend(browser_options)
+    browser_backend = TestChromeBrowserBackend(
+        browser_options,
+        does_forwarder_override_dns=False,
+        is_running_locally=True)
     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))
@@ -85,14 +103,16 @@ class ReplayStartupArgsTest(unittest.TestCase):
         expected_args,
         sorted(browser_backend.GetReplayBrowserStartupArgs()))
 
-  def testForwaderOverridesDnsGivesNoHostResolver(self):
+  def ForwarderOverridesDnsHelper(self, is_netsim):
     # 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)
+        netsim=is_netsim)
     browser_backend = TestChromeBrowserBackend(
-        browser_options, does_forwarder_override_dns=True)
+        browser_options,
+        does_forwarder_override_dns=True,
+        is_running_locally=False)
     browser_backend.wpr_port_pairs = forwarders.PortPairs(
         http=forwarders.PortPair(123, 80),
         https=forwarders.PortPair(234, 443),
@@ -101,3 +121,26 @@ class ReplayStartupArgsTest(unittest.TestCase):
     self.assertEqual(
         expected_args,
         sorted(browser_backend.GetReplayBrowserStartupArgs()))
+
+  def testAndroidRndisGivesNoFixedPortsNorHostResolverRules(self):
+    # The result is the same regardless of netsim setting.
+    self.ForwarderOverridesDnsHelper(is_netsim=True)
+    self.ForwarderOverridesDnsHelper(is_netsim=False)
+
+  def testCrOsNetsimStillUsesHostResolver(self):
+    # CrOS has not implemented the forwarder override for DNS.
+    browser_options = FakeBrowserOptions(
+        wpr_mode=wpr_modes.WPR_REPLAY,
+        netsim=True)
+    browser_backend = TestChromeBrowserBackend(
+        browser_options,
+        does_forwarder_override_dns=False,
+        is_running_locally=False)
+    expected_args = [
+        '--host-resolver-rules=MAP * 127.0.0.1,EXCLUDE localhost',
+        '--ignore-certificate-errors',
+        ]
+    self.assertEqual(
+        expected_args,
+        sorted(browser_backend.GetReplayBrowserStartupArgs()))
+
diff --git a/tools/telemetry/telemetry/core/backends/chrome/cros_browser_backend.py b/tools/telemetry/telemetry/core/backends/chrome/cros_browser_backend.py
index 5bd8f8a..77b28fc 100644
--- a/tools/telemetry/telemetry/core/backends/chrome/cros_browser_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/cros_browser_backend.py
@@ -95,11 +95,6 @@ class CrOSBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
     return args
 
   @property
-  def _use_host_resolver_rules(self):
-    """Always use the --host-resolver-rules Chrome flag; even for netsim."""
-    return True
-
-  @property
   def pid(self):
     return self._cri.GetChromePid()
 
diff --git a/tools/telemetry/telemetry/core/platform/__init__.py b/tools/telemetry/telemetry/core/platform/__init__.py
index e11d51f..1f77028 100644
--- a/tools/telemetry/telemetry/core/platform/__init__.py
+++ b/tools/telemetry/telemetry/core/platform/__init__.py
@@ -98,6 +98,10 @@ class Platform(object):
         self._platform_backend.profiling_controller_backend)
 
   @property
+  def is_host_platform(self):
+    return self == GetHostPlatform()
+
+  @property
   def network_controller(self):
     """Control network settings and servers to simulate the Web."""
     return self._network_controller
diff --git a/tools/telemetry/telemetry/core/platform/platform_backend.py b/tools/telemetry/telemetry/core/platform/platform_backend.py
index 337cdb1..4264f03 100644
--- a/tools/telemetry/telemetry/core/platform/platform_backend.py
+++ b/tools/telemetry/telemetry/core/platform/platform_backend.py
@@ -82,6 +82,10 @@ class PlatformBackend(object):
     return self._platform
 
   @property
+  def is_host_platform(self):
+    return self._platform.is_host_platform
+
+  @property
   def running_browser_backends(self):
     return list(self._running_browser_backends)