Add rtc_use_memcheck flag, update MB and GN to handle it, and add gni files listing the runtime deps

tools/mb/mb.py

 #!/usr/bin/env python
 # Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
 #
 # Use of this source code is governed by a BSD-style license
 # that can be found in the LICENSE file in the root of the source
 # tree. An additional intellectual property rights grant can be found
 # in the file PATENTS.  All contributing project authors may
 # be found in the AUTHORS file in the root of the source tree.
 
 """MB - the Meta-Build wrapper around GYP and GN
@@ skipping 1043 matching lines @@
       outp = json.loads(self.ReadFile(self.args.output_path[0]))
       self.Print()
       self.Print('analyze output:')
       self.PrintJSON(outp)
       self.Print()
 
     return ret
 
   def GetIsolateCommand(self, target, vals):
     android = 'target_os="android"' in vals['gn_args']
+    is_linux = self.platform == 'linux2' and not android
+
+    # Memcheck is only supported for linux. Ignore in other platforms.
+    memcheck = is_linux and 'rtc_use_memcheck=true' in vals['gn_args']
 
     # This needs to mirror the settings in //build/config/ui.gni:
     # use_x11 = is_linux && !use_ozone.
-    use_x11 = (self.platform == 'linux2' and
-               not android and
-               not 'use_ozone=true' in vals['gn_args'])
+    use_x11 = is_linux and not 'use_ozone=true' in vals['gn_args']

[kjellander_webrtc, 2016/11/21 08:33:59]

Ouch, so there's logic in here that will change the values of gn variables? This
is likely to cause trouble for us in the future (and I remember this particular
variable also needed an ugly hack in an internal project).

I wondering why not just use_x11 can be used as is here instead, avoiding to
override its value.

[ehmaldonado_webrtc, 2016/11/21 10:51:55]

No, this doesn't override the logic in the GN variables. I think maybe this was
needed by Chromium, we could take a look and just use whatever GN uses.

[ehmaldonado_webrtc, 2016/11/21 11:02:07]

So, MB only has access to the GN flags that we pass to it. |use_x11| is defined
in some .gni file somewhere and we don't have access to it. Hence the comment
that the |use_x11| variable here must be kept in sync with the .gni file (since
we don't have access to it)

 
     asan = 'is_asan=true' in vals['gn_args']
     msan = 'is_msan=true' in vals['gn_args']
     tsan = 'is_tsan=true' in vals['gn_args']
 
     isolate_map = self.ReadIsolateMap()
     test_type = isolate_map[target]['type']
 
     executable = isolate_map[target].get('executable', target)
     executable_suffix = '.exe' if self.platform == 'win32' else ''
     executable_prefix = '.\\' if self.platform == 'win32' else './'
 
     cmdline = []
     extra_files = []
+
+    gtest_cmdline = [
+      'python',
+      '../../third_party/gtest-parallel/gtest-parallel',
+    ]
+    memcheck_cmdline = [
+      'bash',
+      '../../tools/valgrind-webrtc/webrtc_tests.sh',
+      '--tool',
+      'memcheck',
+      '--target',
+      'Release',
+      '--build-dir',
+      '..',
+      '--test',
+    ]
     common_cmdline = [
       executable_prefix + str(executable) + executable_suffix,
       '--',
       '--asan=%d' % asan,
       '--msan=%d' % msan,
       '--tsan=%d' % tsan,
     ]
 
     if test_type == 'nontest':
       self.WriteFailureAndRaise('We should not be isolating %s.' % target,
@@ skipping 20 matching lines @@
     elif use_x11 and test_type == 'windowed_test_launcher':
       extra_files = [
           'xdisplaycheck',
           '../../testing/test_env.py',
           '../../testing/xvfb.py',
           '../../third_party/gtest-parallel/gtest-parallel',
       ]
       cmdline = [
         '../../testing/xvfb.py',
         '.',
-        'python',
-        '../../third_party/gtest-parallel/gtest-parallel',
-      ] + common_cmdline
+      ]
+      if memcheck:

[kjellander_webrtc, 2016/11/21 08:33:58]

There must be a way to avoid duplicating this section and lines 1154-1158, since
test_type == 'windowed_test_launcher' is true for both and this section handles
adding xdisplaycheck due to use_x11 being true.

Please refactor these if-else clauses and add unit tests to verify we get the
resulting command line for the different cases, since this is starting to get
complicated.

+        cmdline += memcheck_cmdline
+      else:
+        cmdline += gtest_cmdline
+      cmdline += common_cmdline
     elif test_type in ('windowed_test_launcher', 'console_test_launcher'):
       extra_files = [
           '../../testing/test_env.py',
           '../../third_party/gtest-parallel/gtest-parallel',
       ]
-      cmdline = [
-          '../../testing/test_env.py',
-          'python',
-          '../../third_party/gtest-parallel/gtest-parallel',
-      ] + common_cmdline
+      cmdline = [ '../../testing/test_env.py' ]
+      if memcheck:

[kjellander_webrtc, 2016/11/21 08:33:58]

You could write this on just one line:
cmdline += memcheck_cmdline if memcheck else gtest_cmdline

+        cmdline += memcheck_cmdline
+      else:
+        cmdline += gtest_cmdline
+      cmdline += common_cmdline
     elif test_type == 'non_parallel_console_test_launcher':
       extra_files = [
           '../../testing/test_env.py',
       ]
       cmdline = [
           '../../testing/test_env.py',
-      ] + common_cmdline
+      ]
+      if memcheck:
+        cmdline += memcheck_cmdline
+      cmdline += common_cmdline
     else:
       self.WriteFailureAndRaise('No command line for %s found (test type %s).'
                                 % (target, test_type), output_path=None)
 
     cmdline += isolate_map[target].get('args', [])
 
     return cmdline, extra_files
 
   def ToAbsPath(self, build_path, *comps):
     return self.PathJoin(self.chromium_src_dir,
@@ skipping 399 matching lines @@
   # Then check to see if the arg contains any metacharacters other than
   # double quotes; if it does, quote everything (including the double
   # quotes) for safety.
   if any(a in UNSAFE_FOR_CMD for a in arg):
     arg = ''.join('^' + a if a in ALL_META_CHARS else a for a in arg)
   return arg
 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))