Create AndroidCommandLineBackend to handle setting up cmdline args

BUG=448204,441959

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

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

tools/telemetry/telemetry/core/backends/android_app_backend.py

@@ -10,9 +10,12 @@ from telemetry.core import util
 from telemetry.core import web_contents
 from telemetry.core.backends import adb_commands
 from telemetry.core.backends import app_backend
+from telemetry.core.backends import android_browser_backend_settings
+from telemetry.core.backends import android_command_line_backend
 
 
 class AndroidAppBackend(app_backend.AppBackend):
+
   def __init__(self, android_platform_backend, start_intent,
                is_app_ready_predicate=None):
     super(AndroidAppBackend, self).__init__(
@@ -36,10 +39,15 @@ class AndroidAppBackend(app_backend.AppBackend):
     AppStory derivations can customize the wait-for-ready-state to wait
     for a more specific event if needed.
     """
-    # TODO(slamm): check if can use "blocking=True" instead of needing to sleep.
-    # If "blocking=True" does not work, switch sleep to "ps" check.
-    self._adb.device().StartActivity(self._start_intent, blocking=False)
-    util.WaitFor(self._IsAppReady, timeout=60)
+    webview_startup_args = self.GetWebviewStartupArgs()
+    backend_settings = android_browser_backend_settings.WebviewBackendSettings(
+        'android-webview')
+    with android_command_line_backend.SetUpCommandLineFlags(
+        self._adb, backend_settings, webview_startup_args):
+      # TODO(slamm): check if can use "blocking=True" instead of needing to
+      # sleep. If "blocking=True" does not work, switch sleep to "ps" check.
+      self._adb.device().StartActivity(self._start_intent, blocking=False)
+      util.WaitFor(self._IsAppReady, timeout=60)
     self._is_running = True
 
   def Close(self):
@@ -81,3 +89,15 @@ class AndroidAppBackend(app_backend.AppBackend):
     for process in self.GetProcesses():
       webviews.update(process.GetWebViews())
     return webviews
+
+  def GetWebviewStartupArgs(self):
+    args = []
+
+    # Turn on GPU benchmarking extension for all runs. The only side effect of
+    # the extension being on is that render stats are tracked. This is believed
+    # to be effectively free. And, by doing so here, it avoids us having to
+    # programmatically inspect a pageset's actions in order to determine if it
+    # might eventually scroll.
+    args.append('--enable-gpu-benchmarking')
+
+    return args

tools/telemetry/telemetry/core/backends/android_browser_backend_settings.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 logging
+import time
+
+from telemetry.core import exceptions
+
+
+class AndroidBrowserBackendSettings(object):
+
+  def __init__(self, activity, cmdline_file, package, pseudo_exec_name,
+               supports_tab_control):
+    self._activity = activity
+    self._cmdline_file = cmdline_file
+    self._package = package
+    self._pseudo_exec_name = pseudo_exec_name
+    self._supports_tab_control = supports_tab_control
+
+  @property
+  def activity(self):
+    return self._activity
+
+  @property
+  def package(self):
+    return self._package
+
+  @property
+  def pseudo_exec_name(self):
+    return self._pseudo_exec_name
+
+  @property
+  def supports_tab_control(self):
+    return self._supports_tab_control
+
+  def GetCommandLineFile(self, is_user_debug_build):  # pylint: disable=W0613
+    return self._cmdline_file
+
+  def GetDevtoolsRemotePort(self, adb):
+    raise NotImplementedError()
+
+  @property
+  def profile_ignore_list(self):
+    # Don't delete lib, since it is created by the installer.
+    return ['lib']
+
+
+class ChromeBackendSettings(AndroidBrowserBackendSettings):
+  # Stores a default Preferences file, re-used to speed up "--page-repeat".
+  _default_preferences_file = None
+
+  def GetCommandLineFile(self, is_user_debug_build):
+    if is_user_debug_build:
+      return '/data/local/tmp/chrome-command-line'
+    else:
+      return '/data/local/chrome-command-line'
+
+  def __init__(self, package):
+    super(ChromeBackendSettings, self).__init__(
+        activity='com.google.android.apps.chrome.Main',
+        cmdline_file=None,
+        package=package,
+        pseudo_exec_name='chrome',
+        supports_tab_control=True)
+
+  def GetDevtoolsRemotePort(self, adb):
+    return 'localabstract:chrome_devtools_remote'
+
+
+class ContentShellBackendSettings(AndroidBrowserBackendSettings):
+  def __init__(self, package):
+    super(ContentShellBackendSettings, self).__init__(
+        activity='org.chromium.content_shell_apk.ContentShellActivity',
+        cmdline_file='/data/local/tmp/content-shell-command-line',
+        package=package,
+        pseudo_exec_name='content_shell',
+        supports_tab_control=False)
+
+  def GetDevtoolsRemotePort(self, adb):
+    return 'localabstract:content_shell_devtools_remote'
+
+
+class ChromeShellBackendSettings(AndroidBrowserBackendSettings):
+  def __init__(self, package):
+    super(ChromeShellBackendSettings, self).__init__(
+          activity='org.chromium.chrome.shell.ChromeShellActivity',
+          cmdline_file='/data/local/tmp/chrome-shell-command-line',
+          package=package,
+          pseudo_exec_name='chrome_shell',
+          supports_tab_control=False)
+
+  def GetDevtoolsRemotePort(self, adb):
+    return 'localabstract:chrome_shell_devtools_remote'
+
+
+class WebviewBackendSettings(AndroidBrowserBackendSettings):
+  def __init__(self,
+               package,
+               activity='org.chromium.telemetry_shell.TelemetryActivity',
+               cmdline_file='/data/local/tmp/webview-command-line'):
+    super(WebviewBackendSettings, self).__init__(
+        activity=activity,
+        cmdline_file=cmdline_file,
+        package=package,
+        pseudo_exec_name='webview',
+        supports_tab_control=False)
+
+  def GetDevtoolsRemotePort(self, adb):
+    # The DevTools socket name for WebView depends on the activity PID's.
+    retries = 0
+    timeout = 1
+    pid = None
+    while True:
+      pids = adb.ExtractPid(self.package)
+      if len(pids) > 0:
+        pid = pids[-1]
+        break
+      time.sleep(timeout)
+      retries += 1
+      timeout *= 2
+      if retries == 4:
+        logging.critical('android_browser_backend: Timeout while waiting for '
+                         'activity %s:%s to come up',
+                         self.package,
+                         self.activity)
+        raise exceptions.BrowserGoneException(self.browser,
+                                              'Timeout waiting for PID.')
+    return 'localabstract:webview_devtools_remote_%s' % str(pid)
+
+
+class WebviewShellBackendSettings(WebviewBackendSettings):
+  def __init__(self, package):
+    super(WebviewShellBackendSettings, self).__init__(
+        activity='org.chromium.android_webview.shell.AwShellActivity',
+        cmdline_file='/data/local/tmp/android-webview-command-line',
+        package=package)

tools/telemetry/telemetry/core/backends/android_command_line_backend.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 logging
+import pipes
+import sys
+
+from telemetry.core import util
+
+util.AddDirToPythonPath(util.GetChromiumSrcDir(), 'build', 'android')
+from pylib.device import device_errors  # pylint: disable=F0401
+
+
+def _QuoteIfNeeded(arg):
+  # Properly escape "key=valueA valueB" to "key='valueA valueB'"
+  # Values without spaces, or that seem to be quoted are left untouched.
+  # This is required so CommandLine.java can parse valueB correctly rather
+  # than as a separate switch.
+  params = arg.split('=', 1)
+  if len(params) != 2:
+    return arg
+  key, values = params
+  if ' ' not in values:
+    return arg
+  if values[0] in '"\'' and values[-1] == values[0]:
+    return arg
+  return '%s=%s' % (key, pipes.quote(values))
+
+
+class SetUpCommandLineFlags(object):
+  """A context manager for setting up the android command line flags.
+
+  This provides a readable way of using the android command line backend class.
+  Example usage:
+
+      with android_command_line_backend.SetUpCommandLineFlags(
+          adb, backend_settings, startup_args):
+        # Something to run while the command line flags are set appropriately.
+  """
+  def __init__(self, adb, backend_settings, startup_args):
+    self._android_command_line_backend = _AndroidCommandLineBackend(
+        adb, backend_settings, startup_args)
+
+  def __enter__(self):
+    self._android_command_line_backend.SetUpCommandLineFlags()
+
+  def __exit__(self, *args):
+    self._android_command_line_backend.RestoreCommandLineFlags()
+
+
+class _AndroidCommandLineBackend(object):
+  """The backend for providing command line flags on android.
+
+  There are command line flags that Chromium accept in order to enable
+  particular features or modify otherwise default functionality. To set the
+  flags for Chrome on Android, specific files on the device must be updated
+  with the flags to enable. This class provides a wrapper around this
+  functionality.
+  """
+
+  def __init__(self, adb, backend_settings, startup_args):
+    self._adb = adb
+    self._backend_settings = backend_settings
+    self._startup_args = startup_args
+    self._saved_command_line_file_contents = ''
+
+  @property
+  def command_line_file(self):
+    return self._backend_settings.GetCommandLineFile(self._adb.IsUserBuild())
+
+  def SetUpCommandLineFlags(self):
+    args = [self._backend_settings.pseudo_exec_name]
+    args.extend(self._startup_args)
+    content = ' '.join(_QuoteIfNeeded(arg) for arg in args)
+
+    try:
+      # Save the current command line to restore later, except if it appears to
+      # be a  Telemetry created one. This is to prevent a common bug where
+      # --host-resolver-rules borks people's browsers if something goes wrong
+      # with Telemetry.
+      self._saved_command_line_file_contents = ''.join(self._ReadFile())
+      if '--host-resolver-rules' in self._saved_command_line_file_contents:
+        self._saved_command_line_file_contents = ''
+      self._WriteFile(content)
+    except device_errors.CommandFailedError:
+      logging.critical('Cannot set Chrome command line. '
+                       'Fix this by flashing to a userdebug build.')
+      sys.exit(1)
+
+  def RestoreCommandLineFlags(self):
+    self._WriteFile(self._saved_command_line_file_contents)
+
+  def _ReadFile(self):
+    return self._adb.device().ReadFile(self.command_line_file)
+
+  def _WriteFile(self, contents):
+    self._adb.device().WriteFile(self.command_line_file, contents, as_root=True)

tools/telemetry/telemetry/core/backends/android_command_line_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.backends import android_command_line_backend
+
+
+class AndroidCommandLineBackendTest(unittest.TestCase):
+
+  def testQuoteIfNeededNoEquals(self):
+    string = 'value'
+    self.assertEqual(string,
+                     android_command_line_backend._QuoteIfNeeded(string))
+
+  def testQuoteIfNeededNoSpaces(self):
+    string = 'key=valueA'
+    self.assertEqual(string,
+                     android_command_line_backend._QuoteIfNeeded(string))
+
+  def testQuoteIfNeededAlreadyQuoted(self):
+    string = "key='valueA valueB'"
+    self.assertEqual(string,
+                     android_command_line_backend._QuoteIfNeeded(string))
+
+  def testQuoteIfNeeded(self):
+    string = 'key=valueA valueB'
+    expected_output = "key='valueA valueB'"
+    self.assertEqual(expected_output,
+                     android_command_line_backend._QuoteIfNeeded(string))

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

@@ -11,6 +11,7 @@ from telemetry.core import exceptions
 from telemetry.core import forwarders
 from telemetry.core import util
 from telemetry.core.backends import adb_commands
+from telemetry.core.backends import android_command_line_backend
 from telemetry.core.backends import browser_backend
 from telemetry.core.backends.chrome import chrome_browser_backend
 from telemetry.core.platform import android_platform_backend as \
@@ -22,118 +23,6 @@ from pylib.device import device_errors  # pylint: disable=F0401
 from pylib.device import intent  # pylint: disable=F0401
 
 
-class AndroidBrowserBackendSettings(object):
-
-  def __init__(self, activity, cmdline_file, package, pseudo_exec_name,
-               supports_tab_control):
-    self.activity = activity
-    self._cmdline_file = cmdline_file
-    self.package = package
-    self.pseudo_exec_name = pseudo_exec_name
-    self.supports_tab_control = supports_tab_control
-
-  def GetCommandLineFile(self, is_user_debug_build):  # pylint: disable=W0613
-    return self._cmdline_file
-
-  def GetDevtoolsRemotePort(self, adb):
-    raise NotImplementedError()
-
-  @property
-  def profile_ignore_list(self):
-    # Don't delete lib, since it is created by the installer.
-    return ['lib']
-
-
-class ChromeBackendSettings(AndroidBrowserBackendSettings):
-  # Stores a default Preferences file, re-used to speed up "--page-repeat".
-  _default_preferences_file = None
-
-  def GetCommandLineFile(self, is_user_debug_build):
-    if is_user_debug_build:
-      return '/data/local/tmp/chrome-command-line'
-    else:
-      return '/data/local/chrome-command-line'
-
-  def __init__(self, package):
-    super(ChromeBackendSettings, self).__init__(
-        activity='com.google.android.apps.chrome.Main',
-        cmdline_file=None,
-        package=package,
-        pseudo_exec_name='chrome',
-        supports_tab_control=True)
-
-  def GetDevtoolsRemotePort(self, adb):
-    return 'localabstract:chrome_devtools_remote'
-
-
-class ContentShellBackendSettings(AndroidBrowserBackendSettings):
-  def __init__(self, package):
-    super(ContentShellBackendSettings, self).__init__(
-        activity='org.chromium.content_shell_apk.ContentShellActivity',
-        cmdline_file='/data/local/tmp/content-shell-command-line',
-        package=package,
-        pseudo_exec_name='content_shell',
-        supports_tab_control=False)
-
-  def GetDevtoolsRemotePort(self, adb):
-    return 'localabstract:content_shell_devtools_remote'
-
-
-class ChromeShellBackendSettings(AndroidBrowserBackendSettings):
-  def __init__(self, package):
-    super(ChromeShellBackendSettings, self).__init__(
-          activity='org.chromium.chrome.shell.ChromeShellActivity',
-          cmdline_file='/data/local/tmp/chrome-shell-command-line',
-          package=package,
-          pseudo_exec_name='chrome_shell',
-          supports_tab_control=False)
-
-  def GetDevtoolsRemotePort(self, adb):
-    return 'localabstract:chrome_shell_devtools_remote'
-
-
-class WebviewBackendSettings(AndroidBrowserBackendSettings):
-  def __init__(self, package,
-               activity='org.chromium.telemetry_shell.TelemetryActivity',
-               cmdline_file='/data/local/tmp/webview-command-line'):
-    super(WebviewBackendSettings, self).__init__(
-        activity=activity,
-        cmdline_file=cmdline_file,
-        package=package,
-        pseudo_exec_name='webview',
-        supports_tab_control=False)
-
-  def GetDevtoolsRemotePort(self, adb):
-    # The DevTools socket name for WebView depends on the activity PID's.
-    retries = 0
-    timeout = 1
-    pid = None
-    while True:
-      pids = adb.ExtractPid(self.package)
-      if len(pids) > 0:
-        pid = pids[-1]
-        break
-      time.sleep(timeout)
-      retries += 1
-      timeout *= 2
-      if retries == 4:
-        logging.critical('android_browser_backend: Timeout while waiting for '
-                         'activity %s:%s to come up',
-                         self.package,
-                         self.activity)
-        raise exceptions.BrowserGoneException(self.browser,
-                                              'Timeout waiting for PID.')
-    return 'localabstract:webview_devtools_remote_%s' % str(pid)
-
-
-class WebviewShellBackendSettings(WebviewBackendSettings):
-  def __init__(self, package):
-    super(WebviewShellBackendSettings, self).__init__(
-        activity='org.chromium.android_webview.shell.AwShellActivity',
-        cmdline_file='/data/local/tmp/android-webview-command-line',
-        package=package)
-
-
 class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
   """The backend for controlling a browser instance running on Android."""
   def __init__(self, android_platform_backend, browser_options,
@@ -153,7 +42,6 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
 
     # Initialize fields so that an explosion during init doesn't break in Close.
     self._backend_settings = backend_settings
-    self._saved_cmdline = ''
     self._target_arch = target_arch
     self._saved_sslflag = ''
 
@@ -197,50 +85,7 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
   def _KillBrowser(self):
     self.platform_backend.KillApplication(self._backend_settings.package)
 
-  def _SetUpCommandLine(self):
-    def QuoteIfNeeded(arg):
-      # Properly escape "key=valueA valueB" to "key='valueA valueB'"
-      # Values without spaces, or that seem to be quoted are left untouched.
-      # This is required so CommandLine.java can parse valueB correctly rather
-      # than as a separate switch.
-      params = arg.split('=', 1)
-      if len(params) != 2:
-        return arg
-      key, values = params
-      if ' ' not in values:
-        return arg
-      if values[0] in '"\'' and values[-1] == values[0]:
-        return arg
-      return '%s=%s' % (key, pipes.quote(values))
-    args = [self._backend_settings.pseudo_exec_name]
-    args.extend(self.GetBrowserStartupArgs())
-    content = ' '.join(QuoteIfNeeded(arg) for arg in args)
-    cmdline_file = self._backend_settings.GetCommandLineFile(
-        self._adb.IsUserBuild())
-
-    try:
-      # Save the current command line to restore later, except if it appears to
-      # be a  Telemetry created one. This is to prevent a common bug where
-      # --host-resolver-rules borks people's browsers if something goes wrong
-      # with Telemetry.
-      self._saved_cmdline = ''.join(self._adb.device().ReadFile(cmdline_file))
-      if '--host-resolver-rules' in self._saved_cmdline:
-        self._saved_cmdline = ''
-      self._adb.device().WriteFile(cmdline_file, content, as_root=True)
-    except device_errors.CommandFailedError:
-      logging.critical('Cannot set Chrome command line. '
-                       'Fix this by flashing to a userdebug build.')
-      sys.exit(1)
-
-  def _RestoreCommandLine(self):
-    cmdline_file = self._backend_settings.GetCommandLineFile(
-        self._adb.IsUserBuild())
-    self._adb.device().WriteFile(cmdline_file, self._saved_cmdline,
-                                 as_root=True)
-
   def Start(self):
-    self._SetUpCommandLine()
-
     self._adb.device().RunShellCommand('logcat -c')
     if self.browser_options.startup_url:
       url = self.browser_options.startup_url
@@ -253,34 +98,36 @@ class AndroidBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
 
     self.platform_backend.DismissCrashDialogIfNeeded()
 
-    self._adb.device().StartActivity(
-        intent.Intent(package=self._backend_settings.package,
-                      activity=self._backend_settings.activity,
-                      action=None, data=url, category=None),
-        blocking=True)
-
     remote_devtools_port = self._backend_settings.GetDevtoolsRemotePort(
         self._adb)
     self.platform_backend.ForwardHostToDevice(self._port, remote_devtools_port)
 
-    try:
-      self._WaitForBrowserToComeUp(remote_devtools_port=remote_devtools_port)
-    except exceptions.BrowserGoneException:
-      logging.critical('Failed to connect to browser.')
-      if not self._adb.device().old_interface.CanAccessProtectedFileContents():
-        logging.critical(
-          'Resolve this by either: '
-          '(1) Flashing to a userdebug build OR '
-          '(2) Manually enabling web debugging in Chrome at '
-          'Settings > Developer tools > Enable USB Web debugging.')
-      sys.exit(1)
-    except:
-      import traceback
-      traceback.print_exc()
-      self.Close()
-      raise
-    finally:
-      self._RestoreCommandLine()
+    browser_startup_args = self.GetBrowserStartupArgs()
+    with android_command_line_backend.SetUpCommandLineFlags(
+        self._adb, self._backend_settings, browser_startup_args):
+      self._adb.device().StartActivity(
+          intent.Intent(package=self._backend_settings.package,
+                        activity=self._backend_settings.activity,
+                        action=None, data=url, category=None),
+          blocking=True)
+
+      try:
+        self._WaitForBrowserToComeUp(remote_devtools_port=remote_devtools_port)
+      except exceptions.BrowserGoneException:
+        logging.critical('Failed to connect to browser.')
+        device = self._adb.device()
+        if not device.old_interface.CanAccessProtectedFileContents():
+          logging.critical(
+            'Resolve this by either: '
+            '(1) Flashing to a userdebug build OR '
+            '(2) Manually enabling web debugging in Chrome at '
+            'Settings > Developer tools > Enable USB Web debugging.')
+        sys.exit(1)
+      except:
+        import traceback
+        traceback.print_exc()
+        self.Close()
+        raise
 
   def GetBrowserStartupArgs(self):
     args = super(AndroidBrowserBackend, self).GetBrowserStartupArgs()

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

@@ -14,46 +14,47 @@ from telemetry.core import possible_browser
 from telemetry.core import platform
 from telemetry.core import util
 from telemetry.core.backends import adb_commands
-from telemetry.core.platform import android_device
+from telemetry.core.backends import android_browser_backend_settings
 from telemetry.core.backends.chrome import android_browser_backend
+from telemetry.core.platform import android_device
 
 
 CHROME_PACKAGE_NAMES = {
   'android-content-shell':
       ['org.chromium.content_shell_apk',
-       android_browser_backend.ContentShellBackendSettings,
+       android_browser_backend_settings.ContentShellBackendSettings,
        'ContentShell.apk'],
   'android-chrome-shell':
       ['org.chromium.chrome.shell',
-       android_browser_backend.ChromeShellBackendSettings,
+       android_browser_backend_settings.ChromeShellBackendSettings,
        'ChromeShell.apk'],
   'android-webview':
       ['org.chromium.telemetry_shell',
-       android_browser_backend.WebviewBackendSettings,
+       android_browser_backend_settings.WebviewBackendSettings,
        None],
   'android-webview-shell':
       ['org.chromium.android_webview.shell',
-       android_browser_backend.WebviewShellBackendSettings,
+       android_browser_backend_settings.WebviewShellBackendSettings,
        'AndroidWebView.apk'],
   'android-chrome':
       ['com.google.android.apps.chrome',
-       android_browser_backend.ChromeBackendSettings,
+       android_browser_backend_settings.ChromeBackendSettings,
        'Chrome.apk'],
   'android-chrome-beta':
       ['com.chrome.beta',
-       android_browser_backend.ChromeBackendSettings,
+       android_browser_backend_settings.ChromeBackendSettings,
        None],
   'android-chrome-dev':
       ['com.google.android.apps.chrome_dev',
-       android_browser_backend.ChromeBackendSettings,
+       android_browser_backend_settings.ChromeBackendSettings,
        None],
   'android-chrome-canary':
       ['com.chrome.canary',
-       android_browser_backend.ChromeBackendSettings,
+       android_browser_backend_settings.ChromeBackendSettings,
        None],
   'android-jb-system-chrome':
       ['com.android.chrome',
-       android_browser_backend.ChromeBackendSettings,
+       android_browser_backend_settings.ChromeBackendSettings,
        None]
 }