Handle replay args within replay module.

tools/telemetry/telemetry/core/webpagereplay.py

Index: tools/telemetry/telemetry/core/webpagereplay.py
diff --git a/tools/telemetry/telemetry/core/webpagereplay.py b/tools/telemetry/telemetry/core/webpagereplay.py
index 19a85bcf1d8764f186618d1c4e130783e9b1ce49..f11e76dcafef5d19954941cca5f2a66dfa68f784 100644
--- a/tools/telemetry/telemetry/core/webpagereplay.py
+++ b/tools/telemetry/telemetry/core/webpagereplay.py
@@ -69,14 +69,11 @@ class ReplayServer(object):
        self.WaitUntil(...)
 
   Environment Variables (for development):
-    WPR_ARCHIVE_PATH: path to alternate archive file (e.g. '/tmp/foo.wpr').
-    WPR_RECORD: if set, puts Web Page Replay in record mode instead of replay.
     WPR_REPLAY_DIR: path to alternate Web Page Replay source.

[nednguyen, 2014/10/06 22:07:50]

Why not also remove this?

[slamm, 2014/10/13 21:45:54]

Done.

   """
 
   def __init__(self, archive_path, replay_host, http_port, https_port, dns_port,
-               replay_options=None, replay_dir=None,
-               log_path=None):
+               replay_options, replay_dir=None, log_path=None):
     """Initialize ReplayServer.
 
     Args:
@@ -93,43 +90,50 @@ class ReplayServer(object):
       replay_dir: directory that has replay.py and related modules.
       log_path: a path to a log file.
     """
-    self.archive_path = os.environ.get('WPR_ARCHIVE_PATH', archive_path)
-    self.replay_options = list(replay_options or ())
     self.replay_dir = os.environ.get('WPR_REPLAY_DIR', replay_dir or REPLAY_DIR)
     self.log_path = log_path or LOG_PATH
     self._replay_host = replay_host
     self._use_dns_server = dns_port is not None
     self._started_ports = {}  # a dict such as {'http': 80, 'https': 443}
 
-    if 'WPR_RECORD' in os.environ and '--record' not in self.replay_options:
-      self.replay_options.append('--record')
-    self.is_record_mode = '--record' in self.replay_options
-    self._AddDefaultReplayOptions(http_port, https_port, dns_port)
+    replay_py = os.path.join(self.replay_dir, 'replay.py')
+    self._cmd_line = self._CommandLine(
+        replay_py, replay_options, archive_path, self._replay_host,
+        http_port, https_port, dns_port)
 
-    self.replay_py = os.path.join(self.replay_dir, 'replay.py')
-
-    if self.is_record_mode:
-      self._CheckPath('archive directory', os.path.dirname(self.archive_path))
-    elif not os.path.exists(self.archive_path):
-      self._CheckPath('archive file', self.archive_path)
-    self._CheckPath('replay script', self.replay_py)
+    if '--record' in replay_options:
+      self._CheckPath('archive directory', os.path.dirname(archive_path))
+    elif not os.path.exists(archive_path):
+      self._CheckPath('archive file', archive_path)
+    self._CheckPath('replay script', replay_py)
 
     self.replay_process = None
 
-  def _AddDefaultReplayOptions(self, http_port, https_port, dns_port):
+  @staticmethod
+  def _CommandLine(replay_py, wpr_args, archive_path, host_ip,
+                   http_port, https_port, dns_port):
     """Set WPR command-line options. Can be overridden if needed."""
-    self.replay_options = [
-        '--host=%s' % self._replay_host,
-        '--port=%s' % http_port,
-        '--ssl_port=%s' % https_port,
-        '--use_closest_match',
-        '--no-dns_forwarding',
-        '--log_level=warning'
-        ] + self.replay_options
-    if self._use_dns_server:
+    cmd_line = [sys.executable, replay_py]
+    cmd_line.extend(wpr_args)
+    cmd_line.extend([
+        '--host=%s' % host_ip,
+        '--port=%s' % http_port,  # if 0, bind to an unused port
+        '--ssl_port=%s' % https_port,  # if 0, bind to an unused port
+        ])
+    if dns_port is None:
+      # Pass --no-dns_forwarding to keep Replay from starting a DNS server.
+      # Otherwise, Replay starts a DNS server even without --dns_port=PORT.
+      cmd_line.append('--no-dns_forwarding')
+    else:
       # Note that if --host is not '127.0.0.1', Replay will override the local
       # DNS nameserver settings to point to the replay-started DNS server.
-      self.replay_options.append('--dns_port=%s' % dns_port)
+      cmd_line.append('--dns_port=%s' % dns_port)
+    cmd_line.extend([
+        '--use_closest_match',
+        '--log_level=warning',
+        archive_path,
+        ])
+    return cmd_line
 
   def _CheckPath(self, label, path):
     if not os.path.exists(path):
@@ -208,17 +212,12 @@ class ReplayServer(object):
     Raises:
       ReplayNotStartedError: if Replay start-up fails.
     """
-    cmd_line = [sys.executable, self.replay_py]
-    cmd_line.extend(self.replay_options)
-    cmd_line.append(self.archive_path)
-
-    logging.debug('Starting Web-Page-Replay: %s', cmd_line)
+    logging.debug('Starting Web-Page-Replay: %s', self._cmd_line)
     with self._OpenLogFile() as log_fh:
       kwargs = {'stdout': log_fh, 'stderr': subprocess.STDOUT}
       if sys.platform.startswith('linux') or sys.platform == 'darwin':
         kwargs['preexec_fn'] = ResetInterruptHandler
-      self.replay_process = subprocess.Popen(cmd_line, **kwargs)
-
+      self.replay_process = subprocess.Popen(self._cmd_line, **kwargs)
     try:
       util.WaitFor(self._IsStarted, 30)
       return (
@@ -252,8 +251,10 @@ class ReplayServer(object):
         self.replay_process.send_signal(signal.SIGINT)
       except:  # pylint: disable=W0702
         # On Windows, we are left with no other option than terminate().
-        if 'no-dns_forwarding' not in self.replay_options:
-          logging.warning('DNS configuration might not be restored!')
+        if self._use_dns_server and self._replay_host == '127.0.0.1':
+          # Replay overrides the DNS nameserver configuration to own DNS server
+          # when the replay host is 127.0.0.1.
+          logging.warning('DNS nameserver configuration might not be restored!')
         try:
           self.replay_process.terminate()
         except:  # pylint: disable=W0702